From 23bbaf523452e50d022a61ea06d8215f08b7ca9e Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Fri, 11 Jan 2019 15:55:59 +0400 Subject: [PATCH 01/13] The implementations of "FindTraceIDs" function for ElasticSearch reader. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index b42e97cef1f..f4aa985a110 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -234,9 +234,24 @@ func (s *SpanReader) FindTraces(ctx context.Context, traceQuery *spanstore.Trace return s.multiRead(ctx, uniqueTraceIDs, traceQuery.StartTimeMin, traceQuery.StartTimeMax) } -// FindTraceIDs is not implemented. +// FindTraceIDs retrieves traces IDs that match the traceQuery func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.TraceQueryParameters) ([]model.TraceID, error) { - return nil, errors.New("not implemented") // TODO: Implement + uniqueTraceIDs, err := s.findTraceIDs(ctx, traceQuery) + if err != nil { + return nil, err + } + + traceIDs := make([]model.TraceID, len(uniqueTraceIDs)) + for i, ID := range uniqueTraceIDs { + traceID, err := model.TraceIDFromString(ID) + if err != nil { + return nil, err + } + + traceIDs[i] = traceID + } + + return traceIDs, nil } func (s *SpanReader) multiRead(ctx context.Context, traceIDs []string, startTime, endTime time.Time) ([]*model.Trace, error) { From c55fde1a919f23ad154cfacb59cbff938a00a2fc Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Fri, 11 Jan 2019 16:16:34 +0400 Subject: [PATCH 02/13] The using validation of query and added default numTraces value. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index f4aa985a110..152535257d7 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -236,19 +236,28 @@ func (s *SpanReader) FindTraces(ctx context.Context, traceQuery *spanstore.Trace // FindTraceIDs retrieves traces IDs that match the traceQuery func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.TraceQueryParameters) ([]model.TraceID, error) { - uniqueTraceIDs, err := s.findTraceIDs(ctx, traceQuery) + if err := validateQuery(traceQuery); err != nil { + return nil, err + } + if traceQuery.NumTraces == 0 { + traceQuery.NumTraces = defaultNumTraces + } + + esTraceIDs, err := s.findTraceIDs(ctx, traceQuery) if err != nil { return nil, err } - traceIDs := make([]model.TraceID, len(uniqueTraceIDs)) - for i, ID := range uniqueTraceIDs { - traceID, err := model.TraceIDFromString(ID) + traceIDs := make([]model.TraceID, traceQuery.NumTraces) + for i, traceID := range esTraceIDs { + if len(traceIDs) > traceQuery.NumTraces { + break + } + + traceIDs[i], err = model.TraceIDFromString(traceID) if err != nil { return nil, err } - - traceIDs[i] = traceID } return traceIDs, nil From 4350e3a7b73ac8825e3b7bc7b94d4716b6f3a8de Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Fri, 11 Jan 2019 18:18:01 +0400 Subject: [PATCH 03/13] The tests for FindTraceIDs were added. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 10 +- plugin/storage/es/spanstore/reader_test.go | 114 ++++++++++++++++++++- 2 files changed, 117 insertions(+), 7 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index 152535257d7..a929b6a8d0f 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -248,16 +248,18 @@ func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.Tra return nil, err } - traceIDs := make([]model.TraceID, traceQuery.NumTraces) - for i, traceID := range esTraceIDs { - if len(traceIDs) > traceQuery.NumTraces { + traceIDs := []model.TraceID{} + for _, ID := range esTraceIDs { + if len(traceIDs) >= traceQuery.NumTraces { break } - traceIDs[i], err = model.TraceIDFromString(traceID) + traceID, err := model.TraceIDFromString(ID) if err != nil { return nil, err } + + traceIDs = append(traceIDs, traceID) } return traceIDs, nil diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index f0cf63f1aed..227f25146e7 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -668,11 +668,119 @@ func TestFindTraceIDs(t *testing.T) { testGet(traceIDAggregation, t) } -func TestFindTraceIDNotImplemented(t *testing.T) { +func TestSpanReader_TestFindTraceIDs(t *testing.T) { + goodAggregations := make(map[string]*json.RawMessage) + rawMessage := []byte(`{"buckets": [{"key": "1","doc_count": 16},{"key": "2","doc_count": 16},{"key": "3","doc_count": 16}]}`) + goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) + withSpanReader(func(r *spanReaderTest) { - traceIDs, err := r.reader.FindTraceIDs(context.Background(), nil) + // find trace IDs + mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) + + traceIDsQuery := &spanstore.TraceQueryParameters{ + ServiceName: serviceName, + Tags: map[string]string{ + "hello": "world", + }, + StartTimeMin: time.Now().Add(-1 * time.Hour), + StartTimeMax: time.Now(), + NumTraces: 1, + } + + traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) + require.NoError(t, err) + assert.Len(t, traceIDs, 1) + assert.EqualValues(t, 1, traceIDs[0].Low) + }) +} + +func TestSpanReader_FindTraceIDsInvalidQuery(t *testing.T) { + goodAggregations := make(map[string]*json.RawMessage) + rawMessage := []byte(`{"buckets": [{"key": "1","doc_count": 16},{"key": "2","doc_count": 16},{"key": "3","doc_count": 16}]}`) + goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) + + withSpanReader(func(r *spanReaderTest) { + mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) + + traceIDsQuery := &spanstore.TraceQueryParameters{ + ServiceName: "", + Tags: map[string]string{ + "hello": "world", + }, + StartTimeMin: time.Now().Add(-1 * time.Hour), + StartTimeMax: time.Now(), + } + + traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) + require.Error(t, err) + assert.Nil(t, traceIDs) + }) +} + +func TestSpanReader_FindTraceIDsAggregationFailure(t *testing.T) { + goodAggregations := make(map[string]*json.RawMessage) + + withSpanReader(func(r *spanReaderTest) { + mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) + + traceIDsQuery := &spanstore.TraceQueryParameters{ + ServiceName: serviceName, + Tags: map[string]string{ + "hello": "world", + }, + StartTimeMin: time.Now().Add(-1 * time.Hour), + StartTimeMax: time.Now(), + } + + traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) + require.Error(t, err) assert.Nil(t, traceIDs) - assert.EqualError(t, err, "not implemented") + }) +} + +func TestSpanReader_FindTraceIDsNoTraceIDs(t *testing.T) { + goodAggregations := make(map[string]*json.RawMessage) + rawMessage := []byte(`{"buckets": []}`) + goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) + + withSpanReader(func(r *spanReaderTest) { + mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) + + traceIDsQuery := &spanstore.TraceQueryParameters{ + ServiceName: serviceName, + Tags: map[string]string{ + "hello": "world", + }, + StartTimeMin: time.Now().Add(-1 * time.Hour), + StartTimeMax: time.Now(), + } + + traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) + require.NoError(t, err) + assert.Len(t, traceIDs, 0) + }) +} + +func TestSpanReader_FindTraceIDsReadTraceIDsFailure(t *testing.T) { + goodAggregations := make(map[string]*json.RawMessage) + rawMessage := []byte(`{"buckets": [{"key": "1","doc_count": 16},{"key": "2","doc_count": 16}]}`) + goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) + + withSpanReader(func(r *spanReaderTest) { + mockSearchService(r).Return(nil, errors.New("read error")) + + traceIDsQuery := &spanstore.TraceQueryParameters{ + ServiceName: serviceName, + Tags: map[string]string{ + "hello": "world", + }, + StartTimeMin: time.Now().Add(-1 * time.Hour), + StartTimeMax: time.Now(), + } + + traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) + require.EqualError(t, err, "Search service failed: read error") + assert.Len(t, traceIDs, 0) }) } From 0ace6df05f97b07567deba7a7ac9b63d48058ee4 Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Fri, 11 Jan 2019 19:14:49 +0400 Subject: [PATCH 04/13] The test for checking incorrect traceID. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader_test.go | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index 227f25146e7..aecc71e433b 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -784,6 +784,29 @@ func TestSpanReader_FindTraceIDsReadTraceIDsFailure(t *testing.T) { }) } +func TestSpanReader_FindTraceIDsIncorrectTraceIDFailure(t *testing.T) { + goodAggregations := make(map[string]*json.RawMessage) + rawMessage := []byte(`{"buckets": [{"key": "dfddslsfdsfdsdscc","doc_count": 16},{"key": "2","doc_count": 16}]}`) + goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) + + withSpanReader(func(r *spanReaderTest) { + mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) + + traceIDsQuery := &spanstore.TraceQueryParameters{ + ServiceName: serviceName, + Tags: map[string]string{ + "hello": "world", + }, + StartTimeMin: time.Now().Add(-1 * time.Hour), + StartTimeMax: time.Now(), + } + + traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) + require.EqualError(t, err, "strconv.ParseUint: parsing \"fddslsfdsfdsdscc\": invalid syntax") + assert.Len(t, traceIDs, 0) + }) +} + func mockMultiSearchService(r *spanReaderTest) *mock.Call { multiSearchService := &mocks.MultiSearchService{} multiSearchService.On("Add", mock.Anything, mock.Anything, mock.Anything).Return(multiSearchService) From d4a2644bd206b05acaa17aae5cc6e4c4e5591491 Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Fri, 11 Jan 2019 20:32:20 +0400 Subject: [PATCH 05/13] The tests are fixed. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index aecc71e433b..93beab03a2f 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -786,7 +786,7 @@ func TestSpanReader_FindTraceIDsReadTraceIDsFailure(t *testing.T) { func TestSpanReader_FindTraceIDsIncorrectTraceIDFailure(t *testing.T) { goodAggregations := make(map[string]*json.RawMessage) - rawMessage := []byte(`{"buckets": [{"key": "dfddslsfdsfdsdscc","doc_count": 16},{"key": "2","doc_count": 16}]}`) + rawMessage := []byte(`{"buckets": [{"key": "sdfsdfdssd234nsdvsdfldjsf","doc_count": 16},{"key": "2","doc_count": 16}]}`) goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) withSpanReader(func(r *spanReaderTest) { @@ -802,7 +802,7 @@ func TestSpanReader_FindTraceIDsIncorrectTraceIDFailure(t *testing.T) { } traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) - require.EqualError(t, err, "strconv.ParseUint: parsing \"fddslsfdsfdsdscc\": invalid syntax") + require.EqualError(t, err, `strconv.ParseUint: parsing "sdfsdfdss": invalid syntax`) assert.Len(t, traceIDs, 0) }) } From e90848de6eb9be7591ea25e81814bbb5ccb8001e Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Tue, 15 Jan 2019 16:39:49 +0400 Subject: [PATCH 06/13] The improvements by code-review. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 34 +++++++++++---------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index a929b6a8d0f..e0b0a45d6e8 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -221,12 +221,6 @@ func (s *SpanReader) FindTraces(ctx context.Context, traceQuery *spanstore.Trace span, ctx := opentracing.StartSpanFromContext(ctx, "FindTraces") defer span.Finish() - if err := validateQuery(traceQuery); err != nil { - return nil, err - } - if traceQuery.NumTraces == 0 { - traceQuery.NumTraces = defaultNumTraces - } uniqueTraceIDs, err := s.findTraceIDs(ctx, traceQuery) if err != nil { return nil, err @@ -236,30 +230,22 @@ func (s *SpanReader) FindTraces(ctx context.Context, traceQuery *spanstore.Trace // FindTraceIDs retrieves traces IDs that match the traceQuery func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.TraceQueryParameters) ([]model.TraceID, error) { - if err := validateQuery(traceQuery); err != nil { - return nil, err - } - if traceQuery.NumTraces == 0 { - traceQuery.NumTraces = defaultNumTraces - } + span, ctx := opentracing.StartSpanFromContext(ctx, "FindTraceIDs") + defer span.Finish() esTraceIDs, err := s.findTraceIDs(ctx, traceQuery) if err != nil { return nil, err } - traceIDs := []model.TraceID{} - for _, ID := range esTraceIDs { - if len(traceIDs) >= traceQuery.NumTraces { - break - } - + traceIDs := make([]model.TraceID, len(esTraceIDs)) + for i, ID := range esTraceIDs { traceID, err := model.TraceIDFromString(ID) if err != nil { - return nil, err + return nil, errors.Wrap(err, fmt.Sprintf("Making traceID from string '%s' failed", ID)) } - traceIDs = append(traceIDs, traceID) + traceIDs[i] = traceID } return traceIDs, nil @@ -365,6 +351,14 @@ func validateQuery(p *spanstore.TraceQueryParameters) error { func (s *SpanReader) findTraceIDs(ctx context.Context, traceQuery *spanstore.TraceQueryParameters) ([]string, error) { childSpan, _ := opentracing.StartSpanFromContext(ctx, "findTraceIDs") defer childSpan.Finish() + + if err := validateQuery(traceQuery); err != nil { + return nil, err + } + if traceQuery.NumTraces == 0 { + traceQuery.NumTraces = defaultNumTraces + } + // Below is the JSON body to our HTTP GET request to ElasticSearch. This function creates this. // { // "size": 0, From ae75936628ea74b69d746d61b00901eca4dbcbc2 Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Tue, 15 Jan 2019 16:40:54 +0400 Subject: [PATCH 07/13] The new function validateQueryAndFindTraceIDs. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 25 ++++++++++++++-------- plugin/storage/es/spanstore/reader_test.go | 6 +++--- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index e0b0a45d6e8..4d828891524 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -221,7 +221,7 @@ func (s *SpanReader) FindTraces(ctx context.Context, traceQuery *spanstore.Trace span, ctx := opentracing.StartSpanFromContext(ctx, "FindTraces") defer span.Finish() - uniqueTraceIDs, err := s.findTraceIDs(ctx, traceQuery) + uniqueTraceIDs, err := s.validateQueryAndFindTraceIDs(ctx, traceQuery) if err != nil { return nil, err } @@ -233,7 +233,7 @@ func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.Tra span, ctx := opentracing.StartSpanFromContext(ctx, "FindTraceIDs") defer span.Finish() - esTraceIDs, err := s.findTraceIDs(ctx, traceQuery) + esTraceIDs, err := s.validateQueryAndFindTraceIDs(ctx, traceQuery) if err != nil { return nil, err } @@ -329,6 +329,20 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []string, startTime return traces, nil } +func (s *SpanReader) validateQueryAndFindTraceIDs(ctx context.Context, traceQuery *spanstore.TraceQueryParameters) ([]string, error) { + childSpan, _ := opentracing.StartSpanFromContext(ctx, "validateQueryAndFindTraceIDs") + defer childSpan.Finish() + + if err := validateQuery(traceQuery); err != nil { + return nil, err + } + if traceQuery.NumTraces == 0 { + traceQuery.NumTraces = defaultNumTraces + } + + return s.findTraceIDs(ctx, traceQuery) +} + func validateQuery(p *spanstore.TraceQueryParameters) error { if p == nil { return ErrMalformedRequestObject @@ -352,13 +366,6 @@ func (s *SpanReader) findTraceIDs(ctx context.Context, traceQuery *spanstore.Tra childSpan, _ := opentracing.StartSpanFromContext(ctx, "findTraceIDs") defer childSpan.Finish() - if err := validateQuery(traceQuery); err != nil { - return nil, err - } - if traceQuery.NumTraces == 0 { - traceQuery.NumTraces = defaultNumTraces - } - // Below is the JSON body to our HTTP GET request to ElasticSearch. This function creates this. // { // "size": 0, diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index 93beab03a2f..ce3b112deb0 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -684,12 +684,12 @@ func TestSpanReader_TestFindTraceIDs(t *testing.T) { }, StartTimeMin: time.Now().Add(-1 * time.Hour), StartTimeMax: time.Now(), - NumTraces: 1, + NumTraces: 3, } traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) require.NoError(t, err) - assert.Len(t, traceIDs, 1) + assert.Len(t, traceIDs, 3) assert.EqualValues(t, 1, traceIDs[0].Low) }) } @@ -802,7 +802,7 @@ func TestSpanReader_FindTraceIDsIncorrectTraceIDFailure(t *testing.T) { } traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) - require.EqualError(t, err, `strconv.ParseUint: parsing "sdfsdfdss": invalid syntax`) + require.EqualError(t, err, `Making traceID from string 'sdfsdfdssd234nsdvsdfldjsf' failed: strconv.ParseUint: parsing "sdfsdfdss": invalid syntax`) assert.Len(t, traceIDs, 0) }) } From 424bc310348d7ba616b006a8600c32f14aebb21a Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Tue, 29 Jan 2019 18:57:24 +0400 Subject: [PATCH 08/13] The span validateQueryAndFindTraceIDs was removed. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index 4d828891524..890b4e6ee8b 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -330,9 +330,6 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []string, startTime } func (s *SpanReader) validateQueryAndFindTraceIDs(ctx context.Context, traceQuery *spanstore.TraceQueryParameters) ([]string, error) { - childSpan, _ := opentracing.StartSpanFromContext(ctx, "validateQueryAndFindTraceIDs") - defer childSpan.Finish() - if err := validateQuery(traceQuery); err != nil { return nil, err } From 5aae75f1e262180ab7f793dc8b9b62c47bc04390 Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Tue, 29 Jan 2019 19:08:30 +0400 Subject: [PATCH 09/13] The using slice of model.TraceID instead slice of string in multiRead function. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 45 ++++++++++++--------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index 890b4e6ee8b..805d3096a71 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -136,7 +136,7 @@ func (s *SpanReader) GetTrace(ctx context.Context, traceID model.TraceID) (*mode span, ctx := opentracing.StartSpanFromContext(ctx, "GetTrace") defer span.Finish() currentTime := time.Now() - traces, err := s.multiRead(ctx, []string{traceID.String()}, currentTime.Add(-s.maxSpanAge), currentTime) + traces, err := s.multiRead(ctx, []model.TraceID{traceID}, currentTime.Add(-s.maxSpanAge), currentTime) if err != nil { return nil, err } @@ -221,7 +221,7 @@ func (s *SpanReader) FindTraces(ctx context.Context, traceQuery *spanstore.Trace span, ctx := opentracing.StartSpanFromContext(ctx, "FindTraces") defer span.Finish() - uniqueTraceIDs, err := s.validateQueryAndFindTraceIDs(ctx, traceQuery) + uniqueTraceIDs, err := s.FindTraceIDs(ctx, traceQuery) if err != nil { return nil, err } @@ -233,7 +233,14 @@ func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.Tra span, ctx := opentracing.StartSpanFromContext(ctx, "FindTraceIDs") defer span.Finish() - esTraceIDs, err := s.validateQueryAndFindTraceIDs(ctx, traceQuery) + if err := validateQuery(traceQuery); err != nil { + return nil, err + } + if traceQuery.NumTraces == 0 { + traceQuery.NumTraces = defaultNumTraces + } + + esTraceIDs, err := s.findTraceIDs(ctx, traceQuery) if err != nil { return nil, err } @@ -251,7 +258,7 @@ func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.Tra return traceIDs, nil } -func (s *SpanReader) multiRead(ctx context.Context, traceIDs []string, startTime, endTime time.Time) ([]*model.Trace, error) { +func (s *SpanReader) multiRead(ctx context.Context, traceIDs []model.TraceID, startTime, endTime time.Time) ([]*model.Trace, error) { childSpan, _ := opentracing.StartSpanFromContext(ctx, "multiRead") childSpan.LogFields(otlog.Object("trace_ids", traceIDs)) @@ -269,9 +276,9 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []string, startTime nextTime := model.TimeAsEpochMicroseconds(startTime.Add(-time.Hour)) - searchAfterTime := make(map[string]uint64) - totalDocumentsFetched := make(map[string]int) - tracesMap := make(map[string]*model.Trace) + searchAfterTime := make(map[model.TraceID]uint64) + totalDocumentsFetched := make(map[model.TraceID]int) + tracesMap := make(map[model.TraceID]*model.Trace) for { if len(traceIDs) == 0 { break @@ -307,18 +314,17 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []string, startTime return nil, err } lastSpan := spans[len(spans)-1] - lastSpanTraceID := lastSpan.TraceID.String() - if traceSpan, ok := tracesMap[lastSpanTraceID]; ok { + if traceSpan, ok := tracesMap[lastSpan.TraceID]; ok { traceSpan.Spans = append(traceSpan.Spans, spans...) } else { - tracesMap[lastSpanTraceID] = &model.Trace{Spans: spans} + tracesMap[lastSpan.TraceID] = &model.Trace{Spans: spans} } - totalDocumentsFetched[lastSpanTraceID] = totalDocumentsFetched[lastSpanTraceID] + len(result.Hits.Hits) - if totalDocumentsFetched[lastSpanTraceID] < int(result.TotalHits()) { - traceIDs = append(traceIDs, lastSpanTraceID) - searchAfterTime[lastSpanTraceID] = model.TimeAsEpochMicroseconds(lastSpan.StartTime) + totalDocumentsFetched[lastSpan.TraceID] = totalDocumentsFetched[lastSpan.TraceID] + len(result.Hits.Hits) + if totalDocumentsFetched[lastSpan.TraceID] < int(result.TotalHits()) { + traceIDs = append(traceIDs, lastSpan.TraceID) + searchAfterTime[lastSpan.TraceID] = model.TimeAsEpochMicroseconds(lastSpan.StartTime) } } } @@ -329,17 +335,6 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []string, startTime return traces, nil } -func (s *SpanReader) validateQueryAndFindTraceIDs(ctx context.Context, traceQuery *spanstore.TraceQueryParameters) ([]string, error) { - if err := validateQuery(traceQuery); err != nil { - return nil, err - } - if traceQuery.NumTraces == 0 { - traceQuery.NumTraces = defaultNumTraces - } - - return s.findTraceIDs(ctx, traceQuery) -} - func validateQuery(p *spanstore.TraceQueryParameters) error { if p == nil { return ErrMalformedRequestObject From 26006349868cb96ff2bc666979a0632edbc5226c Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Wed, 30 Jan 2019 11:38:33 +0400 Subject: [PATCH 10/13] The converting slice of string to slice of mode.TraceID was extracted to "convertTraceStringsToTraceID" function. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 13 +- plugin/storage/es/spanstore/reader_test.go | 178 +++------------------ 2 files changed, 33 insertions(+), 158 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index 784a0ea0a3e..31a0335d5bc 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -275,17 +275,21 @@ func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.Tra return nil, err } - traceIDs := make([]model.TraceID, len(esTraceIDs)) - for i, ID := range esTraceIDs { + return convertTraceStringsToTraceID(esTraceIDs) +} + +func convertTraceStringsToTraceID(traceIDs []string) ([]model.TraceID, error) { + modelTraceIDs := make([]model.TraceID, len(traceIDs)) + for i, ID := range traceIDs { traceID, err := model.TraceIDFromString(ID) if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("Making traceID from string '%s' failed", ID)) } - traceIDs[i] = traceID + modelTraceIDs[i] = traceID } - return traceIDs, nil + return modelTraceIDs, nil } func (s *SpanReader) multiRead(ctx context.Context, traceIDs []model.TraceID, startTime, endTime time.Time) ([]*model.Trace, error) { @@ -395,7 +399,6 @@ func validateQuery(p *spanstore.TraceQueryParameters) error { func (s *SpanReader) findTraceIDs(ctx context.Context, traceQuery *spanstore.TraceQueryParameters) ([]string, error) { childSpan, _ := opentracing.StartSpanFromContext(ctx, "findTraceIDs") defer childSpan.Finish() - // Below is the JSON body to our HTTP GET request to ElasticSearch. This function creates this. // { // "size": 0, diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index afc1f08c7f3..a821c88abfc 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -23,11 +23,11 @@ import ( "testing" "time" - "github.com/uber/jaeger-lib/metrics/metricstest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/uber/jaeger-lib/metrics" + "github.com/uber/jaeger-lib/metrics/metricstest" "go.uber.org/zap" "gopkg.in/olivere/elastic.v5" @@ -145,23 +145,23 @@ func TestSpanReaderIndices(t *testing.T) { dateFormat := date.UTC().Format("2006-01-02") testCases := []struct { indices []string - params SpanReaderParams + params SpanReaderParams }{ - {params:SpanReaderParams{Client:client, Logger: logger, MetricsFactory: metricsFactory, - IndexPrefix:"", Archive: false}, - indices: []string{spanIndex+dateFormat}}, - {params:SpanReaderParams{Client:client, Logger: logger, MetricsFactory: metricsFactory, - IndexPrefix:"foo:", Archive: false}, - indices: []string{"foo:"+indexPrefixSeparator+spanIndex+dateFormat,"foo:"+indexPrefixSeparatorDeprecated+spanIndex+dateFormat}}, - {params:SpanReaderParams{Client:client, Logger: logger, MetricsFactory: metricsFactory, - IndexPrefix:"", Archive: true}, - indices: []string{spanIndex+archiveIndexSuffix}}, - {params:SpanReaderParams{Client:client, Logger: logger, MetricsFactory: metricsFactory, - IndexPrefix:"foo:", Archive: true}, - indices: []string{"foo:"+indexPrefixSeparator+spanIndex+archiveIndexSuffix}}, - {params:SpanReaderParams{Client:client, Logger: logger, MetricsFactory: metricsFactory, - IndexPrefix:"foo:", Archive: true, UseReadWriteAliases:true}, - indices: []string{"foo:"+indexPrefixSeparator+spanIndex+archiveReadIndexSuffix}}, + {params: SpanReaderParams{Client: client, Logger: logger, MetricsFactory: metricsFactory, + IndexPrefix: "", Archive: false}, + indices: []string{spanIndex + dateFormat}}, + {params: SpanReaderParams{Client: client, Logger: logger, MetricsFactory: metricsFactory, + IndexPrefix: "foo:", Archive: false}, + indices: []string{"foo:" + indexPrefixSeparator + spanIndex + dateFormat, "foo:" + indexPrefixSeparatorDeprecated + spanIndex + dateFormat}}, + {params: SpanReaderParams{Client: client, Logger: logger, MetricsFactory: metricsFactory, + IndexPrefix: "", Archive: true}, + indices: []string{spanIndex + archiveIndexSuffix}}, + {params: SpanReaderParams{Client: client, Logger: logger, MetricsFactory: metricsFactory, + IndexPrefix: "foo:", Archive: true}, + indices: []string{"foo:" + indexPrefixSeparator + spanIndex + archiveIndexSuffix}}, + {params: SpanReaderParams{Client: client, Logger: logger, MetricsFactory: metricsFactory, + IndexPrefix: "foo:", Archive: true, UseReadWriteAliases: true}, + indices: []string{"foo:" + indexPrefixSeparator + spanIndex + archiveReadIndexSuffix}}, } for _, testCase := range testCases { r := NewSpanReader(testCase.params) @@ -702,143 +702,15 @@ func TestFindTraceIDs(t *testing.T) { testGet(traceIDAggregation, t) } -func TestSpanReader_TestFindTraceIDs(t *testing.T) { - goodAggregations := make(map[string]*json.RawMessage) - rawMessage := []byte(`{"buckets": [{"key": "1","doc_count": 16},{"key": "2","doc_count": 16},{"key": "3","doc_count": 16}]}`) - goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) - - withSpanReader(func(r *spanReaderTest) { - // find trace IDs - mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) - - traceIDsQuery := &spanstore.TraceQueryParameters{ - ServiceName: serviceName, - Tags: map[string]string{ - "hello": "world", - }, - StartTimeMin: time.Now().Add(-1 * time.Hour), - StartTimeMax: time.Now(), - NumTraces: 3, - } - - traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) - require.NoError(t, err) - assert.Len(t, traceIDs, 3) - assert.EqualValues(t, 1, traceIDs[0].Low) - }) -} - -func TestSpanReader_FindTraceIDsInvalidQuery(t *testing.T) { - goodAggregations := make(map[string]*json.RawMessage) - rawMessage := []byte(`{"buckets": [{"key": "1","doc_count": 16},{"key": "2","doc_count": 16},{"key": "3","doc_count": 16}]}`) - goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) - - withSpanReader(func(r *spanReaderTest) { - mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) - - traceIDsQuery := &spanstore.TraceQueryParameters{ - ServiceName: "", - Tags: map[string]string{ - "hello": "world", - }, - StartTimeMin: time.Now().Add(-1 * time.Hour), - StartTimeMax: time.Now(), - } - - traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) - require.Error(t, err) - assert.Nil(t, traceIDs) - }) -} - -func TestSpanReader_FindTraceIDsAggregationFailure(t *testing.T) { - goodAggregations := make(map[string]*json.RawMessage) - - withSpanReader(func(r *spanReaderTest) { - mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) +func TestTraceStringsToTraceIDConvertation(t *testing.T) { + traceIDs, err := convertTraceStringsToTraceID([]string{"1", "2", "3"}) + assert.NoError(t, err) + assert.Equal(t, 3, len(traceIDs)) + assert.Equal(t, "1", traceIDs[0].String()) - traceIDsQuery := &spanstore.TraceQueryParameters{ - ServiceName: serviceName, - Tags: map[string]string{ - "hello": "world", - }, - StartTimeMin: time.Now().Add(-1 * time.Hour), - StartTimeMax: time.Now(), - } - - traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) - require.Error(t, err) - assert.Nil(t, traceIDs) - }) -} - -func TestSpanReader_FindTraceIDsNoTraceIDs(t *testing.T) { - goodAggregations := make(map[string]*json.RawMessage) - rawMessage := []byte(`{"buckets": []}`) - goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) - - withSpanReader(func(r *spanReaderTest) { - mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) - - traceIDsQuery := &spanstore.TraceQueryParameters{ - ServiceName: serviceName, - Tags: map[string]string{ - "hello": "world", - }, - StartTimeMin: time.Now().Add(-1 * time.Hour), - StartTimeMax: time.Now(), - } - - traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) - require.NoError(t, err) - assert.Len(t, traceIDs, 0) - }) -} - -func TestSpanReader_FindTraceIDsReadTraceIDsFailure(t *testing.T) { - goodAggregations := make(map[string]*json.RawMessage) - rawMessage := []byte(`{"buckets": [{"key": "1","doc_count": 16},{"key": "2","doc_count": 16}]}`) - goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) - - withSpanReader(func(r *spanReaderTest) { - mockSearchService(r).Return(nil, errors.New("read error")) - - traceIDsQuery := &spanstore.TraceQueryParameters{ - ServiceName: serviceName, - Tags: map[string]string{ - "hello": "world", - }, - StartTimeMin: time.Now().Add(-1 * time.Hour), - StartTimeMax: time.Now(), - } - - traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) - require.EqualError(t, err, "Search service failed: read error") - assert.Len(t, traceIDs, 0) - }) -} - -func TestSpanReader_FindTraceIDsIncorrectTraceIDFailure(t *testing.T) { - goodAggregations := make(map[string]*json.RawMessage) - rawMessage := []byte(`{"buckets": [{"key": "sdfsdfdssd234nsdvsdfldjsf","doc_count": 16},{"key": "2","doc_count": 16}]}`) - goodAggregations[traceIDAggregation] = (*json.RawMessage)(&rawMessage) - - withSpanReader(func(r *spanReaderTest) { - mockSearchService(r).Return(&elastic.SearchResult{Aggregations: elastic.Aggregations(goodAggregations)}, nil) - - traceIDsQuery := &spanstore.TraceQueryParameters{ - ServiceName: serviceName, - Tags: map[string]string{ - "hello": "world", - }, - StartTimeMin: time.Now().Add(-1 * time.Hour), - StartTimeMax: time.Now(), - } - - traceIDs, err := r.reader.FindTraceIDs(context.Background(), traceIDsQuery) - require.EqualError(t, err, `Making traceID from string 'sdfsdfdssd234nsdvsdfldjsf' failed: strconv.ParseUint: parsing "sdfsdfdss": invalid syntax`) - assert.Len(t, traceIDs, 0) - }) + traceIDs, err = convertTraceStringsToTraceID([]string{"dsfjsdklfjdsofdfsdbfkgbgoaemlrksdfbsdofgerjl"}) + assert.EqualError(t, err, "Making traceID from string 'dsfjsdklfjdsofdfsdbfkgbgoaemlrksdfbsdofgerjl' failed: TraceID cannot be longer than 32 hex characters: dsfjsdklfjdsofdfsdbfkgbgoaemlrksdfbsdofgerjl") + assert.Equal(t, 0, len(traceIDs)) } func mockMultiSearchService(r *spanReaderTest) *mock.Call { From b10880987b60bcf8ec565240187c7227b318fca8 Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Wed, 30 Jan 2019 11:51:48 +0400 Subject: [PATCH 11/13] The method "convertTraceIDsModelsToStrings" was added to convert slice of traceID models to slice of strings. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 41 +++++++++++++--------- plugin/storage/es/spanstore/reader_test.go | 16 +++++++-- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index 31a0335d5bc..df1de4117e1 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -275,27 +275,13 @@ func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.Tra return nil, err } - return convertTraceStringsToTraceID(esTraceIDs) -} - -func convertTraceStringsToTraceID(traceIDs []string) ([]model.TraceID, error) { - modelTraceIDs := make([]model.TraceID, len(traceIDs)) - for i, ID := range traceIDs { - traceID, err := model.TraceIDFromString(ID) - if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("Making traceID from string '%s' failed", ID)) - } - - modelTraceIDs[i] = traceID - } - - return modelTraceIDs, nil + return convertTraceIDsStringsToModels(esTraceIDs) } func (s *SpanReader) multiRead(ctx context.Context, traceIDs []model.TraceID, startTime, endTime time.Time) ([]*model.Trace, error) { childSpan, _ := opentracing.StartSpanFromContext(ctx, "multiRead") - childSpan.LogFields(otlog.Object("trace_ids", traceIDs)) + childSpan.LogFields(otlog.Object("trace_ids", convertTraceIDsModelsToStrings(traceIDs))) defer childSpan.Finish() if len(traceIDs) == 0 { @@ -377,6 +363,29 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []model.TraceID, st return traces, nil } +func convertTraceIDsStringsToModels(traceIDs []string) ([]model.TraceID, error) { + traceIDsModels := make([]model.TraceID, len(traceIDs)) + for i, ID := range traceIDs { + traceID, err := model.TraceIDFromString(ID) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("Making traceID from string '%s' failed", ID)) + } + + traceIDsModels[i] = traceID + } + + return traceIDsModels, nil +} + +func convertTraceIDsModelsToStrings(traceIDs []model.TraceID) []string { + traceIDsStrings := make([]string, len(traceIDs)) + for i, traceID := range traceIDs { + traceIDsStrings[i] = traceID.String() + } + + return traceIDsStrings +} + func validateQuery(p *spanstore.TraceQueryParameters) error { if p == nil { return ErrMalformedRequestObject diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index a821c88abfc..dbabbb92f65 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -702,17 +702,27 @@ func TestFindTraceIDs(t *testing.T) { testGet(traceIDAggregation, t) } -func TestTraceStringsToTraceIDConvertation(t *testing.T) { - traceIDs, err := convertTraceStringsToTraceID([]string{"1", "2", "3"}) +func TestTraceIDsStringsToModelsConvertation(t *testing.T) { + traceIDs, err := convertTraceIDsStringsToModels([]string{"1", "2", "3"}) assert.NoError(t, err) assert.Equal(t, 3, len(traceIDs)) assert.Equal(t, "1", traceIDs[0].String()) - traceIDs, err = convertTraceStringsToTraceID([]string{"dsfjsdklfjdsofdfsdbfkgbgoaemlrksdfbsdofgerjl"}) + traceIDs, err = convertTraceIDsStringsToModels([]string{"dsfjsdklfjdsofdfsdbfkgbgoaemlrksdfbsdofgerjl"}) assert.EqualError(t, err, "Making traceID from string 'dsfjsdklfjdsofdfsdbfkgbgoaemlrksdfbsdofgerjl' failed: TraceID cannot be longer than 32 hex characters: dsfjsdklfjdsofdfsdbfkgbgoaemlrksdfbsdofgerjl") assert.Equal(t, 0, len(traceIDs)) } +func TestTraceIDsModelsToStringsConvertation(t *testing.T) { + traceID1 := model.NewTraceID(0, 1) + traceID2 := model.NewTraceID(0, 2) + traceID3 := model.NewTraceID(0, 3) + + traceIDs := convertTraceIDsModelsToStrings([]model.TraceID{traceID1, traceID2, traceID3}) + assert.Equal(t, 3, len(traceIDs)) + assert.Equal(t, "3", traceIDs[2]) +} + func mockMultiSearchService(r *spanReaderTest) *mock.Call { multiSearchService := &mocks.MultiSearchService{} multiSearchService.On("Add", mock.Anything, mock.Anything, mock.Anything).Return(multiSearchService) From debd4d644c6f71e163901e2001997586ecb12cd7 Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Wed, 30 Jan 2019 13:29:03 +0400 Subject: [PATCH 12/13] The fix of passing traceID into NewTermQuery function. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index df1de4117e1..58ded897bb7 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -304,7 +304,7 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []model.TraceID, st } for i, traceID := range traceIDs { - query := elastic.NewTermQuery("traceID", traceID) + query := elastic.NewTermQuery("traceID", traceID.String()) if val, ok := searchAfterTime[traceID]; ok { nextTime = val } From 75c30d33db9ae836f1a0b4704f2ecae97b06c466 Mon Sep 17 00:00:00 2001 From: vlamug <vmugultyanov@ozon.ru> Date: Thu, 31 Jan 2019 11:25:01 +0400 Subject: [PATCH 13/13] The convertTraceIDsModelsToStrings method was remove, because it is redundant. Signed-off-by: vlamug <vmugultyanov@ozon.ru> --- plugin/storage/es/spanstore/reader.go | 11 +---------- plugin/storage/es/spanstore/reader_test.go | 12 +----------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index 58ded897bb7..9afd69d03ca 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -281,7 +281,7 @@ func (s *SpanReader) FindTraceIDs(ctx context.Context, traceQuery *spanstore.Tra func (s *SpanReader) multiRead(ctx context.Context, traceIDs []model.TraceID, startTime, endTime time.Time) ([]*model.Trace, error) { childSpan, _ := opentracing.StartSpanFromContext(ctx, "multiRead") - childSpan.LogFields(otlog.Object("trace_ids", convertTraceIDsModelsToStrings(traceIDs))) + childSpan.LogFields(otlog.Object("trace_ids", traceIDs)) defer childSpan.Finish() if len(traceIDs) == 0 { @@ -377,15 +377,6 @@ func convertTraceIDsStringsToModels(traceIDs []string) ([]model.TraceID, error) return traceIDsModels, nil } -func convertTraceIDsModelsToStrings(traceIDs []model.TraceID) []string { - traceIDsStrings := make([]string, len(traceIDs)) - for i, traceID := range traceIDs { - traceIDsStrings[i] = traceID.String() - } - - return traceIDsStrings -} - func validateQuery(p *spanstore.TraceQueryParameters) error { if p == nil { return ErrMalformedRequestObject diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index dbabbb92f65..080b560fbec 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -702,7 +702,7 @@ func TestFindTraceIDs(t *testing.T) { testGet(traceIDAggregation, t) } -func TestTraceIDsStringsToModelsConvertation(t *testing.T) { +func TestTraceIDsStringsToModelsConversion(t *testing.T) { traceIDs, err := convertTraceIDsStringsToModels([]string{"1", "2", "3"}) assert.NoError(t, err) assert.Equal(t, 3, len(traceIDs)) @@ -713,16 +713,6 @@ func TestTraceIDsStringsToModelsConvertation(t *testing.T) { assert.Equal(t, 0, len(traceIDs)) } -func TestTraceIDsModelsToStringsConvertation(t *testing.T) { - traceID1 := model.NewTraceID(0, 1) - traceID2 := model.NewTraceID(0, 2) - traceID3 := model.NewTraceID(0, 3) - - traceIDs := convertTraceIDsModelsToStrings([]model.TraceID{traceID1, traceID2, traceID3}) - assert.Equal(t, 3, len(traceIDs)) - assert.Equal(t, "3", traceIDs[2]) -} - func mockMultiSearchService(r *spanReaderTest) *mock.Call { multiSearchService := &mocks.MultiSearchService{} multiSearchService.On("Add", mock.Anything, mock.Anything, mock.Anything).Return(multiSearchService)