Do not truncate tags (jaegertracing#1970)

* Do not truncate tags in Elasticserch Signed-off-by: Pavol Loffay <[email protected]> * Add AsString to model and rename original to Lossy Signed-off-by: Pavol Loffay <[email protected]> * Review comments Signed-off-by: Pavol Loffay <[email protected]>
ilyamor · Dec 11, 2019 · 071fc85 · 071fc85
1 parent 58d3f5c
commit 071fc85
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 18 deletions.
diff --git a/cmd/collector/app/sanitizer/utf8_sanitizer.go b/cmd/collector/app/sanitizer/utf8_sanitizer.go
@@ -72,7 +72,7 @@ func (s *utf8Sanitizer) logSpan(span *model.Span, message string, field zapcore.
 func sanitizeKV(keyValues model.KeyValues) {
 	for i, kv := range keyValues {
 		if !utf8.ValidString(kv.Key) {
-			keyValues[i] = model.Binary(invalidTagKey, []byte(fmt.Sprintf("%s:%s", kv.Key, kv.AsString())))
+			keyValues[i] = model.Binary(invalidTagKey, []byte(fmt.Sprintf("%s:%s", kv.Key, kv.AsStringLossy())))
 		} else if kv.VType == model.StringType && !utf8.ValidString(kv.VStr) {
 			keyValues[i] = model.Binary(kv.Key, []byte(kv.VStr))
 		}

diff --git a/model/keyvalue.go b/model/keyvalue.go
@@ -120,8 +120,17 @@ func (kv *KeyValue) Value() interface{} {
 	}
 }
 
-// AsString returns a potentially lossy string representation of the value.
+// AsStringLossy returns a potentially lossy string representation of the value.
+func (kv *KeyValue) AsStringLossy() string {
+	return kv.asString(true)
+}
+
+// AsString returns a string representation of the value.
 func (kv *KeyValue) AsString() string {
+	return kv.asString(false)
+}
+
+func (kv *KeyValue) asString(truncate bool) string {
 	switch kv.VType {
 	case StringType:
 		return kv.VStr
@@ -135,7 +144,7 @@ func (kv *KeyValue) AsString() string {
 	case Float64Type:
 		return strconv.FormatFloat(kv.Float64(), 'g', 10, 64)
 	case BinaryType:
-		if len(kv.VBinary) > 256 {
+		if truncate && len(kv.VBinary) > 256 {
 			return hex.EncodeToString(kv.VBinary[0:256]) + "..."
 		}
 		return hex.EncodeToString(kv.VBinary)

diff --git a/model/keyvalue_test.go b/model/keyvalue_test.go
@@ -112,11 +112,13 @@ func TestKeyValueAsStringAndValue(t *testing.T) {
 	Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues
 	Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues
 	Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues `
-	expectedBinaryStr := `42656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565730a0942656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565730a0942656e6465722042656e64696e6720526f647269677565732042656e64...`
+	expectedBinaryStr := `42656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565730a0942656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565730a0942656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565730a0942656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565730a0942656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f6472696775657320`
+	expectedBinaryStrLossy := `42656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565730a0942656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565732042656e6465722042656e64696e6720526f647269677565730a0942656e6465722042656e64696e6720526f647269677565732042656e64...`
 	testCases := []struct {
-		kv  model.KeyValue
-		str string
-		val interface{}
+		kv       model.KeyValue
+		str      string
+		strLossy string
+		val      interface{}
 	}{
 		{kv: model.String("x", "Bender is great!"), str: "Bender is great!", val: "Bender is great!"},
 		{kv: model.Bool("x", false), str: "false", val: false},
@@ -126,11 +128,16 @@ func TestKeyValueAsStringAndValue(t *testing.T) {
 		{kv: model.Float64("x", 3.14159265359), str: "3.141592654", val: float64(3.14159265359)},
 		{kv: model.Binary("x", []byte("Bender")), str: "42656e646572", val: []byte("Bender")},
 		{kv: model.Binary("x", []byte(longString)), str: expectedBinaryStr, val: []byte(longString)},
+		{kv: model.Binary("x", []byte(longString)), strLossy: expectedBinaryStrLossy, val: []byte(longString)},
 	}
 	for _, tt := range testCases {
 		testCase := tt // capture loop var
 		t.Run(testCase.str, func(t *testing.T) {
-			assert.Equal(t, testCase.str, testCase.kv.AsString())
+			if testCase.strLossy != "" {
+				assert.Equal(t, testCase.strLossy, testCase.kv.AsStringLossy())
+			} else {
+				assert.Equal(t, testCase.str, testCase.kv.AsString())
+			}
 			assert.Equal(t, testCase.val, testCase.kv.Value())
 		})
 	}

diff --git a/plugin/storage/es/spanstore/dbmodel/from_domain.go b/plugin/storage/es/spanstore/dbmodel/from_domain.go
@@ -95,11 +95,7 @@ func (fd FromDomain) convertKeyValuesString(keyValues model.KeyValues) ([]KeyVal
 			}
 			tagsMap[strings.Replace(kv.Key, ".", fd.tagDotReplacement, -1)] = kv.Value()
 		} else {
-			kvs = append(kvs, KeyValue{
-				Key:   kv.Key,
-				Type:  ValueType(strings.ToLower(kv.VType.String())),
-				Value: kv.AsString(),
-			})
+			kvs = append(kvs, convertKeyValue(kv))
 		}
 	}
 	if kvs == nil {
@@ -113,11 +109,7 @@ func (fd FromDomain) convertLogs(logs []model.Log) []Log {
 	for i, log := range logs {
 		var kvs []KeyValue
 		for _, kv := range log.Fields {
-			kvs = append(kvs, KeyValue{
-				Key:   kv.Key,
-				Type:  ValueType(strings.ToLower(kv.VType.String())),
-				Value: kv.AsString(),
-			})
+			kvs = append(kvs, convertKeyValue(kv))
 		}
 		out[i] = Log{
 			Timestamp: model.TimeAsEpochMicroseconds(log.Timestamp),
@@ -135,3 +127,11 @@ func (fd FromDomain) convertProcess(process *model.Process) Process {
 		Tag:         tagsMap,
 	}
 }
+
+func convertKeyValue(kv model.KeyValue) KeyValue {
+	return KeyValue{
+		Key:   kv.Key,
+		Type:  ValueType(strings.ToLower(kv.VType.String())),
+		Value: kv.AsString(),
+	}
+}
diff --git a/plugin/storage/es/spanstore/dbmodel/from_domain_test.go b/plugin/storage/es/spanstore/dbmodel/from_domain_test.go
@@ -17,6 +17,7 @@ package dbmodel
 
 import (
 	"bytes"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -106,3 +107,52 @@ func TestTagMap(t *testing.T) {
 	assert.Equal(t, tagsMap, dbSpan.Tag)
 	assert.Equal(t, tagsMap, dbSpan.Process.Tag)
 }
+
+func TestConvertKeyValueValue(t *testing.T) {
+	longString := `Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues
+	Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues
+	Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues
+	Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues
+	Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues Bender Bending Rodrigues `
+	key := "key"
+	tests := []struct {
+		kv       model.KeyValue
+		expected KeyValue
+	}{
+		{
+			kv:       model.Bool(key, true),
+			expected: KeyValue{Key: key, Value: "true", Type: "bool"},
+		},
+		{
+			kv:       model.Bool(key, false),
+			expected: KeyValue{Key: key, Value: "false", Type: "bool"},
+		},
+		{
+			kv:       model.Int64(key, int64(1499)),
+			expected: KeyValue{Key: key, Value: "1499", Type: "int64"},
+		},
+		{
+			kv:       model.Float64(key, float64(15.66)),
+			expected: KeyValue{Key: key, Value: "15.66", Type: "float64"},
+		},
+		{
+			kv:       model.String(key, longString),
+			expected: KeyValue{Key: key, Value: longString, Type: "string"},
+		},
+		{
+			kv:       model.Binary(key, []byte(longString)),
+			expected: KeyValue{Key: key, Value: hex.EncodeToString([]byte(longString)), Type: "binary"},
+		},
+		{
+			kv:       model.KeyValue{VType: 1500, Key: key},
+			expected: KeyValue{Key: key, Value: "unknown type 1500", Type: "1500"},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("%s:%s", test.expected.Type, test.expected.Key), func(t *testing.T) {
+			actual := convertKeyValue(test.kv)
+			assert.Equal(t, test.expected, actual)
+		})
+	}
+}