Skip to content

Commit 7117fa8

Browse files
mahadzaryab1mahadzaryab1
authored
[query] Enable trace adjusters in api_v2 and api_v3 handlers (#6423)
## Which problem is this PR solving? - Resolves #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]>
1 parent a6616fb commit 7117fa8

File tree

5 files changed

+161
-22
lines changed

5 files changed

+161
-22
lines changed

‎cmd/query/app/apiv3/gateway_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ func makeTestTrace() (*model.Trace, spanstore.GetTraceParameters) {
9393
model.SpanKindTag(model.SpanKindServer),
9494
model.Bool("error", true),
9595
},
96+
Process: &model.Process{},
9697
},
9798
},
9899
}, query

‎cmd/query/app/apiv3/grpc_handler_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ func TestGetTrace(t *testing.T) {
116116
&model.Trace{
117117
Spans: []*model.Span{
118118
{
119+
Process: &model.Process{},
119120
OperationName: "foobar",
120121
},
121122
},
@@ -172,6 +173,7 @@ func TestFindTraces(t *testing.T) {
172173
{
173174
Spans: []*model.Span{
174175
{
176+
Process: &model.Process{},
175177
OperationName: "name",
176178
},
177179
},
@@ -204,6 +206,9 @@ func TestFindTracesSendError(t *testing.T) {
204206
{
205207
Spans: []*model.Span{
206208
{
209+
Process: &model.Process{
210+
ServiceName: "myservice",
211+
},
207212
OperationName: "name",
208213
},
209214
},

‎cmd/query/app/http_handler.go

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) {
188188
}
189189

190190
var uiErrors []structuredError
191-
structuredRes := aH.tracesToResponse(traces, false, uiErrors)
191+
structuredRes := aH.tracesToResponse(traces, uiErrors)
192192
aH.writeJSON(w, r, structuredRes)
193193
}
194194

@@ -245,14 +245,14 @@ func (aH *APIHandler) search(w http.ResponseWriter, r *http.Request) {
245245
}
246246
}
247247

248-
structuredRes := aH.tracesToResponse(tracesFromStorage, true, uiErrors)
248+
structuredRes := aH.tracesToResponse(tracesFromStorage, uiErrors)
249249
aH.writeJSON(w, r, structuredRes)
250250
}
251251

252-
func (aH *APIHandler) tracesToResponse(traces []*model.Trace, adjust bool, uiErrors []structuredError) *structuredResponse {
252+
func (*APIHandler) tracesToResponse(traces []*model.Trace, uiErrors []structuredError) *structuredResponse {
253253
uiTraces := make([]*ui.Trace, len(traces))
254254
for i, v := range traces {
255-
uiTrace := aH.convertModelToUI(v, adjust)
255+
uiTrace := uiconv.FromDomain(v)
256256
uiTraces[i] = uiTrace
257257
}
258258

@@ -362,14 +362,6 @@ func (aH *APIHandler) metrics(w http.ResponseWriter, r *http.Request, getMetrics
362362
aH.writeJSON(w, r, m)
363363
}
364364

365-
func (aH *APIHandler) convertModelToUI(trc *model.Trace, adjust bool) *ui.Trace {
366-
if adjust {
367-
aH.queryService.Adjust(trc)
368-
}
369-
uiTrace := uiconv.FromDomain(trc)
370-
return uiTrace
371-
}
372-
373365
func (*APIHandler) deduplicateDependencies(dependencies []model.DependencyLink) []ui.DependencyLink {
374366
type Key struct {
375367
parent string
@@ -483,16 +475,10 @@ func (aH *APIHandler) getTrace(w http.ResponseWriter, r *http.Request) {
483475
}
484476

485477
var uiErrors []structuredError
486-
structuredRes := aH.tracesToResponse([]*model.Trace{trc}, shouldAdjust(r), uiErrors)
478+
structuredRes := aH.tracesToResponse([]*model.Trace{trc}, uiErrors)
487479
aH.writeJSON(w, r, structuredRes)
488480
}
489481

490-
func shouldAdjust(r *http.Request) bool {
491-
raw := r.FormValue("raw")
492-
isRaw, _ := strconv.ParseBool(raw)
493-
return !isRaw
494-
}
495-
496482
// archiveTrace implements the REST API POST:/archive/{trace-id}.
497483
// It passes the traceID to queryService.ArchiveTrace for writing.
498484
func (aH *APIHandler) archiveTrace(w http.ResponseWriter, r *http.Request) {

‎cmd/query/app/querysvc/query_service.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,13 @@ func (qs QueryService) GetTrace(ctx context.Context, query GetTraceParameters) (
8686
}
8787
trace, err = qs.options.ArchiveSpanReader.GetTrace(ctx, query.GetTraceParameters)
8888
}
89-
return trace, err
89+
if err != nil {
90+
return nil, err
91+
}
92+
if !query.RawTraces {
93+
qs.adjust(trace)
94+
}
95+
return trace, nil
9096
}
9197

9298
// GetServices is the queryService implementation of spanstore.Reader.GetServices
@@ -116,7 +122,16 @@ func (qs QueryService) FindTraces(ctx context.Context, query *TraceQueryParamete
116122
if err != nil {
117123
return nil, err
118124
}
119-
return spanReader.FindTraces(ctx, &query.TraceQueryParameters)
125+
traces, err := spanReader.FindTraces(ctx, &query.TraceQueryParameters)
126+
if err != nil {
127+
return nil, err
128+
}
129+
if !query.RawTraces {
130+
for _, trace := range traces {
131+
qs.adjust(trace)
132+
}
133+
}
134+
return traces, nil
120135
}
121136

122137
// ArchiveTrace is the queryService utility to archive traces.
@@ -140,7 +155,7 @@ func (qs QueryService) ArchiveTrace(ctx context.Context, query spanstore.GetTrac
140155
}
141156

142157
// Adjust applies adjusters to the trace.
143-
func (qs QueryService) Adjust(trace *model.Trace) {
158+
func (qs QueryService) adjust(trace *model.Trace) {
144159
qs.options.Adjuster.Adjust(trace)
145160
}
146161

‎cmd/query/app/querysvc/query_service_test.go

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package querysvc
66
import (
77
"context"
88
"errors"
9+
"fmt"
910
"testing"
1011
"time"
1112

@@ -115,6 +116,68 @@ func TestGetTraceSuccess(t *testing.T) {
115116
assert.Equal(t, res, mockTrace)
116117
}
117118

119+
func TestGetTraceWithRawTraces(t *testing.T) {
120+
traceID := model.NewTraceID(0, 1)
121+
tests := []struct {
122+
rawTraces bool
123+
tags model.KeyValues
124+
expected model.KeyValues
125+
}{
126+
{
127+
// tags should not get sorted by SortTagsAndLogFields adjuster
128+
rawTraces: true,
129+
tags: model.KeyValues{
130+
model.String("z", "key"),
131+
model.String("a", "key"),
132+
},
133+
expected: model.KeyValues{
134+
model.String("z", "key"),
135+
model.String("a", "key"),
136+
},
137+
},
138+
{
139+
// tags should get sorted by SortTagsAndLogFields adjuster
140+
rawTraces: false,
141+
tags: model.KeyValues{
142+
model.String("z", "key"),
143+
model.String("a", "key"),
144+
},
145+
expected: model.KeyValues{
146+
model.String("a", "key"),
147+
model.String("z", "key"),
148+
},
149+
},
150+
}
151+
for _, test := range tests {
152+
t.Run(fmt.Sprintf("rawTraces=%v", test.rawTraces), func(t *testing.T) {
153+
trace := &model.Trace{
154+
Spans: []*model.Span{
155+
{
156+
TraceID: traceID,
157+
SpanID: model.NewSpanID(1),
158+
Process: &model.Process{},
159+
Tags: test.tags,
160+
},
161+
},
162+
}
163+
tqs := initializeTestService()
164+
tqs.spanReader.On("GetTrace", mock.Anything, mock.AnythingOfType("spanstore.GetTraceParameters")).
165+
Return(trace, nil).Once()
166+
query := GetTraceParameters{
167+
GetTraceParameters: spanstore.GetTraceParameters{
168+
TraceID: mockTraceID,
169+
},
170+
RawTraces: test.rawTraces,
171+
}
172+
gotTrace, err := tqs.queryService.GetTrace(context.Background(), query)
173+
require.NoError(t, err)
174+
spans := gotTrace.Spans
175+
require.Len(t, spans, 1)
176+
require.EqualValues(t, test.expected, spans[0].Tags)
177+
})
178+
}
179+
}
180+
118181
// Test QueryService.GetTrace() without ArchiveSpanReader
119182
func TestGetTraceNotFound(t *testing.T) {
120183
tqs := initializeTestService()
@@ -239,6 +302,66 @@ func TestFindTraces(t *testing.T) {
239302
assert.Len(t, traces, 1)
240303
}
241304

305+
func TestFindTracesWithRawTraces(t *testing.T) {
306+
traceID := model.NewTraceID(0, 1)
307+
tests := []struct {
308+
rawTraces bool
309+
tags model.KeyValues
310+
expected model.KeyValues
311+
}{
312+
{
313+
// tags should not get sorted by SortTagsAndLogFields adjuster
314+
rawTraces: true,
315+
tags: model.KeyValues{
316+
model.String("z", "key"),
317+
model.String("a", "key"),
318+
},
319+
expected: model.KeyValues{
320+
model.String("z", "key"),
321+
model.String("a", "key"),
322+
},
323+
},
324+
{
325+
// tags should get sorted by SortTagsAndLogFields adjuster
326+
rawTraces: false,
327+
tags: model.KeyValues{
328+
model.String("z", "key"),
329+
model.String("a", "key"),
330+
},
331+
expected: model.KeyValues{
332+
model.String("a", "key"),
333+
model.String("z", "key"),
334+
},
335+
},
336+
}
337+
for _, test := range tests {
338+
t.Run(fmt.Sprintf("rawTraces=%v", test.rawTraces), func(t *testing.T) {
339+
trace := &model.Trace{
340+
Spans: []*model.Span{
341+
{
342+
TraceID: traceID,
343+
SpanID: model.NewSpanID(1),
344+
Process: &model.Process{},
345+
Tags: test.tags,
346+
},
347+
},
348+
}
349+
tqs := initializeTestService()
350+
tqs.spanReader.On("FindTraces", mock.Anything, mock.AnythingOfType("*spanstore.TraceQueryParameters")).
351+
Return([]*model.Trace{trace}, nil).Once()
352+
params := &TraceQueryParameters{
353+
RawTraces: test.rawTraces,
354+
}
355+
traces, err := tqs.queryService.FindTraces(context.Background(), params)
356+
require.NoError(t, err)
357+
require.Len(t, traces, 1)
358+
spans := traces[0].Spans
359+
require.Len(t, spans, 1)
360+
require.EqualValues(t, test.expected, spans[0].Tags)
361+
})
362+
}
363+
}
364+
242365
func TestFindTraces_V1ReaderNotFound(t *testing.T) {
243366
fr := new(tracestoremocks.Reader)
244367
qs := QueryService{
@@ -259,6 +382,15 @@ func TestFindTraces_V1ReaderNotFound(t *testing.T) {
259382
require.Error(t, err)
260383
}
261384

385+
func TestFindTracesError(t *testing.T) {
386+
tqs := initializeTestService()
387+
tqs.spanReader.On("FindTraces", mock.Anything, mock.AnythingOfType("*spanstore.TraceQueryParameters")).
388+
Return(nil, assert.AnError).Once()
389+
traces, err := tqs.queryService.FindTraces(context.Background(), &TraceQueryParameters{})
390+
require.ErrorIs(t, err, assert.AnError)
391+
require.Nil(t, traces)
392+
}
393+
262394
// Test QueryService.ArchiveTrace() with no ArchiveSpanWriter.
263395
func TestArchiveTraceNoOptions(t *testing.T) {
264396
tqs := initializeTestService()

0 commit comments

Comments
 (0)