Skip to content

Commit c3f8f21

Browse files
Manik2708Manik2708
authored
[ES] Make NestedTags and ElevatedTags distinction at CoreSpanReader level (#7067)
## Which problem is this PR solving? - Fixes a part of: #7034 ## Description of the changes - Make `NestedTags` and `ElevatedTags` distinction at `CoreSpanReader` level and a follow-up PR for #6946 ## How was this change tested? - Unit And Integration 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 49cf000 commit c3f8f21

File tree

5 files changed

+156
-129
lines changed

5 files changed

+156
-129
lines changed

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

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,12 @@ func (s *SpanReader) collectSpans(esSpansRaw []*elastic.SearchHit) ([]dbmodel.Sp
249249
spans := make([]dbmodel.Span, len(esSpansRaw))
250250

251251
for i, esSpanRaw := range esSpansRaw {
252-
jsonSpan, err := s.unmarshalJSONSpan(esSpanRaw)
252+
dbSpan, err := s.unmarshalJSONSpan(esSpanRaw)
253253
if err != nil {
254254
return nil, fmt.Errorf("marshalling JSON to span object failed: %w", err)
255255
}
256-
spans[i] = jsonSpan
256+
s.mergeAllNestedAndElevatedTagsOfSpan(&dbSpan)
257+
spans[i] = dbSpan
257258
}
258259
return spans, nil
259260
}
@@ -677,6 +678,71 @@ func (*SpanReader) buildObjectQuery(field string, k string, v string) elastic.Qu
677678
return elastic.NewBoolQuery().Must(keyQuery)
678679
}
679680

