From 084ec66b9ceb764b6d67aa6c87c173e26059869a Mon Sep 17 00:00:00 2001 From: Manik2708 Date: Tue, 21 Jan 2025 19:41:56 +0530 Subject: [PATCH] minor fixes Signed-off-by: Manik2708 --- plugin/storage/es/spanstore/reader.go | 2 +- plugin/storage/es/spanstore/reader_test.go | 28 +++++++++---------- .../storage/es/spanstore/service_operation.go | 28 +++++++++---------- .../es/spanstore/service_operation_test.go | 4 +-- 4 files changed, 30 insertions(+), 32 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index a930001834e3..1ce849c0d827 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -298,7 +298,7 @@ func (s *SpanReader) GetOperations( currentTime, cfg.RolloverFrequencyAsNegativeDuration(s.serviceIndex.RolloverFrequency), ) - operations, err := s.serviceOperationStorage.getOperations(ctx, jaegerIndices, query.ServiceName, query.SpanKind, s.maxDocCount) + operations, err := s.serviceOperationStorage.getOperationsWithKind(ctx, jaegerIndices, query.ServiceName, query.SpanKind, s.maxDocCount) if err != nil { return nil, err } diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index 16e2cc9dc953..b1bda69ac92d 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -629,7 +629,7 @@ func testGetWithKind(typ string, t *testing.T, testKind bool) { rawMessage := []byte(`{"buckets": [{"key": "123","doc_count": 16}]}`) goodAggregations[typ] = (*json.RawMessage)(&rawMessage) var filterRawMessage json.RawMessage - if typ == operationsAggregation { + if typ == operationsAggName { if testKind { filterRawMessage = rawMessage } @@ -646,7 +646,7 @@ func testGetWithKind(typ string, t *testing.T, testKind bool) { caption: typ + " search error", searchError: errors.New("Search failure"), expectedError: func() string { - if typ == operationsAggregation { + if typ == operationsAggName { return "search operations failed: Search failure" } return "search services failed: Search failure" @@ -654,7 +654,7 @@ func testGetWithKind(typ string, t *testing.T, testKind bool) { }, } - if (typ == operationsAggregation && testKind) || (typ != operationsAggregation) { + if (typ == operationsAggName && testKind) || (typ != operationsAggName) { testCase := testGetStruct{ caption: typ + " search error", searchResult: &elastic.SearchResult{Aggregations: elastic.Aggregations(badAggregations)}, @@ -670,8 +670,8 @@ func testGetWithKind(typ string, t *testing.T, testKind bool) { caption: typ + " full behavior with kind", searchResult: &elastic.SearchResult{Aggregations: goodAggregations}, expectedOutput: map[string]any{ - operationsAggregation: []spanstore.Operation{{Name: "123", SpanKind: "server"}}, - "default": []string{"123"}, + operationsAggName: []spanstore.Operation{{Name: "123", SpanKind: "server"}}, + "default": []string{"123"}, }, expectedError: func() string { return "" @@ -679,7 +679,7 @@ func testGetWithKind(typ string, t *testing.T, testKind bool) { }) } - if typ == operationsAggregation && !testKind { + if typ == operationsAggName && !testKind { score := 0.6931471 msg := json.RawMessage(`{"operationName": "123"}`) hitModel := &elastic.SearchHits{ @@ -699,8 +699,8 @@ func testGetWithKind(typ string, t *testing.T, testKind bool) { caption: typ + " full behavior", searchResult: &elastic.SearchResult{Hits: hitModel}, expectedOutput: map[string]any{ - operationsAggregation: []spanstore.Operation{{Name: "123"}}, - "default": []string{"123"}, + operationsAggName: []spanstore.Operation{{Name: "123"}}, + "default": []string{"123"}, }, expectedError: func() string { return "" @@ -708,13 +708,13 @@ func testGetWithKind(typ string, t *testing.T, testKind bool) { }) } - if typ != operationsAggregation { + if typ != operationsAggName { testCases = append(testCases, testGetStruct{ caption: typ + " full behavior", searchResult: &elastic.SearchResult{Aggregations: goodAggregations}, expectedOutput: map[string]any{ - operationsAggregation: []spanstore.Operation{{Name: "123"}}, - "default": []string{"123"}, + operationsAggName: []spanstore.Operation{{Name: "123"}}, + "default": []string{"123"}, }, expectedError: func() string { return "" @@ -749,7 +749,7 @@ func returnSearchFunc(typ string, r *spanReaderTest, testKind bool) (any, error) switch typ { case servicesAggregation: return r.reader.GetServices(context.Background()) - case operationsAggregation: + case operationsAggName: if testKind { return r.reader.GetOperations( context.Background(), @@ -1025,7 +1025,7 @@ func TestFindTraceIDs(t *testing.T) { }{ {traceIDAggregation}, {servicesAggregation}, - {operationsAggregation}, + {operationsAggName}, } for _, testCase := range testCases { t.Run(testCase.aggregrationID, func(t *testing.T) { @@ -1083,7 +1083,7 @@ func mockSearchServiceWithSpanKind(r *spanReaderTest, inputHasSpanKind bool) *mo searchService.On("IgnoreUnavailable", mock.AnythingOfType("bool")).Return(searchService) searchService.On("Aggregation", stringMatcher(servicesAggregation), mock.MatchedBy(matchTermsAggregation)).Return(searchService) if inputHasSpanKind { - searchService.On("Aggregation", stringMatcher(operationsAggregation), mock.MatchedBy(matchTermsAggregation)).Return(searchService) + searchService.On("Aggregation", stringMatcher(operationsAggName), mock.MatchedBy(matchTermsAggregation)).Return(searchService) } else { searchService.On("FetchSourceContext", mock.AnythingOfType("*elastic.FetchSourceContext"), mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(searchService) searchService.On("Size", mock.AnythingOfType("int")).Return(searchService) diff --git a/plugin/storage/es/spanstore/service_operation.go b/plugin/storage/es/spanstore/service_operation.go index 93a1a07c61f1..6eb748041815 100644 --- a/plugin/storage/es/spanstore/service_operation.go +++ b/plugin/storage/es/spanstore/service_operation.go @@ -24,10 +24,10 @@ import ( ) const ( - serviceName = "serviceName" - spanKind = "spanKind" - operationsAggregation = "distinct_operations" - servicesAggregation = "distinct_services" + serviceName = "serviceName" + spanKindField = "spanKindField" + operationsAggName = "distinct_operations" + servicesAggregation = "distinct_services" ) // ServiceOperationStorage stores service to operation pairs. @@ -100,18 +100,16 @@ func getServicesAggregation(maxDocCount int) elastic.Query { Size(maxDocCount) // ES deprecated size omission for aggregating all. https://github.com/elastic/elasticsearch/issues/18838 } -func (s *ServiceOperationStorage) getOperations(ctx context.Context, indices []string, service, kind string, maxDocCount int) ([]spanstore.Operation, error) { +func (s *ServiceOperationStorage) getOperationsWithKind(ctx context.Context, indices []string, service, kind string, maxDocCount int) ([]spanstore.Operation, error) { var searchService es.SearchService if kind != "" { - serviceFilter := getOperationsAggregation(maxDocCount) - serviceNameQuery := elastic.NewTermQuery(serviceName, service) - serviceKindQuery := elastic.NewTermQuery(spanKind, kind) - serviceQuery := elastic.NewBoolQuery().Must(serviceNameQuery, serviceKindQuery) searchService = s.client().Search(indices...). - Size(0). - Query(serviceQuery). + Size(0). // set to 0 because we don't want actual documents. + Query(elastic.NewBoolQuery().Must( + elastic.NewTermQuery(serviceName, service), + elastic.NewTermQuery(spanKindField, kind))). IgnoreUnavailable(true). - Aggregation(operationsAggregation, serviceFilter) + Aggregation(operationsAggName, getOperationsAggregation(maxDocCount)) searchResult, err := searchService.Do(ctx) if err != nil { return nil, fmt.Errorf("search operations failed: %w", es.DetailedError(err)) @@ -119,9 +117,9 @@ func (s *ServiceOperationStorage) getOperations(ctx context.Context, indices []s if searchResult.Aggregations == nil { return []spanstore.Operation{}, nil } - bucket, found := searchResult.Aggregations.Terms(operationsAggregation) + bucket, found := searchResult.Aggregations.Terms(operationsAggName) if !found { - return nil, errors.New("could not find aggregation of " + operationsAggregation) + return nil, errors.New("could not find aggregation of " + operationsAggName) } operationNamesBucket := bucket.Buckets return bucketOfOperationNamesToOperationsArray(operationNamesBucket, kind) @@ -130,7 +128,7 @@ func (s *ServiceOperationStorage) getOperations(ctx context.Context, indices []s searchService = s.client().Search(indices...). Query(serviceQuery). IgnoreUnavailable(true). - FetchSourceContext(elastic.NewFetchSourceContext(true).Include(spanKind, operationNameField)). + FetchSourceContext(elastic.NewFetchSourceContext(true).Include(spanKindField, operationNameField)). Size(maxDocCount) searchResult, err := searchService.Do(ctx) if err != nil { diff --git a/plugin/storage/es/spanstore/service_operation_test.go b/plugin/storage/es/spanstore/service_operation_test.go index 17dbb3b7aa8e..8cc790d3ad9f 100644 --- a/plugin/storage/es/spanstore/service_operation_test.go +++ b/plugin/storage/es/spanstore/service_operation_test.go @@ -122,8 +122,8 @@ func TestSpanReader_GetServices(t *testing.T) { } func TestSpanReader_GetOperations(t *testing.T) { - testGet(operationsAggregation, t) - testGetWithKind(operationsAggregation, t, true) + testGet(operationsAggName, t) + testGetWithKind(operationsAggName, t, true) } func TestSpanReader_GetServicesEmptyIndex(t *testing.T) {