[query] Enable trace adjusters in api_v2 and api_v3 handlers (jaegert…

…racing#6423)

## Which problem is this PR solving?
- Resolves jaegertracing#6417

## Description of the changes
- 🛑 This PR contains a breaking change for the api_v2 handler.
- All
[GetTrace](https://github.com/jaegertracing/jaeger-idl/blob/main/proto/api_v2/query.proto#L37)
and
[FindTraces](https://github.com/jaegertracing/jaeger-idl/blob/main/proto/api_v2/query.proto#L125)
requests will apply
[these](https://github.com/jaegertracing/jaeger/blob/main/cmd/query/app/querysvc/adjusters.go#L17-L24)
adjusters/enrichments on the trace by default.
- In order to disable the adjusters for the `FindTraces` request, set
the
[raw_traces](https://github.com/jaegertracing/jaeger-idl/blob/main/proto/api_v2/query.proto#L122)
flag in the query parameters to `true`.
- In order to disable the adjusters for the `GetTrace` request, set the
[raw_traces](https://github.com/jaegertracing/jaeger-idl/blob/main/proto/api_v2/query.proto#L56)
flag in the request to `true`.
- 🛑 This PR contains a breaking change for the api_v3 handler.
- All
[GetTrace](https://github.com/jaegertracing/jaeger-idl/blob/main/proto/api_v3/query_service.proto#L27)
and
[FindTraces](https://github.com/jaegertracing/jaeger-idl/blob/main/proto/api_v3/query_service.proto#L88)
requests will apply
[these](https://github.com/jaegertracing/jaeger/blob/main/cmd/query/app/querysvc/adjusters.go#L17-L24)
adjusters/enrichments on the trace by default.
- In order to disable the adjusters for the `FindTraces` request, set
the
[raw_traces](https://github.com/jaegertracing/jaeger-idl/blob/main/proto/api_v3/query_service.proto#L84)
flag in the query parameters to `true`.
- In order to disable the adjusters for the `GetTrace` request, set the
[raw_traces](https://github.com/jaegertracing/jaeger-idl/blob/main/proto/api_v3/query_service.proto#L40)
flag in the request to `true`.

## How was this change tested?
- Added 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: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>

cmd/query/app/apiv3/gateway_test.go

@@ -93,6 +93,7 @@ func makeTestTrace() (*model.Trace, spanstore.GetTraceParameters) {
 					model.SpanKindTag(model.SpanKindServer),
 					model.Bool("error", true),
 				},
+				Process: &model.Process{},
 			},
 		},
 	}, query

cmd/query/app/apiv3/grpc_handler_test.go

@@ -116,6 +116,7 @@ func TestGetTrace(t *testing.T) {
 				&model.Trace{
 					Spans: []*model.Span{
 						{
+							Process:       &model.Process{},
 							OperationName: "foobar",
 						},
 					},
@@ -172,6 +173,7 @@ func TestFindTraces(t *testing.T) {
 			{
 				Spans: []*model.Span{
 					{
+						Process:       &model.Process{},
 						OperationName: "name",
 					},
 				},
@@ -204,6 +206,9 @@ func TestFindTracesSendError(t *testing.T) {
 			{
 				Spans: []*model.Span{
 					{
+						Process: &model.Process{
+							ServiceName: "myservice",
+						},
 						OperationName: "name",
 					},
 				},

cmd/query/app/http_handler.go

@@ -188,7 +188,7 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var uiErrors []structuredError
-	structuredRes := aH.tracesToResponse(traces, false, uiErrors)
+	structuredRes := aH.tracesToResponse(traces, uiErrors)
 	aH.writeJSON(w, r, structuredRes)
 }
 
@@ -245,14 +245,14 @@ func (aH *APIHandler) search(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	structuredRes := aH.tracesToResponse(tracesFromStorage, true, uiErrors)
+	structuredRes := aH.tracesToResponse(tracesFromStorage, uiErrors)
 	aH.writeJSON(w, r, structuredRes)
 }
 
-func (aH *APIHandler) tracesToResponse(traces []*model.Trace, adjust bool, uiErrors []structuredError) *structuredResponse {
+func (*APIHandler) tracesToResponse(traces []*model.Trace, uiErrors []structuredError) *structuredResponse {
 	uiTraces := make([]*ui.Trace, len(traces))
 	for i, v := range traces {
-		uiTrace := aH.convertModelToUI(v, adjust)
+		uiTrace := uiconv.FromDomain(v)
 		uiTraces[i] = uiTrace
 	}
 
@@ -362,14 +362,6 @@ func (aH *APIHandler) metrics(w http.ResponseWriter, r *http.Request, getMetrics
 	aH.writeJSON(w, r, m)
 }
 
-func (aH *APIHandler) convertModelToUI(trc *model.Trace, adjust bool) *ui.Trace {
-	if adjust {
-		aH.queryService.Adjust(trc)
-	}
-	uiTrace := uiconv.FromDomain(trc)
-	return uiTrace
-}
-
 func (*APIHandler) deduplicateDependencies(dependencies []model.DependencyLink) []ui.DependencyLink {
 	type Key struct {
 		parent string
@@ -483,16 +475,10 @@ func (aH *APIHandler) getTrace(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var uiErrors []structuredError
-	structuredRes := aH.tracesToResponse([]*model.Trace{trc}, shouldAdjust(r), uiErrors)
+	structuredRes := aH.tracesToResponse([]*model.Trace{trc}, uiErrors)
 	aH.writeJSON(w, r, structuredRes)
 }
 
-func shouldAdjust(r *http.Request) bool {
-	raw := r.FormValue("raw")
-	isRaw, _ := strconv.ParseBool(raw)
-	return !isRaw
-}
-
 // archiveTrace implements the REST API POST:/archive/{trace-id}.
 // It passes the traceID to queryService.ArchiveTrace for writing.
 func (aH *APIHandler) archiveTrace(w http.ResponseWriter, r *http.Request) {

cmd/query/app/querysvc/query_service.go

@@ -86,7 +86,13 @@ func (qs QueryService) GetTrace(ctx context.Context, query GetTraceParameters) (
 		}
 		trace, err = qs.options.ArchiveSpanReader.GetTrace(ctx, query.GetTraceParameters)
 	}
-	return trace, err
+	if err != nil {
+		return nil, err
+	}
+	if !query.RawTraces {
+		qs.adjust(trace)
+	}
+	return trace, nil
 }
 
 // GetServices is the queryService implementation of spanstore.Reader.GetServices
@@ -116,7 +122,16 @@ func (qs QueryService) FindTraces(ctx context.Context, query *TraceQueryParamete
 	if err != nil {
 		return nil, err
 	}
-	return spanReader.FindTraces(ctx, &query.TraceQueryParameters)
+	traces, err := spanReader.FindTraces(ctx, &query.TraceQueryParameters)
+	if err != nil {
+		return nil, err
+	}
+	if !query.RawTraces {
+		for _, trace := range traces {
+			qs.adjust(trace)
+		}
+	}
+	return traces, nil
 }
 
 // ArchiveTrace is the queryService utility to archive traces.
@@ -140,7 +155,7 @@ func (qs QueryService) ArchiveTrace(ctx context.Context, query spanstore.GetTrac
 }
 
 // Adjust applies adjusters to the trace.
-func (qs QueryService) Adjust(trace *model.Trace) {
+func (qs QueryService) adjust(trace *model.Trace) {
 	qs.options.Adjuster.Adjust(trace)
 }
 

cmd/query/app/querysvc/query_service_test.go

@@ -6,6 +6,7 @@ package querysvc
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 	"time"
 
@@ -115,6 +116,68 @@ func TestGetTraceSuccess(t *testing.T) {
 	assert.Equal(t, res, mockTrace)
 }
 
+func TestGetTraceWithRawTraces(t *testing.T) {
+	traceID := model.NewTraceID(0, 1)
+	tests := []struct {
+		rawTraces bool
+		tags      model.KeyValues
+		expected  model.KeyValues
+	}{
+		{
+			// tags should not get sorted by SortTagsAndLogFields adjuster
+			rawTraces: true,
+			tags: model.KeyValues{
+				model.String("z", "key"),
+				model.String("a", "key"),
+			},
+			expected: model.KeyValues{
+				model.String("z", "key"),
+				model.String("a", "key"),
+			},
+		},
+		{
+			// tags should get sorted by SortTagsAndLogFields adjuster
+			rawTraces: false,
+			tags: model.KeyValues{
+				model.String("z", "key"),
+				model.String("a", "key"),
+			},
+			expected: model.KeyValues{
+				model.String("a", "key"),
+				model.String("z", "key"),
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("rawTraces=%v", test.rawTraces), func(t *testing.T) {
+			trace := &model.Trace{
+				Spans: []*model.Span{
+					{
+						TraceID: traceID,
+						SpanID:  model.NewSpanID(1),
+						Process: &model.Process{},
+						Tags:    test.tags,
+					},
+				},
+			}
+			tqs := initializeTestService()
+			tqs.spanReader.On("GetTrace", mock.Anything, mock.AnythingOfType("spanstore.GetTraceParameters")).
+				Return(trace, nil).Once()
+			query := GetTraceParameters{
+				GetTraceParameters: spanstore.GetTraceParameters{
+					TraceID: mockTraceID,
+				},
+				RawTraces: test.rawTraces,
+			}
+			gotTrace, err := tqs.queryService.GetTrace(context.Background(), query)
+			require.NoError(t, err)
+			spans := gotTrace.Spans
+			require.Len(t, spans, 1)
+			require.EqualValues(t, test.expected, spans[0].Tags)
+		})
+	}
+}
+
 // Test QueryService.GetTrace() without ArchiveSpanReader
 func TestGetTraceNotFound(t *testing.T) {
 	tqs := initializeTestService()
@@ -239,6 +302,66 @@ func TestFindTraces(t *testing.T) {
 	assert.Len(t, traces, 1)
 }
 
+func TestFindTracesWithRawTraces(t *testing.T) {
+	traceID := model.NewTraceID(0, 1)
+	tests := []struct {
+		rawTraces bool
+		tags      model.KeyValues
+		expected  model.KeyValues
+	}{
+		{
+			// tags should not get sorted by SortTagsAndLogFields adjuster
+			rawTraces: true,
+			tags: model.KeyValues{
+				model.String("z", "key"),
+				model.String("a", "key"),
+			},
+			expected: model.KeyValues{
+				model.String("z", "key"),
+				model.String("a", "key"),
+			},
+		},
+		{
+			// tags should get sorted by SortTagsAndLogFields adjuster
+			rawTraces: false,
+			tags: model.KeyValues{
+				model.String("z", "key"),
+				model.String("a", "key"),
+			},
+			expected: model.KeyValues{
+				model.String("a", "key"),
+				model.String("z", "key"),
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("rawTraces=%v", test.rawTraces), func(t *testing.T) {
+			trace := &model.Trace{
+				Spans: []*model.Span{
+					{
+						TraceID: traceID,
+						SpanID:  model.NewSpanID(1),
+						Process: &model.Process{},
+						Tags:    test.tags,
+					},
+				},
+			}
+			tqs := initializeTestService()
+			tqs.spanReader.On("FindTraces", mock.Anything, mock.AnythingOfType("*spanstore.TraceQueryParameters")).
+				Return([]*model.Trace{trace}, nil).Once()
+			params := &TraceQueryParameters{
+				RawTraces: test.rawTraces,
+			}
+			traces, err := tqs.queryService.FindTraces(context.Background(), params)
+			require.NoError(t, err)
+			require.Len(t, traces, 1)
+			spans := traces[0].Spans
+			require.Len(t, spans, 1)
+			require.EqualValues(t, test.expected, spans[0].Tags)
+		})
+	}
+}
+
 func TestFindTraces_V1ReaderNotFound(t *testing.T) {
 	fr := new(tracestoremocks.Reader)
 	qs := QueryService{
@@ -259,6 +382,15 @@ func TestFindTraces_V1ReaderNotFound(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestFindTracesError(t *testing.T) {
+	tqs := initializeTestService()
+	tqs.spanReader.On("FindTraces", mock.Anything, mock.AnythingOfType("*spanstore.TraceQueryParameters")).
+		Return(nil, assert.AnError).Once()
+	traces, err := tqs.queryService.FindTraces(context.Background(), &TraceQueryParameters{})
+	require.ErrorIs(t, err, assert.AnError)
+	require.Nil(t, traces)
+}
+
 // Test QueryService.ArchiveTrace() with no ArchiveSpanWriter.
 func TestArchiveTraceNoOptions(t *testing.T) {
 	tqs := initializeTestService()