Skip to content

Commit 1eac31e

Browse files
Manik2708Manik2708
authored
[ES][v1] Change the DB Tag value from string to any type (#6998)
## Which problem is this PR solving? - Discussed in #6994 ## Description of the changes - We shouldn't be storing values as string in ES rather they should be stored raw but due to some unkown reason they were being stored as string so now backward compatibility should be considered for the old data and new data should be stored as raw ## How was this change tested? - Unit and E2E 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 <[email protected]>
1 parent d12f1f9 commit 1eac31e

File tree

6 files changed

+190
-71
lines changed

6 files changed

+190
-71
lines changed

‎internal/storage/v1/elasticsearch/spanstore/fixtures/es_01.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,17 @@
3232
{
3333
"key": "peer.ipv4",
3434
"type": "int64",
35-
"value": "23456"
35+
"value": 23456
3636
},
3737
{
3838
"key": "error",
3939
"type": "bool",
40-
"value": "true"
40+
"value": true
4141
},
4242
{
4343
"key": "temperature",
4444
"type": "float64",
45-
"value": "72.5"
45+
"value": 72.5
4646
},
4747
{
4848
"key": "blob",
@@ -57,7 +57,7 @@
5757
{
5858
"key": "event",
5959
"type": "int64",
60-
"value": "123415"
60+
"value": 123415
6161
}
6262
]
6363
},
@@ -78,12 +78,12 @@
7878
{
7979
"key": "peer.ipv4",
8080
"type": "int64",
81-
"value": "23456"
81+
"value": 23456
8282
},
8383
{
8484
"key": "error",
8585
"type": "bool",
86-
"value": "true"
86+
"value": true
8787
}
8888
]
8989
}

‎internal/storage/v1/elasticsearch/spanstore/from_domain.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,14 @@ func (fd FromDomain) convertProcess(process *model.Process) dbmodel.Process {
119119
}
120120

