Skip to content

Commit f00df84

Browse files
Manik2708sAchin-680
Manik2708
authored and
sAchin-680
committed
[ES] Remove pointer signatures from CoreSpanReader (#6874)
## Which problem is this PR solving? Fixes a part of: #6458 ## Description of the changes - Removal of pointer signatures from `CoreSpanReader` ## 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: sAchin-680 <[email protected]>
1 parent 08d7463 commit f00df84

File tree

7 files changed

+69
-79
lines changed

7 files changed

+69
-79
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ import (
1212
// CoreSpanReader is a DB-Level abstraction which directly deals with database level operations
1313
type CoreSpanReader interface {
1414
// FindTraceIDs retrieves traces IDs that match the traceQuery
15-
FindTraceIDs(ctx context.Context, traceQuery *dbmodel.TraceQueryParameters) ([]dbmodel.TraceID, error)
15+
FindTraceIDs(ctx context.Context, traceQuery dbmodel.TraceQueryParameters) ([]dbmodel.TraceID, error)
1616
// FindTraces retrieves traces that match the traceQuery
17-
FindTraces(ctx context.Context, traceQuery *dbmodel.TraceQueryParameters) ([]*dbmodel.Trace, error)
17+
FindTraces(ctx context.Context, traceQuery dbmodel.TraceQueryParameters) ([]dbmodel.Trace, error)
1818
// GetOperations returns all operations for a specific service traced by Jaeger
1919
GetOperations(ctx context.Context, query dbmodel.OperationQueryParameters) ([]dbmodel.Operation, error)
2020
// GetServices returns all services traced by Jaeger, ordered by frequency
2121
GetServices(ctx context.Context) ([]string, error)
2222
// GetTraces takes a traceID and returns a Trace associated with that traceID
23-
GetTraces(ctx context.Context, query []dbmodel.TraceID) ([]*dbmodel.Trace, error)
23+
GetTraces(ctx context.Context, query []dbmodel.TraceID) ([]dbmodel.Trace, error)
2424
}

‎internal/storage/v1/elasticsearch/spanstore/internal/dbmodel/model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type ValueType string
2020

2121
// Trace is the type of traces
2222
type Trace struct {
23-
Spans []*Span
23+
Spans []Span
2424
}
2525

2626
const (

‎internal/storage/v1/elasticsearch/spanstore/mocks/CoreSpanReader.go

Lines changed: 15 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -237,16 +237,16 @@ func timeRangeIndices(indexName, indexDateLayout string, startTime time.Time, en
237237
}
238238

239239
// GetTraces takes a traceID and returns a Trace associated with that traceID
240-
func (s *SpanReader) GetTraces(ctx context.Context, query []dbmodel.TraceID) ([]*dbmodel.Trace, error) {
240+
func (s *SpanReader) GetTraces(ctx context.Context, query []dbmodel.TraceID) ([]dbmodel.Trace, error) {
241241
ctx, span := s.tracer.Start(ctx, "GetTrace")
242242
defer span.End()
243243
currentTime := time.Now()
244244
// TODO: use start time & end time in "query" struct
245245
return s.multiRead(ctx, query, currentTime.Add(-s.maxSpanAge), currentTime)
246246
}
247247

248-
func (s *SpanReader) collectSpans(esSpansRaw []*elastic.SearchHit) ([]*dbmodel.Span, error) {
249-
spans := make([]*dbmodel.Span, len(esSpansRaw))
248+
func (s *SpanReader) collectSpans(esSpansRaw []*elastic.SearchHit) ([]dbmodel.Span, error) {
249+
spans := make([]dbmodel.Span, len(esSpansRaw))
250250

251251
for i, esSpanRaw := range esSpansRaw {
252252
jsonSpan, err := s.unmarshalJSONSpan(esSpanRaw)
@@ -258,17 +258,17 @@ func (s *SpanReader) collectSpans(esSpansRaw []*elastic.SearchHit) ([]*dbmodel.S
258258
return spans, nil
259259
}
260260

261-
func (*SpanReader) unmarshalJSONSpan(esSpanRaw *elastic.SearchHit) (*dbmodel.Span, error) {
261+
func (*SpanReader) unmarshalJSONSpan(esSpanRaw *elastic.SearchHit) (dbmodel.Span, error) {
262262
esSpanInByteArray := esSpanRaw.Source
263263

264264
var jsonSpan dbmodel.Span
265265

266266
d := json.NewDecoder(bytes.NewReader(*esSpanInByteArray))
267267
d.UseNumber()
268268
if err := d.Decode(&jsonSpan); err != nil {
269-
return nil, err
269+
return dbmodel.Span{}, err
270270
}
271-
return &jsonSpan, nil
271+
return jsonSpan, nil
272272
}
273273

274274
// GetServices returns all services traced by Jaeger, ordered by frequency
@@ -330,7 +330,7 @@ func bucketToStringArray[T ~string](buckets []*elastic.AggregationBucketKeyItem)
330330
}
331331

332332
// FindTraces retrieves traces that match the traceQuery
333-
func (s *SpanReader) FindTraces(ctx context.Context, traceQuery *dbmodel.TraceQueryParameters) ([]*dbmodel.Trace, error) {
333+
func (s *SpanReader) FindTraces(ctx context.Context, traceQuery dbmodel.TraceQueryParameters) ([]dbmodel.Trace, error) {
334334
ctx, span := s.tracer.Start(ctx, "FindTraces")
335335
defer span.End()
336336

@@ -342,7 +342,7 @@ func (s *SpanReader) FindTraces(ctx context.Context, traceQuery *dbmodel.TraceQu
342342
}
343343

344344
// FindTraceIDs retrieves traces IDs that match the traceQuery
345-
func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *dbmodel.TraceQueryParameters) ([]dbmodel.TraceID, error) {
345+
func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery dbmodel.TraceQueryParameters) ([]dbmodel.TraceID, error) {
346346
ctx, span := s.tracer.Start(ctx, "FindTraceIDs")
347347
defer span.End()
348348

@@ -361,7 +361,7 @@ func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *dbmodel.Trace
361361
return esTraceIDs, nil
362362
}
363363

364-
func (s *SpanReader) multiRead(ctx context.Context, traceIDs []dbmodel.TraceID, startTime, endTime time.Time) ([]*dbmodel.Trace, error) {
364+
func (s *SpanReader) multiRead(ctx context.Context, traceIDs []dbmodel.TraceID, startTime, endTime time.Time) ([]dbmodel.Trace, error) {
365365
ctx, childSpan := s.tracer.Start(ctx, "multiRead")
366366
defer childSpan.End()
367367

@@ -373,8 +373,10 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []dbmodel.TraceID,
373373
childSpan.SetAttributes(attribute.Key("trace_ids").StringSlice(tracesIDs))
374374
}
375375

376+
traces := make([]dbmodel.Trace, 0, len(traceIDs))
377+
376378
if len(traceIDs) == 0 {
377-
return []*dbmodel.Trace{}, nil
379+
return traces, nil
378380
}
379381

380382
// Add an hour in both directions so that traces that straddle two indexes are retrieved.
@@ -441,7 +443,8 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []dbmodel.TraceID,
441443
if traceSpan, ok := tracesMap[lastSpan.TraceID]; ok {
442444
traceSpan.Spans = append(traceSpan.Spans, spans...)
443445
} else {
444-
tracesMap[lastSpan.TraceID] = &dbmodel.Trace{Spans: spans}
446+
traces = append(traces, dbmodel.Trace{Spans: spans})
447+
tracesMap[lastSpan.TraceID] = &traces[len(traces)-1]
445448
}
446449

447450
totalDocumentsFetched[lastSpan.TraceID] += len(result.Hits.Hits)
@@ -451,11 +454,6 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []dbmodel.TraceID,
451454
}
452455
}
453456
}
454-
455-
var traces []*dbmodel.Trace
456-
for _, t := range tracesMap {
457-
traces = append(traces, t)
458-
}
459457
return traces, nil
460458
}
461459

@@ -472,10 +470,7 @@ func buildTraceByIDQuery(traceID dbmodel.TraceID) elastic.Query {
472470
elastic.NewTermQuery(traceIDField, legacyTraceID))
473471
}
474472

475-
func validateQuery(p *dbmodel.TraceQueryParameters) error {
476-
if p == nil {
477-
return ErrMalformedRequestObject
478-
}
473+
func validateQuery(p dbmodel.TraceQueryParameters) error {
479474
if p.ServiceName == "" && len(p.Tags) > 0 {
480475
return ErrServiceNameNotSet
481476
}
@@ -491,7 +486,7 @@ func validateQuery(p *dbmodel.TraceQueryParameters) error {
491486
return nil
492487
}
493488

494-
func (s *SpanReader) findTraceIDs(ctx context.Context, traceQuery *dbmodel.TraceQueryParameters) ([]dbmodel.TraceID, error) {
489+
func (s *SpanReader) findTraceIDs(ctx context.Context, traceQuery dbmodel.TraceQueryParameters) ([]dbmodel.TraceID, error) {
495490
ctx, childSpan := s.tracer.Start(ctx, "findTraceIDs")
496491
defer childSpan.End()
497492
// Below is the JSON body to our HTTP GET request to ElasticSearch. This function creates this.
@@ -594,7 +589,7 @@ func (*SpanReader) buildTraceIDSubAggregation() elastic.Aggregation {
594589
Field(startTimeField)
595590
}
596591

597-
func (s *SpanReader) buildFindTraceIDsQuery(traceQuery *dbmodel.TraceQueryParameters) elastic.Query {
592+
func (s *SpanReader) buildFindTraceIDsQuery(traceQuery dbmodel.TraceQueryParameters) elastic.Query {
598593
boolQuery := elastic.NewBoolQuery()
599594

600595
// add duration query

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

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ func TestSpanReader_multiRead_followUp_query(t *testing.T) {
395395
require.NotNil(t, traces)
396396
require.Len(t, traces, 2)
397397

398-
for _, s := range []*dbmodel.Span{&spanID1, &spanID2} {
398+
for _, s := range []dbmodel.Span{spanID1, spanID2} {
399399
found := reflect.DeepEqual(traces[0].Spans[0], s) || reflect.DeepEqual(traces[1].Spans[0], s)
400400
assert.True(t, found, "span was expected to be within one of the traces but was not: %v", s)
401401
}
@@ -509,7 +509,7 @@ func TestSpanReader_esJSONtoJSONSpanModel(t *testing.T) {
509509

510510
var expectedSpan dbmodel.Span
511511
require.NoError(t, json.Unmarshal(exampleESSpan, &expectedSpan))
512-
assert.EqualValues(t, &expectedSpan, span)
512+
assert.EqualValues(t, expectedSpan, span)
513513
})
514514
}
515515

@@ -522,9 +522,8 @@ func TestSpanReader_esJSONtoJSONSpanModelError(t *testing.T) {
522522
Source: jsonPayload,
523523
}
524524

525-
span, err := r.reader.unmarshalJSONSpan(esSpanRaw)
525+
_, err := r.reader.unmarshalJSONSpan(esSpanRaw)
526526
require.Error(t, err)
527-
assert.Nil(t, span)
528527
})
529528
}
530529

@@ -655,7 +654,7 @@ func returnSearchFunc(typ string, r *spanReaderTest) (any, error) {
655654
dbmodel.OperationQueryParameters{ServiceName: "someService"},
656655
)
657656
case traceIDAggregation:
658-
return r.reader.findTraceIDs(context.Background(), &dbmodel.TraceQueryParameters{})
657+
return r.reader.findTraceIDs(context.Background(), dbmodel.TraceQueryParameters{})
659658
}
660659
return nil, errors.New("Specify services, operations, traceIDs only")
661660
}
@@ -710,7 +709,7 @@ func TestSpanReader_FindTraces(t *testing.T) {
710709
},
711710
}, nil)
712711

713-
traceQuery := &dbmodel.TraceQueryParameters{
712+
traceQuery := dbmodel.TraceQueryParameters{
714713
ServiceName: serviceName,
715714
Tags: map[string]string{
716715
"hello": "world",
@@ -756,7 +755,7 @@ func TestSpanReader_FindTracesInvalidQuery(t *testing.T) {
756755
},
757756
}, nil)
758757

759-
traceQuery := &dbmodel.TraceQueryParameters{
758+
traceQuery := dbmodel.TraceQueryParameters{
760759
ServiceName: "",
761760
Tags: map[string]string{
762761
"hello": "world",
@@ -789,7 +788,7 @@ func TestSpanReader_FindTracesAggregationFailure(t *testing.T) {
789788
Responses: []*elastic.SearchResult{},
790789
}, nil)
791790

792-
traceQuery := &dbmodel.TraceQueryParameters{
791+
traceQuery := dbmodel.TraceQueryParameters{
793792
ServiceName: serviceName,
794793
Tags: map[string]string{
795794
"hello": "world",
@@ -824,7 +823,7 @@ func TestSpanReader_FindTracesNoTraceIDs(t *testing.T) {
824823
Responses: []*elastic.SearchResult{},
825824
}, nil)
826825

827-
traceQuery := &dbmodel.TraceQueryParameters{
826+
traceQuery := dbmodel.TraceQueryParameters{
828827
ServiceName: serviceName,
829828
Tags: map[string]string{
830829
"hello": "world",
@@ -858,7 +857,7 @@ func TestSpanReader_FindTracesReadTraceFailure(t *testing.T) {
858857
mockMultiSearchService(r).
859858
Return(nil, errors.New("read error"))
860859

861-
traceQuery := &dbmodel.TraceQueryParameters{
860+
traceQuery := dbmodel.TraceQueryParameters{
862861
ServiceName: serviceName,
863862
Tags: map[string]string{
864863
"hello": "world",
@@ -897,7 +896,7 @@ func TestSpanReader_FindTracesSpanCollectionFailure(t *testing.T) {
897896
},
898897
}, nil)
899898

900-
traceQuery := &dbmodel.TraceQueryParameters{
899+
traceQuery := dbmodel.TraceQueryParameters{
901900
ServiceName: serviceName,
902901
Tags: map[string]string{
903902
"hello": "world",
@@ -968,17 +967,13 @@ func mockSearchService(r *spanReaderTest) *mock.Call {
968967
}
969968

970969
func TestTraceQueryParameterValidation(t *testing.T) {
971-
var malformedtqp *dbmodel.TraceQueryParameters
972-
err := validateQuery(malformedtqp)
973-
require.EqualError(t, err, ErrMalformedRequestObject.Error())
974-
975-
tqp := &dbmodel.TraceQueryParameters{
970+
tqp := dbmodel.TraceQueryParameters{
976971
ServiceName: "",
977972
Tags: map[string]string{
978973
"hello": "world",
979974
},
980975
}
981-
err = validateQuery(tqp)
976+
err := validateQuery(tqp)
982977
require.EqualError(t, err, ErrServiceNameNotSet.Error())
983978

984979
tqp.ServiceName = serviceName
@@ -1030,7 +1025,7 @@ func TestSpanReader_buildTraceIDAggregation(t *testing.T) {
10301025

10311026
func TestSpanReader_buildFindTraceIDsQuery(t *testing.T) {
10321027
withSpanReader(t, func(r *spanReaderTest) {
1033-
traceQuery := &dbmodel.TraceQueryParameters{
1028+
traceQuery := dbmodel.TraceQueryParameters{
10341029
DurationMin: time.Second,
10351030
DurationMax: time.Second * 2,
10361031
StartTimeMin: time.Time{},
@@ -1191,7 +1186,7 @@ func TestSpanReader_GetEmptyIndex(t *testing.T) {
11911186
Responses: []*elastic.SearchResult{},
11921187
}, nil)
11931188

1194-
traceQuery := &dbmodel.TraceQueryParameters{
1189+
traceQuery := dbmodel.TraceQueryParameters{
11951190
ServiceName: serviceName,
11961191
Tags: map[string]string{
11971192
"hello": "world",

0 commit comments

Comments
 (0)