681+
func (s *SpanReader) mergeAllNestedAndElevatedTagsOfSpan(span *dbmodel.Span) {
682+
processTags := s.mergeNestedAndElevatedTags(span.Process.Tags, span.Process.Tag)
683+
span.Process.Tags = processTags
684+
spanTags := s.mergeNestedAndElevatedTags(span.Tags, span.Tag)
685+
span.Tags = spanTags
686+
}
687+
688+
func (s *SpanReader) mergeNestedAndElevatedTags(nestedTags []dbmodel.KeyValue, elevatedTags map[string]any) []dbmodel.KeyValue {
689+
mergedTags := make([]dbmodel.KeyValue, 0, len(nestedTags)+len(elevatedTags))
690+
mergedTags = append(mergedTags, nestedTags...)
691+
for k, v := range elevatedTags {
692+
kv := s.convertTagField(k, v)
693+
mergedTags = append(mergedTags, kv)
694+
delete(elevatedTags, k)
695+
}
696+
return mergedTags
697+
}
698+
699+
func (s *SpanReader) convertTagField(k string, v any) dbmodel.KeyValue {
700+
dKey := s.dotReplacer.ReplaceDotReplacement(k)
701+
kv := dbmodel.KeyValue{
702+
Key: dKey,
703+
Value: v,
704+
}
705+
switch val := v.(type) {
706+
case int64:
707+
kv.Type = dbmodel.Int64Type
708+
case float64:
709+
kv.Type = dbmodel.Float64Type
710+
case bool:
711+
kv.Type = dbmodel.BoolType
712+
case string:
713+
kv.Type = dbmodel.StringType
714+
// the binary is never returned, ES returns it as string with base64 encoding
715+
case []byte:
716+
kv.Type = dbmodel.BinaryType
717+
// in spans are decoded using json.UseNumber() to preserve the type
718+
// however note that float(1) will be parsed as int as ES does not store decimal point
719+
case json.Number:
720+
n, err := val.Int64()
721+
if err == nil {
722+
kv.Value = n
723+
kv.Type = dbmodel.Int64Type
724+
} else {
725+
f, err := val.Float64()
726+
if err != nil {
727+
return dbmodel.KeyValue{
728+
Key: dKey,
729+
Value: fmt.Sprintf("invalid tag type in %+v: %s", v, err.Error()),
730+
Type: dbmodel.StringType,
731+
}
732+
}
733+
kv.Value = f
734+
kv.Type = dbmodel.Float64Type
735+
}
736+
default:
737+
return dbmodel.KeyValue{
738+
Key: dKey,
739+
Value: fmt.Sprintf("invalid tag type in %+v", v),
740+
Type: dbmodel.StringType,
741+
}
742+
}
743+
return kv
744+
}
745+
680746
func logErrorToSpan(span trace.Span, err error) {
681747
span.RecordError(err)
682748
span.SetStatus(codes.Error, err.Error())

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

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,10 +332,26 @@ func TestSpanReader_multiRead_followUp_query(t *testing.T) {
332332
traceID1 := dbmodel.TraceID(testingTraceId + "1")
333333
traceID2 := dbmodel.TraceID(testingTraceId + "2")
334334
date := time.Date(2019, 10, 10, 5, 0, 0, 0, time.UTC)
335-
spanID1 := dbmodel.Span{SpanID: "0", TraceID: traceID1, StartTime: model.TimeAsEpochMicroseconds(date)}
335+
spanID1 := dbmodel.Span{
336+
SpanID: "0",
337+
TraceID: traceID1,
338+
StartTime: model.TimeAsEpochMicroseconds(date),
339+
Tags: []dbmodel.KeyValue{},
340+
Process: dbmodel.Process{
341+
Tags: []dbmodel.KeyValue{},
342+
},
343+
}
336344
spanBytesID1, err := json.Marshal(spanID1)
337345
require.NoError(t, err)
338-
spanID2 := dbmodel.Span{SpanID: "0", TraceID: traceID2, StartTime: model.TimeAsEpochMicroseconds(date)}
346+
spanID2 := dbmodel.Span{
347+
SpanID: "0",
348+
TraceID: traceID2,
349+
StartTime: model.TimeAsEpochMicroseconds(date),
350+
Tags: []dbmodel.KeyValue{},
351+
Process: dbmodel.Process{
352+
Tags: []dbmodel.KeyValue{},
353+
},
354+
}
339355
spanBytesID2, err := json.Marshal(spanID2)
340356
require.NoError(t, err)
341357

@@ -403,9 +419,13 @@ func TestSpanReader_multiRead_followUp_query(t *testing.T) {
403419
require.NotNil(t, traces)
404420
require.Len(t, traces, 2)
405421

406-
for _, s := range []dbmodel.Span{spanID1, spanID2} {
407-
found := reflect.DeepEqual(traces[0].Spans[0], s) || reflect.DeepEqual(traces[1].Spans[0], s)
408-
assert.True(t, found, "span was expected to be within one of the traces but was not: %v", s)
422+
for i, s := range []dbmodel.Span{spanID1, spanID2} {
423+
actual := traces[i].Spans[0]
424+
actualData, err := json.Marshal(actual)
425+
require.NoError(t, err)
426+
expectedData, err := json.Marshal(s)
427+
require.NoError(t, err)
428+
assert.Equal(t, string(expectedData), string(actualData))
409429
}
410430
})
411431
}
@@ -1311,3 +1331,59 @@ func TestTerminateAfterNotSet(t *testing.T) {
13111331
require.True(t, ok)
13121332
assert.Equal(t, 99, size)
13131333
}
1334+
1335+
func TestTagsMap(t *testing.T) {
1336+
tests := []struct {
1337+
fieldTags map[string]any
1338+
expected dbmodel.KeyValue
1339+
}{
1340+
{fieldTags: map[string]any{"bool:bool": true}, expected: dbmodel.KeyValue{Key: "bool.bool", Value: true, Type: dbmodel.BoolType}},
1341+
{fieldTags: map[string]any{"int.int": int64(1)}, expected: dbmodel.KeyValue{Key: "int.int", Value: int64(1), Type: dbmodel.Int64Type}},
1342+
{fieldTags: map[string]any{"int:int": int64(2)}, expected: dbmodel.KeyValue{Key: "int.int", Value: int64(2), Type: dbmodel.Int64Type}},
1343+
{fieldTags: map[string]any{"float": float64(1.1)}, expected: dbmodel.KeyValue{Key: "float", Value: float64(1.1), Type: dbmodel.Float64Type}},
1344+
{fieldTags: map[string]any{"float": float64(123)}, expected: dbmodel.KeyValue{Key: "float", Value: float64(123), Type: dbmodel.Float64Type}},
1345+
{fieldTags: map[string]any{"float": float64(123.0)}, expected: dbmodel.KeyValue{Key: "float", Value: float64(123.0), Type: dbmodel.Float64Type}},
1346+
{fieldTags: map[string]any{"float:float": float64(123)}, expected: dbmodel.KeyValue{Key: "float.float", Value: float64(123), Type: dbmodel.Float64Type}},
1347+
{fieldTags: map[string]any{"json_number:int": json.Number("123")}, expected: dbmodel.KeyValue{Key: "json_number.int", Value: int64(123), Type: dbmodel.Int64Type}},
1348+
{fieldTags: map[string]any{"json_number:float": json.Number("123.0")}, expected: dbmodel.KeyValue{Key: "json_number.float", Value: float64(123.0), Type: dbmodel.Float64Type}},
1349+
{fieldTags: map[string]any{"json_number:err": json.Number("foo")}, expected: dbmodel.KeyValue{Key: "json_number.err", Value: "invalid tag type in foo: strconv.ParseFloat: parsing \"foo\": invalid syntax", Type: dbmodel.StringType}},
1350+
{fieldTags: map[string]any{"str": "foo"}, expected: dbmodel.KeyValue{Key: "str", Value: "foo", Type: dbmodel.StringType}},
1351+
{fieldTags: map[string]any{"str:str": "foo"}, expected: dbmodel.KeyValue{Key: "str.str", Value: "foo", Type: dbmodel.StringType}},
1352+
{fieldTags: map[string]any{"binary": []byte("foo")}, expected: dbmodel.KeyValue{Key: "binary", Value: []byte("foo"), Type: dbmodel.BinaryType}},
1353+
{fieldTags: map[string]any{"binary:binary": []byte("foo")}, expected: dbmodel.KeyValue{Key: "binary.binary", Value: []byte("foo"), Type: dbmodel.BinaryType}},
1354+
{fieldTags: map[string]any{"unsupported": struct{}{}}, expected: dbmodel.KeyValue{Key: "unsupported", Value: fmt.Sprintf("invalid tag type in %+v", struct{}{}), Type: dbmodel.StringType}},
1355+
}
1356+
reader := NewSpanReader(SpanReaderParams{
1357+
TagDotReplacement: ":",
1358+
Logger: zap.NewNop(),
1359+
})
1360+
for i, test := range tests {
1361+
t.Run(fmt.Sprintf("%d, %s", i, test.fieldTags), func(t *testing.T) {
1362+
tags := []dbmodel.KeyValue{
1363+
{
1364+
Key: "testing-key",
1365+
Type: dbmodel.StringType,
1366+
Value: "testing-value",
1367+
},
1368+
}
1369+
spanTags := make(map[string]any)
1370+
for k, v := range test.fieldTags {
1371+
spanTags[k] = v
1372+
}
1373+
span := &dbmodel.Span{
1374+
Process: dbmodel.Process{
1375+
Tag: test.fieldTags,
1376+
Tags: tags,
1377+
},
1378+
Tag: spanTags,
1379+
Tags: tags,
1380+
}
1381+
reader.mergeAllNestedAndElevatedTagsOfSpan(span)
1382+
tags = append(tags, test.expected)
1383+
assert.Empty(t, span.Tag)
1384+
assert.Empty(t, span.Process.Tag)
1385+
assert.Equal(t, tags, span.Tags)
1386+
assert.Equal(t, tags, span.Process.Tags)
1387+
})
1388+
}
1389+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ type SpanReaderV1 struct {
2424
func NewSpanReaderV1(p SpanReaderParams) *SpanReaderV1 {
2525
return &SpanReaderV1{
2626
spanReader: NewSpanReader(p),
27-
spanConverter: NewToDomain(p.TagDotReplacement),
27+
spanConverter: NewToDomain(),
2828
}
2929
}
3030

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

Lines changed: 3 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,12 @@ import (
1515
)
1616

1717
// NewToDomain creates ToDomain
18-
func NewToDomain(tagDotReplacement string) ToDomain {
19-
return ToDomain{dotReplacer: dbmodel.NewDotReplacer(tagDotReplacement)}
18+
func NewToDomain() ToDomain {
19+
return ToDomain{}
2020
}
2121

2222
// ToDomain is used to convert Span to model.Span
23-
type ToDomain struct {
24-
dotReplacer dbmodel.DotReplacer
25-
}
23+
type ToDomain struct{}
2624

2725
// SpanToDomain converts db span into model Span
2826
func (td ToDomain) SpanToDomain(dbSpan *dbmodel.Span) (*model.Span, error) {
@@ -59,13 +57,6 @@ func (td ToDomain) SpanToDomain(dbSpan *dbmodel.Span) (*model.Span, error) {
5957
}
6058
refs = model.MaybeAddParentSpanID(traceID, parentSpanID, refs)
6159
}
62-
63-
fieldTags, err := td.convertTagFields(dbSpan.Tag)
64-
if err != nil {
65-
return nil, err
66-
}
67-
tags = append(tags, fieldTags...)
68-
6960
span := &model.Span{
7061
TraceID: traceID,
7162
SpanID: model.NewSpanID(uint64(spanIDInt)),
@@ -126,51 +117,6 @@ func (td ToDomain) convertKeyValues(tags []dbmodel.KeyValue) ([]model.KeyValue,
126117
return retMe, nil
127118
}
128119

129-
func (td ToDomain) convertTagFields(tagsMap map[string]any) ([]model.KeyValue, error) {
130-
kvs := make([]model.KeyValue, len(tagsMap))
131-
i := 0
132-
for k, v := range tagsMap {
133-
tag, err := td.convertTagField(k, v)
134-
if err != nil {
135-
return nil, err
136-
}
137-
kvs[i] = tag
138-
i++
139-
}
140-
return kvs, nil
141-
}
142-
143-
func (td ToDomain) convertTagField(k string, v any) (model.KeyValue, error) {
144-
dKey := td.dotReplacer.ReplaceDotReplacement(k)
145-
switch val := v.(type) {
146-
case int64:
147-
return model.Int64(dKey, val), nil
148-
case float64:
149-
return model.Float64(dKey, val), nil
150-
case bool:
151-
return model.Bool(dKey, val), nil
152-
case string:
153-
return model.String(dKey, val), nil
154-
// the binary is never returned, ES returns it as string with base64 encoding
155-
case []byte:
156-
return model.Binary(dKey, val), nil
157-
// in spans are decoded using json.UseNumber() to preserve the type
158-
// however note that float(1) will be parsed as int as ES does not store decimal point
159-
case json.Number:
160-
n, err := val.Int64()
161-
if err == nil {
162-
return model.Int64(dKey, n), nil
163-
}
164-
f, err := val.Float64()
165-
if err == nil {
166-
return model.Float64(dKey, f), nil
167-
}
168-
return model.String("", ""), fmt.Errorf("invalid tag type in %+v: %w", v, err)
169-
default:
170-
return model.String("", ""), fmt.Errorf("invalid tag type in %+v", v)
171-
}
172-
}
173-
174120
// convertKeyValue expects the Value field to be string, because it only works
175121
// as a reverse transformation after FromDomain() for ElasticSearch model.
176122
func (td ToDomain) convertKeyValue(tag *dbmodel.KeyValue) (model.KeyValue, error) {
@@ -283,12 +229,6 @@ func (td ToDomain) convertProcess(process dbmodel.Process) (*model.Process, erro
283229
if err != nil {
284230
return nil, err
285231
}
286-
fieldTags, err := td.convertTagFields(process.Tag)
287-
if err != nil {
288-
return nil, err
289-
}
290-
tags = append(tags, fieldTags...)
291-
292232
return &model.Process{
293233
Tags: tags,
294234
ServiceName: process.ServiceName,

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

Lines changed: 3 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package spanstore
77
import (
88
"bytes"
99
"encoding/json"
10-
"errors"
1110
"fmt"
1211
"math"
1312
"os"
@@ -37,7 +36,7 @@ func testToDomain(t *testing.T, testParentSpanID bool) {
3736
span.ParentSpanID = "3"
3837
}
3938

40-
actualSpan, err := NewToDomain(":").SpanToDomain(&span)
39+
actualSpan, err := NewToDomain().SpanToDomain(&span)
4140
require.NoError(t, err)
4241

4342
out := fmt.Sprintf("fixtures/domain_%02d.json", i)
@@ -62,13 +61,13 @@ func loadESSpanFixture(i int) (dbmodel.Span, error) {
6261
}
6362

6463
func failingSpanTransform(t *testing.T, embeddedSpan *dbmodel.Span, errMsg string) {
65-
domainSpan, err := NewToDomain(":").SpanToDomain(embeddedSpan)
64+
domainSpan, err := NewToDomain().SpanToDomain(embeddedSpan)
6665
assert.Nil(t, domainSpan)
6766
require.EqualError(t, err, errMsg)
6867
}
6968

7069
func failingSpanTransformAnyMsg(t *testing.T, embeddedSpan *dbmodel.Span) {
71-
domainSpan, err := NewToDomain(":").SpanToDomain(embeddedSpan)
70+
domainSpan, err := NewToDomain().SpanToDomain(embeddedSpan)
7271
assert.Nil(t, domainSpan)
7372
require.Error(t, err)
7473
}
@@ -374,20 +373,6 @@ func TestFailureBadParentSpanID(t *testing.T) {
374373
failingSpanTransformAnyMsg(t, &badParentSpanIDESSpan)
375374
}
376375

377-
func TestFailureBadSpanFieldTag(t *testing.T) {
378-
badParentSpanIDESSpan, err := loadESSpanFixture(1)
379-
require.NoError(t, err)
380-
badParentSpanIDESSpan.Tag = map[string]any{"foo": struct{}{}}
381-
failingSpanTransformAnyMsg(t, &badParentSpanIDESSpan)
382-
}
383-
384-
func TestFailureBadProcessFieldTag(t *testing.T) {
385-
badParentSpanIDESSpan, err := loadESSpanFixture(1)
386-
require.NoError(t, err)
387-
badParentSpanIDESSpan.Process.Tag = map[string]any{"foo": struct{}{}}
388-
failingSpanTransformAnyMsg(t, &badParentSpanIDESSpan)
389-
}
390-
391376
func CompareModelSpans(t *testing.T, expected *model.Span, actual *model.Span) {
392377
model.SortSpan(expected)
393378
model.SortSpan(actual)
@@ -401,43 +386,3 @@ func CompareModelSpans(t *testing.T, expected *model.Span, actual *model.Span) {
401386
t.Logf("Actual trace: %s", string(out))
402387
}
403388
}
404-
405-
func TestTagsMap(t *testing.T) {
406-
tests := []struct {
407-
fieldTags map[string]any
408-
expected []model.KeyValue
409-
err error
410-
}{
411-
{fieldTags: map[string]any{"bool:bool": true}, expected: []model.KeyValue{model.Bool("bool.bool", true)}},
412-
{fieldTags: map[string]any{"int.int": int64(1)}, expected: []model.KeyValue{model.Int64("int.int", 1)}},
413-
{fieldTags: map[string]any{"int:int": int64(2)}, expected: []model.KeyValue{model.Int64("int.int", 2)}},
414-
{fieldTags: map[string]any{"float": float64(1.1)}, expected: []model.KeyValue{model.Float64("float", 1.1)}},
415-
{fieldTags: map[string]any{"float": float64(123)}, expected: []model.KeyValue{model.Float64("float", float64(123))}},
416-
{fieldTags: map[string]any{"float": float64(123.0)}, expected: []model.KeyValue{model.Float64("float", float64(123.0))}},
417-
{fieldTags: map[string]any{"float:float": float64(123)}, expected: []model.KeyValue{model.Float64("float.float", float64(123))}},
418-
{fieldTags: map[string]any{"json_number:int": json.Number("123")}, expected: []model.KeyValue{model.Int64("json_number.int", 123)}},
419-
{fieldTags: map[string]any{"json_number:float": json.Number("123.0")}, expected: []model.KeyValue{model.Float64("json_number.float", float64(123.0))}},
420-
{fieldTags: map[string]any{"json_number:err": json.Number("foo")}, err: errors.New("invalid tag type in foo: strconv.ParseFloat: parsing \"foo\": invalid syntax")},
421-
{fieldTags: map[string]any{"str": "foo"}, expected: []model.KeyValue{model.String("str", "foo")}},
422-
{fieldTags: map[string]any{"str:str": "foo"}, expected: []model.KeyValue{model.String("str.str", "foo")}},
423-
{fieldTags: map[string]any{"binary": []byte("foo")}, expected: []model.KeyValue{model.Binary("binary", []byte("foo"))}},
424-
{fieldTags: map[string]any{"binary:binary": []byte("foo")}, expected: []model.KeyValue{model.Binary("binary.binary", []byte("foo"))}},
425-
{fieldTags: map[string]any{"unsupported": struct{}{}}, err: fmt.Errorf("invalid tag type in %+v", struct{}{})},
426-
}
427-
converter := NewToDomain(":")
428-
for i, test := range tests {
429-
t.Run(fmt.Sprintf("%d, %s", i, test.fieldTags), func(t *testing.T) {
430-
tags, err := converter.convertTagFields(test.fieldTags)
431-
if err != nil {
432-
fmt.Println(err.Error())
433-
}
434-
if test.err != nil {
435-
assert.Equal(t, test.err.Error(), err.Error())
436-
require.Nil(t, tags)
437-
} else {
438-
require.NoError(t, err)
439-
assert.Equal(t, test.expected, tags)
440-
}
441-
})
442-
}
443-
}

0 commit comments

Comments
 (0)