Skip to content

Commit 4853cc6

Browse files
Manik2708amol-verma-allen
Manik2708
authored and
amol-verma-allen
committed
[ES] Remove pointer signatures from FromDBModel and ToDBModel (jaegertracing#6942)
## Which problem is this PR solving? - Fixes a part of: jaegertracing#6458 ## Description of the changes - `CoreSpanReader/Writer` doesn't deal with pointers and so shouldn't `FromDBModel` and `ToDBModel` ## 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 <[email protected]> Signed-off-by: amol-verma-allen <[email protected]>
1 parent 441aac7 commit 4853cc6

File tree

4 files changed

+53
-85
lines changed

4 files changed

+53
-85
lines changed

‎internal/storage/v2/elasticsearch/tracestore/from_dbmodel.go

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"encoding/hex"
1111
"errors"
1212
"fmt"
13-
"reflect"
1413
"strconv"
1514
"strings"
1615

@@ -22,13 +21,10 @@ import (
2221
"github.com/jaegertracing/jaeger/internal/storage/elasticsearch/dbmodel"
2322
)
2423

25-
var (
26-
blankJaegerProtoSpan = new(dbmodel.Span)
27-
errType = errors.New("invalid type")
28-
)
24+
var errType = errors.New("invalid type")
2925

3026
// FromDBModel converts multiple ES DB Spans to internal traces
31-
func FromDBModel(spans []*dbmodel.Span) (ptrace.Traces, error) {
27+
func FromDBModel(spans []dbmodel.Span) (ptrace.Traces, error) {
3228
traceData := ptrace.NewTraces()
3329
if len(spans) == 0 {
3430
return traceData, nil
@@ -58,21 +54,18 @@ func dbProcessToResource(process dbmodel.Process, resource pcommon.Resource) {
5854
dbTagsToAttributes(tags, attrs)
5955
}
6056

61-
func dbSpansToSpans(dbSpans []*dbmodel.Span, resourceSpans ptrace.ResourceSpansSlice) error {
62-
for _, dbSpan := range dbSpans {
63-
// TODO why is this needed?
64-
if dbSpan == nil || reflect.DeepEqual(dbSpan, blankJaegerProtoSpan) {
65-
continue
66-
}
57+
func dbSpansToSpans(dbSpans []dbmodel.Span, resourceSpans ptrace.ResourceSpansSlice) error {
58+
for i := range dbSpans {
59+
span := &dbSpans[i]
6760
resourceSpan := resourceSpans.AppendEmpty()
68-
dbProcessToResource(dbSpan.Process, resourceSpan.Resource())
61+
dbProcessToResource(span.Process, resourceSpan.Resource())
6962

7063
scopeSpans := resourceSpan.ScopeSpans()
7164
scopeSpan := scopeSpans.AppendEmpty()
72-
dbSpanToScope(dbSpan, scopeSpan)
65+
dbSpanToScope(span, scopeSpan)
7366

7467
// TODO there should be no errors returned from translation from db model
75-
err := dbSpanToSpan(dbSpan, scopeSpan.Spans().AppendEmpty())
68+
err := dbSpanToSpan(span, scopeSpan.Spans().AppendEmpty())
7669
if err != nil {
7770
return err
7871
}

‎internal/storage/v2/elasticsearch/tracestore/from_dbmodel_test.go

Lines changed: 30 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -81,42 +81,17 @@ func TestCodeFromAttr(t *testing.T) {
8181
}
8282

8383
func TestZeroBatchLength(t *testing.T) {
84-
trace, err := FromDBModel([]*dbmodel.Span{})
84+
trace, err := FromDBModel([]dbmodel.Span{})
8585
require.NoError(t, err)
8686
assert.Equal(t, 0, trace.ResourceSpans().Len())
8787
}
8888

8989
func TestEmptySpansAndProcess(t *testing.T) {
90-
trace, err := FromDBModel([]*dbmodel.Span{})
90+
trace, err := FromDBModel([]dbmodel.Span{})
9191
require.NoError(t, err)
9292
assert.Equal(t, 0, trace.ResourceSpans().Len())
9393
}
9494

95-
func Test_setSpansFromDbSpans_EmptyOrNilSpans(t *testing.T) {
96-
tests := []struct {
97-
name string
98-
spans []*dbmodel.Span
99-
}{
100-
{
101-
name: "nil spans",
102-
spans: []*dbmodel.Span{nil},
103-
},
104-
{
105-
name: "empty spans",
106-
spans: []*dbmodel.Span{new(dbmodel.Span)},
107-
},
108-
}
109-
for _, tt := range tests {
110-
t.Run(tt.name, func(t *testing.T) {
111-
traceData := ptrace.NewTraces()
112-
rss := traceData.ResourceSpans()
113-
err := dbSpansToSpans(tt.spans, rss)
114-
require.NoError(t, err)
115-
assert.Equal(t, 0, rss.Len())
116-
})
117-
}
118-
}
119-
12095
func TestGetStatusCodeFromHTTPStatusAttr(t *testing.T) {
12196
tests := []struct {
12297
name string
@@ -361,33 +336,33 @@ func TestSetAttributesFromDbTags(t *testing.T) {
361336
func TestFromDBModel(t *testing.T) {
362337
tests := []struct {
363338
name string
364-
jb []*dbmodel.Span
339+
jb []dbmodel.Span
365340
td ptrace.Traces
366341
}{
367342
{
368343
name: "empty",
369-
jb: []*dbmodel.Span{},
344+
jb: []dbmodel.Span{},
370345
td: ptrace.NewTraces(),
371346
},
372347
{
373348
name: "two-spans-child-parent",
374-
jb: []*dbmodel.Span{
349+
jb: []dbmodel.Span{
375350
generateProtoSpan(),
376351
generateProtoChildSpan(),
377352
},
378353
td: generateTracesWithDifferentResourceTwoSpansChildParent(),
379354
},
380355
{
381356
name: "two-spans-with-follower",
382-
jb: []*dbmodel.Span{
357+
jb: []dbmodel.Span{
383358
generateProtoSpan(),
384359
generateProtoFollowerSpan(),
385360
},
386361
td: generateTracesWithDifferentResourceTwoSpansWithFollower(),
387362
},
388363
{
389364
name: "a-spans-with-two-parent",
390-
jb: []*dbmodel.Span{
365+
jb: []dbmodel.Span{
391366
generateProtoSpan(),
392367
generateProtoFollowerSpan(),
393368
generateProtoTwoParentsSpan(),
@@ -396,7 +371,7 @@ func TestFromDBModel(t *testing.T) {
396371
},
397372
{
398373
name: "no-error-from-server-span-with-4xx-http-code",
399-
jb: []*dbmodel.Span{
374+
jb: []dbmodel.Span{
400375
{
401376
StartTime: model.TimeAsEpochMicroseconds(testSpanStartTime),
402377
Duration: model.DurationAsMicroseconds(testSpanEndTime.Sub(testSpanStartTime)),
@@ -444,31 +419,31 @@ func TestFromDBModelErrors(t *testing.T) {
444419
tests := []struct {
445420
name string
446421
err string
447-
dbSpans []*dbmodel.Span
422+
dbSpans []dbmodel.Span
448423
}{
449424
{
450425
name: "wrong trace-id",
451-
dbSpans: []*dbmodel.Span{{TraceID: dbmodel.TraceID("trace-id")}},
426+
dbSpans: []dbmodel.Span{{TraceID: dbmodel.TraceID("trace-id")}},
452427
err: "encoding/hex: invalid byte: U+0074 't'",
453428
},
454429
{
455430
name: "wrong span-id",
456-
dbSpans: []*dbmodel.Span{{SpanID: dbmodel.SpanID("span-id")}},
431+
dbSpans: []dbmodel.Span{{SpanID: dbmodel.SpanID("span-id")}},
457432
err: "encoding/hex: invalid byte: U+0073 's'",
458433
},
459434
{
460435
name: "wrong parent span-id",
461-
dbSpans: []*dbmodel.Span{{ParentSpanID: dbmodel.SpanID("parent-span-id")}},
436+
dbSpans: []dbmodel.Span{{ParentSpanID: dbmodel.SpanID("parent-span-id")}},
462437
err: "encoding/hex: invalid byte: U+0070 'p'",
463438
},
464439
{
465440
name: "wrong-ref-trace-id",
466-
dbSpans: []*dbmodel.Span{{References: []dbmodel.Reference{{TraceID: dbmodel.TraceID("ref-trace-id")}}}},
441+
dbSpans: []dbmodel.Span{{References: []dbmodel.Reference{{TraceID: dbmodel.TraceID("ref-trace-id")}}}},
467442
err: "encoding/hex: invalid byte: U+0072 'r'",
468443
},
469444
{
470445
name: "wrong-ref-span-id",
471-
dbSpans: []*dbmodel.Span{{References: []dbmodel.Reference{{SpanID: dbmodel.SpanID("ref-span-id")}}}},
446+
dbSpans: []dbmodel.Span{{References: []dbmodel.Reference{{SpanID: dbmodel.SpanID("ref-span-id")}}}},
472447
err: "encoding/hex: invalid byte: U+0072 'r'",
473448
},
474449
}
@@ -482,21 +457,21 @@ func TestFromDBModelErrors(t *testing.T) {
482457

483458
func TestSetParentId(t *testing.T) {
484459
parentSpanId := [8]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8}
485-
trace, err := FromDBModel([]*dbmodel.Span{{ParentSpanID: getDbSpanIdFromByteArray(parentSpanId)}})
460+
trace, err := FromDBModel([]dbmodel.Span{{ParentSpanID: getDbSpanIdFromByteArray(parentSpanId)}})
486461
require.NoError(t, err)
487462
assert.Equal(t, pcommon.SpanID(parentSpanId), trace.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).ParentSpanID())
488463
}
489464

490465
func TestParentIdWhenRefTraceIdIsDifferent(t *testing.T) {
491466
traceId := getDbTraceIdFromByteArray([16]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x80})
492467
refTraceId := getDbTraceIdFromByteArray([16]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x81})
493-
trace, err := FromDBModel([]*dbmodel.Span{{TraceID: traceId, References: []dbmodel.Reference{{TraceID: refTraceId}}}})
468+
trace, err := FromDBModel([]dbmodel.Span{{TraceID: traceId, References: []dbmodel.Reference{{TraceID: refTraceId}}}})
494469
require.NoError(t, err)
495470
assert.True(t, trace.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0).ParentSpanID().IsEmpty())
496471
}
497472

498473
func TestFromDBModelForTracesWithTwoLibraries(t *testing.T) {
499-
jb := []*dbmodel.Span{
474+
jb := []dbmodel.Span{
500475
{
501476
StartTime: model.TimeAsEpochMicroseconds(testSpanStartTime),
502477
Duration: model.DurationAsMicroseconds(testSpanEndTime.Sub(testSpanStartTime)),
@@ -654,7 +629,7 @@ func TestSetInternalSpanStatus(t *testing.T) {
654629
}
655630

656631
func TestFromDBModelToInternalTraces(t *testing.T) {
657-
batches := []*dbmodel.Span{
632+
batches := []dbmodel.Span{
658633
generateProtoSpan(),
659634
generateProtoSpan(),
660635
generateProtoChildSpan(),
@@ -817,10 +792,10 @@ func generateTracesOneSpanNoResourceWithTraceState() ptrace.Traces {
817792
return td
818793
}
819794

820-
func generateProtoSpan() *dbmodel.Span {
795+
func generateProtoSpan() dbmodel.Span {
821796
spanId := [8]byte{0xAF, 0xAE, 0xAD, 0xAC, 0xAB, 0xAA, 0xA9, 0xA8}
822797
traceId := [16]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x80}
823-
return &dbmodel.Span{
798+
return dbmodel.Span{
824799
TraceID: getDbTraceIdFromByteArray(traceId),
825800
SpanID: getDbSpanIdFromByteArray(spanId),
826801
OperationName: "operationA",
@@ -876,7 +851,7 @@ func generateProtoSpan() *dbmodel.Span {
876851
}
877852
}
878853

879-
func generateProtoSpanWithLibraryInfo(libraryName string) *dbmodel.Span {
854+
func generateProtoSpanWithLibraryInfo(libraryName string) dbmodel.Span {
880855
span := generateProtoSpan()
881856
span.Tags = append([]dbmodel.KeyValue{
882857
{
@@ -901,10 +876,10 @@ func getDbSpanIdFromByteArray(arr [8]byte) dbmodel.SpanID {
901876
return dbmodel.SpanID(hex.EncodeToString(arr[:]))
902877
}
903878

904-
func generateProtoSpanWithTraceState() *dbmodel.Span {
879+
func generateProtoSpanWithTraceState() dbmodel.Span {
905880
spanId := [8]byte{0xAF, 0xAE, 0xAD, 0xAC, 0xAB, 0xAA, 0xA9, 0xA8}
906881
traceId := [16]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x80}
907-
return &dbmodel.Span{
882+
return dbmodel.Span{
908883
TraceID: getDbTraceIdFromByteArray(traceId),
909884
SpanID: getDbSpanIdFromByteArray(spanId),
910885
OperationName: "operationA",
@@ -995,9 +970,9 @@ func setChildSpan(span, parentSpan ptrace.Span) {
995970
span.Attributes().PutInt(conventions.AttributeHTTPStatusCode, 404)
996971
}
997972

998-
func generateProtoChildSpan() *dbmodel.Span {
973+
func generateProtoChildSpan() dbmodel.Span {
999974
traceID := getDbTraceIdFromByteArray([16]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x80})
1000-
return &dbmodel.Span{
975+
return dbmodel.Span{
1001976
TraceID: traceID,
1002977
SpanID: getDbSpanIdFromByteArray([8]byte{0x1F, 0x1E, 0x1D, 0x1C, 0x1B, 0x1A, 0x19, 0x18}),
1003978
OperationName: "operationB",
@@ -1065,9 +1040,9 @@ func setFollowFromSpan(span, followFromSpan ptrace.Span) {
10651040
)
10661041
}
10671042

1068-
func generateProtoFollowerSpan() *dbmodel.Span {
1043+
func generateProtoFollowerSpan() dbmodel.Span {
10691044
traceID := getDbTraceIdFromByteArray([16]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x80})
1070-
return &dbmodel.Span{
1045+
return dbmodel.Span{
10711046
TraceID: traceID,
10721047
SpanID: getDbSpanIdFromByteArray([8]byte{0x1F, 0x1E, 0x1D, 0x1C, 0x1B, 0x1A, 0x19, 0x18}),
10731048
OperationName: "operationC",
@@ -1142,9 +1117,9 @@ func setSpanWithTwoParents(span, parent, parent2 ptrace.Span) {
11421117
)
11431118
}
11441119

1145-
func generateProtoTwoParentsSpan() *dbmodel.Span {
1120+
func generateProtoTwoParentsSpan() dbmodel.Span {
11461121
traceID := getDbTraceIdFromByteArray([16]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x80})
1147-
return &dbmodel.Span{
1122+
return dbmodel.Span{
11481123
TraceID: traceID,
11491124
SpanID: getDbSpanIdFromByteArray([8]byte{0x1F, 0x1E, 0x1D, 0x1C, 0x1B, 0x1A, 0x19, 0x20}),
11501125
OperationName: "operationD",
@@ -1186,7 +1161,7 @@ func generateProtoTwoParentsSpan() *dbmodel.Span {
11861161
}
11871162

11881163
func BenchmarkProtoBatchToInternalTraces(b *testing.B) {
1189-
jb := []*dbmodel.Span{
1164+
jb := []dbmodel.Span{
11901165
generateProtoSpan(),
11911166
generateProtoChildSpan(),
11921167
}

‎internal/storage/v2/elasticsearch/tracestore/to_dbmodel.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ const (
2626

2727
// ToDBModel translates internal trace data into the DB Spans.
2828
// Returns slice of translated DB Spans and error if translation failed.
29-
func ToDBModel(td ptrace.Traces) []*dbmodel.Span {
29+
func ToDBModel(td ptrace.Traces) []dbmodel.Span {
3030
resourceSpans := td.ResourceSpans()
3131

3232
if resourceSpans.Len() == 0 {
3333
return nil
3434
}
3535

36-
batches := make([]*dbmodel.Span, 0, resourceSpans.Len())
36+
batches := make([]dbmodel.Span, 0, resourceSpans.Len())
3737
for i := 0; i < resourceSpans.Len(); i++ {
3838
rs := resourceSpans.At(i)
3939
batch := resourceSpansToDbSpans(rs)
@@ -45,19 +45,19 @@ func ToDBModel(td ptrace.Traces) []*dbmodel.Span {
4545
return batches
4646
}
4747

48-
func resourceSpansToDbSpans(resourceSpans ptrace.ResourceSpans) []*dbmodel.Span {
48+
func resourceSpansToDbSpans(resourceSpans ptrace.ResourceSpans) []dbmodel.Span {
4949
resource := resourceSpans.Resource()
5050
scopeSpans := resourceSpans.ScopeSpans()
5151

5252
if scopeSpans.Len() == 0 {
53-
return []*dbmodel.Span{}
53+
return []dbmodel.Span{}
5454
}
5555

5656
process := resourceToDbProcess(resource)
5757

5858
// Approximate the number of the spans as the number of the spans in the first
5959
// instrumentation library info.
60-
dbSpans := make([]*dbmodel.Span, 0, scopeSpans.At(0).Spans().Len())
60+
dbSpans := make([]dbmodel.Span, 0, scopeSpans.At(0).Spans().Len())
6161

6262
for _, scopeSpan := range scopeSpans.All() {
6363
for _, span := range scopeSpan.Spans().All() {
@@ -115,11 +115,11 @@ func attributeToDbTag(key string, attr pcommon.Value) dbmodel.KeyValue {
115115
return tag
116116
}
117117

118-
func spanToDbSpan(span ptrace.Span, libraryTags pcommon.InstrumentationScope, process dbmodel.Process) *dbmodel.Span {
118+
func spanToDbSpan(span ptrace.Span, libraryTags pcommon.InstrumentationScope, process dbmodel.Process) dbmodel.Span {
119119
traceID := dbmodel.TraceID(span.TraceID().String())
120120
parentSpanID := dbmodel.SpanID(span.ParentSpanID().String())
121121
startTime := span.StartTimestamp().AsTime()
122-
return &dbmodel.Span{
122+
return dbmodel.Span{
123123
TraceID: traceID,
124124
SpanID: dbmodel.SpanID(span.SpanID().String()),
125125
OperationName: span.Name(),

0 commit comments

Comments
 (0)