121121
func convertKeyValue(kv model.KeyValue) dbmodel.KeyValue {
122-
return dbmodel.KeyValue{
123-
Key: kv.Key,
124-
Type: dbmodel.ValueType(strings.ToLower(kv.VType.String())),
125-
Value: kv.AsString(),
122+
outKv := dbmodel.KeyValue{
123+
Key: kv.Key,
124+
Type: dbmodel.ValueType(strings.ToLower(kv.VType.String())),
126125
}
126+
if kv.GetVType() == model.BinaryType {
127+
outKv.Value = kv.AsString()
128+
} else {
129+
outKv.Value = kv.Value()
130+
}
131+
return outKv
127132
}

‎internal/storage/v1/elasticsearch/spanstore/from_domain_test.go

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,9 @@ func TestFromDomainEmbedProcess(t *testing.T) {
3232
converter := NewFromDomain(false, nil, ":")
3333
embeddedSpan := converter.FromDomainEmbedProcess(&span)
3434

35-
var expectedSpan dbmodel.Span
36-
require.NoError(t, json.Unmarshal(jsonStr, &expectedSpan))
37-
3835
testJSONEncoding(t, i, jsonStr, embeddedSpan)
3936

40-
CompareJSONSpans(t, &expectedSpan, embeddedSpan)
37+
CompareJSONSpans(t, jsonStr, embeddedSpan)
4138
})
4239
}
4340
}
@@ -112,19 +109,19 @@ func TestConvertKeyValueValue(t *testing.T) {
112109
}{
113110
{
114111
kv: model.Bool(key, true),
115-
expected: dbmodel.KeyValue{Key: key, Value: "true", Type: "bool"},
112+
expected: dbmodel.KeyValue{Key: key, Value: true, Type: "bool"},
116113
},
117114
{
118115
kv: model.Bool(key, false),
119-
expected: dbmodel.KeyValue{Key: key, Value: "false", Type: "bool"},
116+
expected: dbmodel.KeyValue{Key: key, Value: false, Type: "bool"},
120117
},
121118
{
122119
kv: model.Int64(key, int64(1499)),
123-
expected: dbmodel.KeyValue{Key: key, Value: "1499", Type: "int64"},
120+
expected: dbmodel.KeyValue{Key: key, Value: int64(1499), Type: "int64"},
124121
},
125122
{
126123
kv: model.Float64(key, float64(15.66)),
127-
expected: dbmodel.KeyValue{Key: key, Value: "15.66", Type: "float64"},
124+
expected: dbmodel.KeyValue{Key: key, Value: 15.66, Type: "float64"},
128125
},
129126
{
130127
kv: model.String(key, longString),
@@ -134,10 +131,6 @@ func TestConvertKeyValueValue(t *testing.T) {
134131
kv: model.Binary(key, []byte(longString)),
135132
expected: dbmodel.KeyValue{Key: key, Value: hex.EncodeToString([]byte(longString)), Type: "binary"},
136133
},
137-
{
138-
kv: model.KeyValue{VType: 1500, Key: key},
139-
expected: dbmodel.KeyValue{Key: key, Value: "unknown type 1500", Type: "1500"},
140-
},
141134
}
142135

143136
for _, test := range tests {

‎internal/storage/v1/elasticsearch/spanstore/json_span_compare_test.go

Lines changed: 7 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
package spanstore
66

77
import (
8+
"bytes"
89
"encoding/json"
9-
"sort"
1010
"testing"
1111

1212
"github.com/kr/pretty"
@@ -16,11 +16,12 @@ import (
1616
"github.com/jaegertracing/jaeger/internal/storage/elasticsearch/dbmodel"
1717
)
1818

19-
func CompareJSONSpans(t *testing.T, expected *dbmodel.Span, actual *dbmodel.Span) {
20-
sortJSONSpan(expected)
21-
sortJSONSpan(actual)
22-
23-
if !assert.Equal(t, expected, actual) {
19+
func CompareJSONSpans(t *testing.T, expected []byte, actual *dbmodel.Span) {
20+
buf := &bytes.Buffer{}
21+
enc := json.NewEncoder(buf)
22+
enc.SetIndent("", " ")
23+
require.NoError(t, enc.Encode(actual))
24+
if !assert.Equal(t, string(expected), buf.String()) {
2425
for _, err := range pretty.Diff(expected, actual) {
2526
t.Log(err)
2627
}
@@ -29,36 +30,3 @@ func CompareJSONSpans(t *testing.T, expected *dbmodel.Span, actual *dbmodel.Span
2930
t.Logf("Actual trace: %s", string(out))
3031
}
3132
}
32-
33-
func sortJSONSpan(span *dbmodel.Span) {
34-
sortJSONTags(span.Tags)
35-
sortJSONLogs(span.Logs)
36-
sortJSONProcess(span.Process)
37-
}
38-
39-
type JSONTagByKey []dbmodel.KeyValue
40-
41-
func (t JSONTagByKey) Len() int { return len(t) }
42-
func (t JSONTagByKey) Swap(i, j int) { t[i], t[j] = t[j], t[i] }
43-
func (t JSONTagByKey) Less(i, j int) bool { return t[i].Key < t[j].Key }
44-
45-
func sortJSONTags(tags []dbmodel.KeyValue) {
46-
sort.Sort(JSONTagByKey(tags))
47-
}
48-
49-
type JSONLogByTimestamp []dbmodel.Log
50-
51-
func (t JSONLogByTimestamp) Len() int { return len(t) }
52-
func (t JSONLogByTimestamp) Swap(i, j int) { t[i], t[j] = t[j], t[i] }
53-
func (t JSONLogByTimestamp) Less(i, j int) bool { return t[i].Timestamp < t[j].Timestamp }
54-
55-
func sortJSONLogs(logs []dbmodel.Log) {
56-
sort.Sort(JSONLogByTimestamp(logs))
57-
for i := range logs {
58-
sortJSONTags(logs[i].Fields)
59-
}
60-
}
61-
62-
func sortJSONProcess(process dbmodel.Process) {
63-
sortJSONTags(process.Tags)
64-
}

‎internal/storage/v1/elasticsearch/spanstore/to_domain.go

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,28 @@ func (td ToDomain) convertTagField(k string, v any) (model.KeyValue, error) {
173173

174174
// convertKeyValue expects the Value field to be string, because it only works
175175
// as a reverse transformation after FromDomain() for ElasticSearch model.
176-
func (ToDomain) convertKeyValue(tag *dbmodel.KeyValue) (model.KeyValue, error) {
176+
func (td ToDomain) convertKeyValue(tag *dbmodel.KeyValue) (model.KeyValue, error) {
177177
if tag.Value == nil {
178178
return model.KeyValue{}, fmt.Errorf("invalid nil Value in %v", tag)
179179
}
180180
tagValue, ok := tag.Value.(string)
181181
if !ok {
182-
return model.KeyValue{}, fmt.Errorf("non-string Value of type %t in %v", tag.Value, tag)
182+
switch tag.Type {
183+
case dbmodel.Int64Type, dbmodel.Float64Type:
184+
kv, err := td.fromDBNumber(tag)
185+
if err != nil {
186+
return model.KeyValue{}, err
187+
}
188+
return kv, nil
189+
case dbmodel.BoolType:
190+
if boolVal, ok := tag.Value.(bool); ok {
191+
return model.Bool(tag.Key, boolVal), nil
192+
}
193+
return model.KeyValue{}, invalidValueErr(tag)
194+
// string and binary values should always be of string type
195+
default:
196+
return model.KeyValue{}, invalidValueErr(tag)
197+
}
183198
}
184199
switch tag.Type {
185200
case dbmodel.StringType:
@@ -212,6 +227,42 @@ func (ToDomain) convertKeyValue(tag *dbmodel.KeyValue) (model.KeyValue, error) {
212227
return model.KeyValue{}, fmt.Errorf("not a valid ValueType string %s", string(tag.Type))
213228
}
214229

230+
func (ToDomain) fromDBNumber(kv *dbmodel.KeyValue) (model.KeyValue, error) {
231+
if kv.Type == dbmodel.Int64Type {
232+
switch v := kv.Value.(type) {
233+
case int64:
234+
return model.Int64(kv.Key, v), nil
235+
// This case is very much possible as JSON converts every number to float64
236+
case float64:
237+
return model.Int64(kv.Key, int64(v)), nil
238+
case json.Number:
239+
n, err := v.Int64()
240+
if err == nil {
241+
return model.Int64(kv.Key, n), nil
242+
}
243+
default:
244+
return model.KeyValue{}, invalidValueErr(kv)
245+
}
246+
} else if kv.Type == dbmodel.Float64Type {
247+
switch v := kv.Value.(type) {
248+
case float64:
249+
return model.Float64(kv.Key, v), nil
250+
case json.Number:
251+
n, err := v.Float64()
252+
if err == nil {
253+
return model.Float64(kv.Key, n), nil
254+
}
255+
default:
256+
return model.KeyValue{}, invalidValueErr(kv)
257+
}
258+
}
259+
return model.KeyValue{}, fmt.Errorf("not a valid number ValueType %s", string(kv.Type))
260+
}
261+
262+
func invalidValueErr(kv *dbmodel.KeyValue) error {
263+
return fmt.Errorf("invalid %s type in %+v", string(kv.Type), kv.Value)
264+
}
265+
215266
func (td ToDomain) convertLogs(logs []dbmodel.Log) ([]model.Log, error) {
216267
retMe := make([]model.Log, len(logs))
217268
for i, l := range logs {

‎internal/storage/v1/elasticsearch/spanstore/to_domain_test.go

Lines changed: 110 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,13 @@ func TestFailureBadLogs(t *testing.T) {
163163

164164
func TestRevertKeyValueOfType(t *testing.T) {
165165
tests := []struct {
166-
kv *dbmodel.KeyValue
167-
err string
166+
name string
167+
kv *dbmodel.KeyValue
168+
err string
169+
outKv model.KeyValue
168170
}{
169171
{
172+
name: "not a valid ValueType string",
170173
kv: &dbmodel.KeyValue{
171174
Key: "sneh",
172175
Type: "badType",
@@ -175,22 +178,121 @@ func TestRevertKeyValueOfType(t *testing.T) {
175178
err: "not a valid ValueType string",
176179
},
177180
{
178-
kv: &dbmodel.KeyValue{},
179-
err: "invalid nil Value",
181+
name: "invalid nil Value",
182+
kv: &dbmodel.KeyValue{},
183+
err: "invalid nil Value",
180184
},
181185
{
186+
name: "right int value",
182187
kv: &dbmodel.KeyValue{
188+
Key: "int-val",
189+
Type: dbmodel.Int64Type,
190+
Value: int64(123),
191+
},
192+
outKv: model.KeyValue{
193+
Key: "int-val",
194+
VInt64: 123,
195+
VType: 2,
196+
},
197+
},
198+
{
199+
name: "right int float value",
200+
kv: &dbmodel.KeyValue{
201+
Key: "int-val",
202+
Type: dbmodel.Int64Type,
203+
Value: float64(123),
204+
},
205+
outKv: model.KeyValue{
206+
Key: "int-val",
207+
VInt64: 123,
208+
VType: 2,
209+
},
210+
},
211+
{
212+
name: "right int json number",
213+
kv: &dbmodel.KeyValue{
214+
Key: "int-val",
215+
Type: dbmodel.Int64Type,
216+
Value: json.Number("123"),
217+
},
218+
outKv: model.KeyValue{
219+
Key: "int-val",
220+
VInt64: 123,
221+
VType: 2,
222+
},
223+
},
224+
{
225+
name: "right float value",
226+
kv: &dbmodel.KeyValue{
227+
Key: "float-val",
228+
Type: dbmodel.Float64Type,
229+
Value: 123.4,
230+
},
231+
outKv: model.KeyValue{
232+
Key: "float-val",
233+
VFloat64: 123.4,
234+
VType: 3,
235+
},
236+
},
237+
{
238+
name: "right float json number",
239+
kv: &dbmodel.KeyValue{
240+
Key: "float-val",
241+
Type: dbmodel.Float64Type,
242+
Value: json.Number("123.4"),
243+
},
244+
outKv: model.KeyValue{
245+
Key: "float-val",
246+
VFloat64: 123.4,
247+
VType: 3,
248+
},
249+
},
250+
{
251+
name: "wrong int64 value",
252+
kv: &dbmodel.KeyValue{
253+
Key: "int-val",
254+
Type: dbmodel.Int64Type,
255+
Value: true,
256+
},
257+
err: "invalid int64 type in true",
258+
},
259+
{
260+
name: "wrong float64 value",
261+
kv: &dbmodel.KeyValue{
262+
Key: "float-val",
263+
Type: dbmodel.Float64Type,
264+
Value: true,
265+
},
266+
err: "invalid float64 type in true",
267+
},
268+
{
269+
name: "wrong bool value",
270+
kv: &dbmodel.KeyValue{
271+
Key: "bool-val",
272+
Type: dbmodel.BoolType,
273+
Value: 1.23,
274+
},
275+
err: "invalid bool type in 1.23",
276+
},
277+
{
278+
name: "wrong string value",
279+
kv: &dbmodel.KeyValue{
280+
Key: "string-val",
281+
Type: dbmodel.StringType,
183282
Value: 123,
184283
},
185-
err: "non-string Value of type",
284+
err: "invalid string type in 123",
186285
},
187286
}
188287
td := ToDomain{}
189288
for _, test := range tests {
190-
t.Run(test.err, func(t *testing.T) {
289+
t.Run(test.name, func(t *testing.T) {
191290
tag := test.kv
192-
_, err := td.convertKeyValue(tag)
193-
assert.ErrorContains(t, err, test.err)
291+
out, err := td.convertKeyValue(tag)
292+
if test.err != "" {
293+
require.ErrorContains(t, err, test.err)
294+
}
295+
assert.Equal(t, test.outKv, out)
194296
})
195297
}
196298
}

0 commit comments

Comments
 (0)