[ES][v2] Change the DB Tag value from string to any type (jaegertracing#6994)

Manik2708 · amol-verma-allen · commit 6a90827e7e22 · 2025-05-05T03:37:31.000+05:30
## Which problem is this PR solving? Fixes a part of: jaegertracing#6458 ## Description of the changes - As discussed in jaegertracing#6946 we need to change the value which was saved as string in v1 to a raw value in v2 ## How was this change tested? - Unit Tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: Manik2708 <mehtamanik96@gmail.com> Signed-off-by: amol-verma-allen <amol.verma@allen.in>
diff --git a/internal/storage/v2/elasticsearch/tracestore/fixtures/es_01.json b/internal/storage/v2/elasticsearch/tracestore/fixtures/es_01.json
@@ -42,7 +42,7 @@
     {
       "key": "peer.ipv4",
       "type": "int64",
-      "value": "23456"
+      "value": 23456
     },
     {
       "key": "blob",
@@ -52,12 +52,12 @@
     {
       "key": "temperature",
       "type": "float64",
-      "value": "72.5"
+      "value": 72.5
     },
     {
       "key": "error",
       "type": "bool",
-      "value": "true"
+      "value": true
     }
   ],
   "logs": [
diff --git a/internal/storage/v2/elasticsearch/tracestore/fixtures/es_01_string_tags.json b/internal/storage/v2/elasticsearch/tracestore/fixtures/es_01_string_tags.json
@@ -0,0 +1,100 @@
+{
+  "traceID": "00000000000000010000000000000000",
+  "spanID": "0000000000000002",
+  "flags": 1,
+  "operationName": "test-general-conversion",
+  "references": [
+    {
+      "refType": "CHILD_OF",
+      "traceID": "00000000000000010000000000000000",
+      "spanID": "0000000000000003"
+    },
+    {
+      "refType": "FOLLOWS_FROM",
+      "traceID": "00000000000000010000000000000000",
+      "spanID": "0000000000000004"
+    },
+    {
+      "refType": "CHILD_OF",
+      "traceID": "00000000000000ff0000000000000000",
+      "spanID": "00000000000000ff"
+    }
+  ],
+  "startTime": 1485467191639875,
+  "startTimeMillis": 1485467191639,
+  "duration": 5,
+  "tags": [
+    {
+      "key": "otel.scope.name",
+      "type": "string",
+      "value": "testing-library"
+    },
+    {
+      "key": "otel.scope.version",
+      "type": "string",
+      "value": "1.1.1"
+    },
+    {
+      "key": "peer.service",
+      "type": "string",
+      "value": "service-y"
+    },
+    {
+      "key": "peer.ipv4",
+      "type": "int64",
+      "value": "23456"
+    },
+    {
+      "key": "blob",
+      "type": "binary",
+      "value": "00003039"
+    },
+    {
+      "key": "temperature",
+      "type": "float64",
+      "value": "72.5"
+    },
+    {
+      "key": "error",
+      "type": "bool",
+      "value": "true"
+    }
+  ],
+  "logs": [
+    {
+      "timestamp": 1485467191639875,
+      "fields": [
+        {
+          "key": "event",
+          "type": "string",
+          "value": "testing-event"
+        },
+        {
+          "key": "event-x",
+          "type": "string",
+          "value": "event-y"
+        }
+      ]
+    },
+    {
+      "timestamp": 1485467191639875,
+      "fields": [
+        {
+          "key": "x",
+          "type": "string",
+          "value": "y"
+        }
+      ]
+    }
+  ],
+  "process": {
+    "serviceName": "service-x",
+    "tags": [
+      {
+        "key": "sdk.version",
+        "type": "string",
+        "value": "1.2.1"
+      }
+    ]
+  }
+}
diff --git a/internal/storage/v2/elasticsearch/tracestore/from_dbmodel.go b/internal/storage/v2/elasticsearch/tracestore/from_dbmodel.go
@@ -8,6 +8,7 @@ package tracestore
 
 import (
 	"encoding/hex"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"strconv"
@@ -134,12 +135,19 @@ func dbTagsToAttributes(tags []dbmodel.KeyValue, attributes pcommon.Map) {
 	for _, tag := range tags {
 		tagValue, ok := tag.Value.(string)
 		if !ok {
-			// We have to do this as we are unsure that whether bool will be a string or a bool
-			tagBoolVal, boolOk := tag.Value.(bool)
-			if boolOk && tag.Type == dbmodel.BoolType {
-				attributes.PutBool(tag.Key, tagBoolVal)
-			} else {
-				attributes.PutStr(tag.Key, "Got non string inputValue for the key "+tag.Key)
+			switch tag.Type {
+			case dbmodel.Float64Type, dbmodel.Int64Type:
+				fromDBNumber(tag, attributes)
+			case dbmodel.BoolType:
+				v, ok := tag.Value.(bool)
+				if !ok {
+					recordTagInvalidTypeError(tag, attributes)
+				} else {
+					attributes.PutBool(tag.Key, v)
+				}
+			default:
+				// This means type is string/binary but value is of non string type, hence record the type error
+				recordTagInvalidTypeError(tag, attributes)
 			}
 			continue
 		}
@@ -180,6 +188,41 @@ func dbTagsToAttributes(tags []dbmodel.KeyValue, attributes pcommon.Map) {
 	}
 }
 
+func fromDBNumber(kv dbmodel.KeyValue, dest pcommon.Map) {
+	if kv.Type == dbmodel.Int64Type {
+		switch v := kv.Value.(type) {
+		case int64:
+			dest.PutInt(kv.Key, v)
+		case float64:
+			// This case is possible as unmarshalling the JSON converts every int value to float64
+			dest.PutInt(kv.Key, int64(v))
+		case json.Number:
+			n, err := v.Int64()
+			if err == nil {
+				dest.PutInt(kv.Key, n)
+			}
+		default:
+			recordTagInvalidTypeError(kv, dest)
+		}
+	} else if kv.Type == dbmodel.Float64Type {
+		switch v := kv.Value.(type) {
+		case float64:
+			dest.PutDouble(kv.Key, v)
+		case json.Number:
+			n, err := v.Float64()
+			if err == nil {
+				dest.PutDouble(kv.Key, n)
+			}
+		default:
+			recordTagInvalidTypeError(kv, dest)
+		}
+	}
+}
+
+func recordTagInvalidTypeError(kv dbmodel.KeyValue, dest pcommon.Map) {
+	dest.PutStr(kv.Key, fmt.Sprintf("invalid %s type in %+v", string(kv.Type), kv.Value))
+}
+
 func recordTagConversionError(kv dbmodel.KeyValue, err error, dest pcommon.Map) {
 	dest.PutStr(kv.Key, fmt.Sprintf("Can't convert the type %s for the key %s: %v", string(kv.Type), kv.Key, err))
 }
diff --git a/internal/storage/v2/elasticsearch/tracestore/from_dbmodel_test.go b/internal/storage/v2/elasticsearch/tracestore/from_dbmodel_test.go
@@ -215,6 +215,17 @@ func TestSetAttributesFromDbTags(t *testing.T) {
 				p.PutBool("bool-val", true)
 			},
 		},
+		{
+			name: "wrong non string int input value",
+			keyModel: dbmodel.KeyValue{
+				Key:   "bool-val",
+				Type:  dbmodel.BoolType,
+				Value: 12,
+			},
+			expectedValueFn: func(p pcommon.Map) {
+				p.PutStr("bool-val", "invalid bool type in 12")
+			},
+		},
 		{
 			name: "wrong int input value",
 			keyModel: dbmodel.KeyValue{
@@ -237,6 +248,50 @@ func TestSetAttributesFromDbTags(t *testing.T) {
 				p.PutInt("int-val", 123)
 			},
 		},
+		{
+			name: "right non string int input value",
+			keyModel: dbmodel.KeyValue{
+				Key:   "int-val",
+				Type:  dbmodel.Int64Type,
+				Value: int64(123),
+			},
+			expectedValueFn: func(p pcommon.Map) {
+				p.PutInt("int-val", 123)
+			},
+		},
+		{
+			name: "right non string int float input value",
+			keyModel: dbmodel.KeyValue{
+				Key:   "int-val",
+				Type:  dbmodel.Int64Type,
+				Value: float64(123),
+			},
+			expectedValueFn: func(p pcommon.Map) {
+				p.PutInt("int-val", 123)
+			},
+		},
+		{
+			name: "right non string json number int input value",
+			keyModel: dbmodel.KeyValue{
+				Key:   "int-val",
+				Type:  dbmodel.Int64Type,
+				Value: json.Number("123"),
+			},
+			expectedValueFn: func(p pcommon.Map) {
+				p.PutInt("int-val", 123)
+			},
+		},
+		{
+			name: "wrong non string int input value",
+			keyModel: dbmodel.KeyValue{
+				Key:   "int-val",
+				Type:  dbmodel.Int64Type,
+				Value: true,
+			},
+			expectedValueFn: func(p pcommon.Map) {
+				p.PutStr("int-val", "invalid int64 type in true")
+			},
+		},
 		{
 			name: "wrong double input value",
 			keyModel: dbmodel.KeyValue{
@@ -259,6 +314,39 @@ func TestSetAttributesFromDbTags(t *testing.T) {
 				p.PutDouble("double-val", 1.23)
 			},
 		},
+		{
+			name: "right non string double input value",
+			keyModel: dbmodel.KeyValue{
+				Key:   "double-val",
+				Type:  dbmodel.Float64Type,
+				Value: 25.6,
+			},
+			expectedValueFn: func(p pcommon.Map) {
+				p.PutDouble("double-val", 25.6)
+			},
+		},
+		{
+			name: "right non string json number double input value",
+			keyModel: dbmodel.KeyValue{
+				Key:   "double-val",
+				Type:  dbmodel.Float64Type,
+				Value: json.Number("123.56"),
+			},
+			expectedValueFn: func(p pcommon.Map) {
+				p.PutDouble("double-val", 123.56)
+			},
+		},
+		{
+			name: "wrong non string float input value",
+			keyModel: dbmodel.KeyValue{
+				Key:   "double-val",
+				Type:  dbmodel.Float64Type,
+				Value: true,
+			},
+			expectedValueFn: func(p pcommon.Map) {
+				p.PutStr("double-val", "invalid float64 type in true")
+			},
+		},
 		{
 			name: "wrong binary input value",
 			keyModel: dbmodel.KeyValue{
@@ -282,14 +370,14 @@ func TestSetAttributesFromDbTags(t *testing.T) {
 			},
 		},
 		{
-			name: "non-string input value",
+			name: "non-string input value with string type",
 			keyModel: dbmodel.KeyValue{
 				Key:   "bool-val",
-				Type:  dbmodel.Int64Type,
+				Type:  dbmodel.StringType,
 				Value: 123,
 			},
 			expectedValueFn: func(p pcommon.Map) {
-				p.PutStr("bool-val", "Got non string inputValue for the key bool-val")
+				p.PutStr("bool-val", "invalid string type in 123")
 			},
 		},
 		{
@@ -551,6 +639,17 @@ func TestFromDbModel_Fixtures(t *testing.T) {
 	testTraces(t, tracesData, actualTd)
 }
 
+func TestToDbModel_Fixtures_StringTags(t *testing.T) {
+	spanData, err := os.ReadFile("fixtures/es_01_string_tags.json")
+	require.NoError(t, err)
+	var dbSpan dbmodel.Span
+	require.NoError(t, json.Unmarshal(spanData, &dbSpan))
+	td, err := FromDBModel([]dbmodel.Span{dbSpan})
+	require.NoError(t, err)
+	expectedTraces := loadTraces(t, 1)
+	testTraces(t, expectedTraces, td)
+}
+
 func getDbTraceIdFromByteArray(arr [16]byte) dbmodel.TraceID {
 	return dbmodel.TraceID(hex.EncodeToString(arr[:]))
 }
diff --git a/internal/storage/v2/elasticsearch/tracestore/to_dbmodel.go b/internal/storage/v2/elasticsearch/tracestore/to_dbmodel.go
@@ -100,11 +100,13 @@ func appendTagsFromAttributes(dest []dbmodel.KeyValue, attrs pcommon.Map) []dbmo
 
 func attributeToDbTag(key string, attr pcommon.Value) dbmodel.KeyValue {
 	var tag dbmodel.KeyValue
-	if attr.Type() == pcommon.ValueTypeBytes {
+	switch attr.Type() {
+	case pcommon.ValueTypeBytes:
 		tag = dbmodel.KeyValue{Key: key, Value: hex.EncodeToString(attr.Bytes().AsRaw())}
-	} else {
-		// TODO why are all values being converted to strings?
+	case pcommon.ValueTypeMap:
 		tag = dbmodel.KeyValue{Key: key, Value: attr.AsString()}
+	default:
+		tag = dbmodel.KeyValue{Key: key, Value: attr.AsRaw()}
 	}
 	switch attr.Type() {
 	case pcommon.ValueTypeStr:
@@ -294,7 +296,7 @@ func getTagFromStatusCode(statusCode ptrace.StatusCode) (dbmodel.KeyValue, bool)
 		return dbmodel.KeyValue{
 			Key:   tagError,
 			Type:  dbmodel.BoolType,
-			Value: "true",
+			Value: true,
 		}, true
 	} else if statusCode == ptrace.StatusCodeOk {
 		return dbmodel.KeyValue{
diff --git a/internal/storage/v2/elasticsearch/tracestore/to_dbmodel_test.go b/internal/storage/v2/elasticsearch/tracestore/to_dbmodel_test.go

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@`
`42`	`42`	`{`
`43`	`43`	`"key": "peer.ipv4",`
`44`	`44`	`"type": "int64",`
`45`		`- "value": "23456"`
	`45`	`+ "value": 23456`
`46`	`46`	`},`
`47`	`47`	`{`
`48`	`48`	`"key": "blob",`
`@@ -52,12 +52,12 @@`
`52`	`52`	`{`
`53`	`53`	`"key": "temperature",`
`54`	`54`	`"type": "float64",`
`55`		`- "value": "72.5"`
	`55`	`+ "value": 72.5`
`56`	`56`	`},`
`57`	`57`	`{`
`58`	`58`	`"key": "error",`
`59`	`59`	`"type": "bool",`
`60`		`- "value": "true"`
	`60`	`+ "value": true`
`61`	`61`	`}`
`62`	`62`	`],`
`63`	`63`	`"logs": [`