From bab92e8cd2e4f15b3d59f86358b1d5a74eb9007e Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 1 Jan 2025 15:34:59 -0500 Subject: [PATCH 01/10] Remove Helper Function Not Required Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/http_gateway.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go index ab257720730..536827a2428 100644 --- a/cmd/query/app/apiv3/http_gateway.go +++ b/cmd/query/app/apiv3/http_gateway.go @@ -15,7 +15,6 @@ import ( "github.com/gogo/protobuf/jsonpb" "github.com/gogo/protobuf/proto" "github.com/gorilla/mux" - "go.opentelemetry.io/collector/pdata/ptrace" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -109,15 +108,6 @@ func (h *HTTPGateway) tryParamError(w http.ResponseWriter, err error, paramName } func (h *HTTPGateway) returnSpans(spans []*model.Span, w http.ResponseWriter) { - // modelToOTLP does not easily return an error, so allow mocking it - h.returnSpansTestable(spans, w, modelToOTLP) -} - -func (h *HTTPGateway) returnSpansTestable( - spans []*model.Span, - w http.ResponseWriter, - modelToOTLP func(_ []*model.Span) ptrace.Traces, -) { td := modelToOTLP(spans) tracesData := api_v3.TracesData(td) response := &api_v3.GRPCGatewayWrapper{ From 976a0e5846618acb81473d6c57b1be33c526e6b8 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 1 Jan 2025 15:50:38 -0500 Subject: [PATCH 02/10] Change api_v3 Handler To Use V2 Query Service Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/http_gateway.go | 36 +++++++++++++++--------- cmd/query/app/apiv3/http_gateway_test.go | 2 +- cmd/query/app/server.go | 14 +++++---- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go index 536827a2428..25ab583476e 100644 --- a/cmd/query/app/apiv3/http_gateway.go +++ b/cmd/query/app/apiv3/http_gateway.go @@ -19,10 +19,12 @@ import ( "go.opentelemetry.io/otel/trace" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" + "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" "github.com/jaegertracing/jaeger/internal/proto/api_v3" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/storage/spanstore" + "github.com/jaegertracing/jaeger/storage_v2/tracestore" + "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) const ( @@ -127,9 +129,11 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) { if h.tryParamError(w, err, paramTraceID) { return } - request := querysvc.GetTraceParameters{ - GetTraceParameters: spanstore.GetTraceParameters{ - TraceID: traceID, + request := querysvc.GetTraceParams{ + TraceIDs: []tracestore.GetTraceParams{ + { + TraceID: traceID.ToOTELTraceID(), + }, }, } http_query := r.URL.Query() @@ -139,7 +143,7 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) { if h.tryParamError(w, err, paramStartTime) { return } - request.StartTime = timeParsed.UTC() + request.TraceIDs[0].Start = timeParsed.UTC() } endTime := http_query.Get(paramEndTime) if endTime != "" { @@ -147,7 +151,7 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) { if h.tryParamError(w, err, paramEndTime) { return } - request.EndTime = timeParsed.UTC() + request.TraceIDs[0].End = timeParsed.UTC() } if r := http_query.Get(paramRawTraces); r != "" { rawTraces, err := strconv.ParseBool(r) @@ -156,11 +160,16 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) { } request.RawTraces = rawTraces } - trc, err := h.QueryService.GetTrace(r.Context(), request) + getTracesIter := h.QueryService.GetTraces(r.Context(), request) + trc, err := v1adapter.V1TracesFromSeq2(getTracesIter) if h.tryHandleError(w, err, http.StatusInternalServerError) { return } - h.returnSpans(trc.Spans, w) + if len(trc) == 0 { + // TODO: should we return 404 if trace not found? + return + } + h.returnSpans(trc[0].Spans, w) } func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) { @@ -169,7 +178,8 @@ func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) { return } - traces, err := h.QueryService.FindTraces(r.Context(), queryParams) + findTracesIter := h.QueryService.FindTraces(r.Context(), *queryParams) + traces, err := v1adapter.V1TracesFromSeq2(findTracesIter) // TODO how do we distinguish internal error from bad parameters for FindTrace? if h.tryHandleError(w, err, http.StatusInternalServerError) { return @@ -181,9 +191,9 @@ func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) { h.returnSpans(spans, w) } -func (h *HTTPGateway) parseFindTracesQuery(q url.Values, w http.ResponseWriter) (*querysvc.TraceQueryParameters, bool) { - queryParams := &querysvc.TraceQueryParameters{ - TraceQueryParameters: spanstore.TraceQueryParameters{ +func (h *HTTPGateway) parseFindTracesQuery(q url.Values, w http.ResponseWriter) (*querysvc.TraceQueryParams, bool) { + queryParams := &querysvc.TraceQueryParams{ + TraceQueryParams: tracestore.TraceQueryParams{ ServiceName: q.Get(paramServiceName), OperationName: q.Get(paramOperationName), Tags: nil, // most curiously not supported by grpc-gateway @@ -252,7 +262,7 @@ func (h *HTTPGateway) getServices(w http.ResponseWriter, r *http.Request) { func (h *HTTPGateway) getOperations(w http.ResponseWriter, r *http.Request) { query := r.URL.Query() - queryParams := spanstore.OperationQueryParameters{ + queryParams := tracestore.OperationQueryParams{ ServiceName: query.Get("service"), SpanKind: query.Get("span_kind"), } diff --git a/cmd/query/app/apiv3/http_gateway_test.go b/cmd/query/app/apiv3/http_gateway_test.go index 37eee29d8e9..c9bb070cc79 100644 --- a/cmd/query/app/apiv3/http_gateway_test.go +++ b/cmd/query/app/apiv3/http_gateway_test.go @@ -17,7 +17,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" + "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/jtracer" "github.com/jaegertracing/jaeger/pkg/testutils" diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index b6a992ad8ef..db33dfc5d83 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -28,7 +28,7 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/apiv3" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" - querysvcv2 "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" + v2querysvc "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" "github.com/jaegertracing/jaeger/internal/proto/api_v3" "github.com/jaegertracing/jaeger/pkg/bearertoken" "github.com/jaegertracing/jaeger/pkg/netutils" @@ -59,7 +59,7 @@ type Server struct { func NewServer( ctx context.Context, querySvc *querysvc.QueryService, - v2QuerySvc *querysvcv2.QueryService, + v2QuerySvc *v2querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, @@ -84,7 +84,7 @@ func NewServer( return nil, err } registerGRPCHandlers(grpcServer, querySvc, v2QuerySvc, metricsQuerySvc, telset) - httpServer, err := createHTTPServer(ctx, querySvc, metricsQuerySvc, options, tm, telset) + httpServer, err := createHTTPServer(ctx, querySvc, v2QuerySvc, metricsQuerySvc, options, tm, telset) if err != nil { return nil, err } @@ -102,7 +102,7 @@ func NewServer( func registerGRPCHandlers( server *grpc.Server, querySvc *querysvc.QueryService, - v2QuerySvc *querysvcv2.QueryService, + v2QuerySvc *v2querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, telset telemetry.Settings, ) { @@ -167,6 +167,7 @@ var _ io.Closer = (*httpServer)(nil) func initRouter( querySvc *querysvc.QueryService, + v2QuerySvc *v2querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, queryOpts *QueryOptions, tenancyMgr *tenancy.Manager, @@ -187,7 +188,7 @@ func initRouter( } (&apiv3.HTTPGateway{ - QueryService: querySvc, + QueryService: v2QuerySvc, Logger: telset.Logger, Tracer: telset.TracerProvider, }).RegisterRoutes(r) @@ -209,12 +210,13 @@ func initRouter( func createHTTPServer( ctx context.Context, querySvc *querysvc.QueryService, + v2QuerySvc *v2querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, queryOpts *QueryOptions, tm *tenancy.Manager, telset telemetry.Settings, ) (*httpServer, error) { - handler, staticHandlerCloser := initRouter(querySvc, metricsQuerySvc, queryOpts, tm, telset) + handler, staticHandlerCloser := initRouter(querySvc, v2QuerySvc, metricsQuerySvc, queryOpts, tm, telset) handler = recoveryhandler.NewRecoveryHandler(telset.Logger, true)(handler) hs, err := queryOpts.HTTP.ToServer( ctx, From cc103c9f6d6d326b9c3e11c9ba4a852112eaaab0 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 1 Jan 2025 16:50:16 -0500 Subject: [PATCH 03/10] Remove Double Translation Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/http_gateway.go | 28 +++++++++++++++++--------- cmd/query/app/apiv3/otlp_translator.go | 17 ---------------- 2 files changed, 18 insertions(+), 27 deletions(-) delete mode 100644 cmd/query/app/apiv3/otlp_translator.go diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go index 25ab583476e..2d76402bcb8 100644 --- a/cmd/query/app/apiv3/http_gateway.go +++ b/cmd/query/app/apiv3/http_gateway.go @@ -15,16 +15,18 @@ import ( "github.com/gogo/protobuf/jsonpb" "github.com/gogo/protobuf/proto" "github.com/gorilla/mux" + "go.opentelemetry.io/collector/pdata/ptrace" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" + "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/internal/proto/api_v3" "github.com/jaegertracing/jaeger/model" + "github.com/jaegertracing/jaeger/pkg/iter" "github.com/jaegertracing/jaeger/storage/spanstore" "github.com/jaegertracing/jaeger/storage_v2/tracestore" - "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) const ( @@ -109,9 +111,8 @@ func (h *HTTPGateway) tryParamError(w http.ResponseWriter, err error, paramName return h.tryHandleError(w, fmt.Errorf("malformed parameter %s: %w", paramName, err), http.StatusBadRequest) } -func (h *HTTPGateway) returnSpans(spans []*model.Span, w http.ResponseWriter) { - td := modelToOTLP(spans) - tracesData := api_v3.TracesData(td) +func (h *HTTPGateway) returnTrace(t ptrace.Traces, w http.ResponseWriter) { + tracesData := api_v3.TracesData(t) response := &api_v3.GRPCGatewayWrapper{ Result: &tracesData, } @@ -161,7 +162,9 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) { request.RawTraces = rawTraces } getTracesIter := h.QueryService.GetTraces(r.Context(), request) - trc, err := v1adapter.V1TracesFromSeq2(getTracesIter) + aggrTracesIter := jptrace.AggregateTraces(getTracesIter) + trc, err := iter.CollectWithErrors(aggrTracesIter) + if h.tryHandleError(w, err, http.StatusInternalServerError) { return } @@ -169,7 +172,7 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) { // TODO: should we return 404 if trace not found? return } - h.returnSpans(trc[0].Spans, w) + h.returnTrace(trc[0], w) } func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) { @@ -179,16 +182,21 @@ func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) { } findTracesIter := h.QueryService.FindTraces(r.Context(), *queryParams) - traces, err := v1adapter.V1TracesFromSeq2(findTracesIter) + aggrTracesIter := jptrace.AggregateTraces(findTracesIter) + traces, err := iter.CollectWithErrors(aggrTracesIter) // TODO how do we distinguish internal error from bad parameters for FindTrace? if h.tryHandleError(w, err, http.StatusInternalServerError) { return } - var spans []*model.Span + combinedTrace := ptrace.NewTraces() for _, t := range traces { - spans = append(spans, t.Spans...) + resources := t.ResourceSpans() + for i := 0; i < resources.Len(); i++ { + resource := resources.At(i) + resource.CopyTo(combinedTrace.ResourceSpans().AppendEmpty()) + } } - h.returnSpans(spans, w) + h.returnTrace(combinedTrace, w) } func (h *HTTPGateway) parseFindTracesQuery(q url.Values, w http.ResponseWriter) (*querysvc.TraceQueryParams, bool) { diff --git a/cmd/query/app/apiv3/otlp_translator.go b/cmd/query/app/apiv3/otlp_translator.go deleted file mode 100644 index 1d1bba2f4cc..00000000000 --- a/cmd/query/app/apiv3/otlp_translator.go +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright (c) 2021 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package apiv3 - -import ( - "go.opentelemetry.io/collector/pdata/ptrace" - - "github.com/jaegertracing/jaeger/model" - "github.com/jaegertracing/jaeger/storage_v2/v1adapter" -) - -func modelToOTLP(spans []*model.Span) ptrace.Traces { - batch := &model.Batch{Spans: spans} - tr := v1adapter.V1BatchesToTraces([]*model.Batch{batch}) - return tr -} From e9bcd4941c9cacbea41c28c748a51f8def7a336a Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 1 Jan 2025 23:17:15 -0500 Subject: [PATCH 04/10] Fix Get Trace Success Unit Test Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/gateway_test.go | 4 +-- cmd/query/app/apiv3/http_gateway_test.go | 46 ++++++++++++++++-------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/cmd/query/app/apiv3/gateway_test.go b/cmd/query/app/apiv3/gateway_test.go index 26ac8fa3179..e0a41dceff4 100644 --- a/cmd/query/app/apiv3/gateway_test.go +++ b/cmd/query/app/apiv3/gateway_test.go @@ -23,7 +23,7 @@ import ( "github.com/jaegertracing/jaeger/model" _ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration "github.com/jaegertracing/jaeger/storage/spanstore" - spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" + tracestoremocks "github.com/jaegertracing/jaeger/storage_v2/tracestore/mocks" ) // Utility functions used from http_gateway_test.go. @@ -38,7 +38,7 @@ const ( var regenerateSnapshots = os.Getenv("REGENERATE_SNAPSHOTS") == "true" type testGateway struct { - reader *spanstoremocks.Reader + reader *tracestoremocks.Reader url string router *mux.Router // used to set a tenancy header when executing requests diff --git a/cmd/query/app/apiv3/http_gateway_test.go b/cmd/query/app/apiv3/http_gateway_test.go index c9bb070cc79..6bcafd5037f 100644 --- a/cmd/query/app/apiv3/http_gateway_test.go +++ b/cmd/query/app/apiv3/http_gateway_test.go @@ -15,16 +15,19 @@ import ( "github.com/gorilla/mux" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/pdata/pcommon" + "go.opentelemetry.io/collector/pdata/ptrace" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" "github.com/jaegertracing/jaeger/model" + "github.com/jaegertracing/jaeger/pkg/iter" "github.com/jaegertracing/jaeger/pkg/jtracer" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/storage/spanstore" - spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" dependencyStoreMocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks" - "github.com/jaegertracing/jaeger/storage_v2/v1adapter" + "github.com/jaegertracing/jaeger/storage_v2/tracestore" + tracestoremocks "github.com/jaegertracing/jaeger/storage_v2/tracestore/mocks" ) func setupHTTPGatewayNoServer( @@ -32,10 +35,10 @@ func setupHTTPGatewayNoServer( basePath string, ) *testGateway { gw := &testGateway{ - reader: &spanstoremocks.Reader{}, + reader: &tracestoremocks.Reader{}, } - q := querysvc.NewQueryService(v1adapter.NewTraceReader(gw.reader), + q := querysvc.NewQueryService(gw.reader, &dependencyStoreMocks.Reader{}, querysvc.QueryServiceOptions{}, ) @@ -93,16 +96,28 @@ func TestHTTPGatewayTryHandleError(t *testing.T) { func TestHTTPGatewayGetTrace(t *testing.T) { traceId, _ := model.TraceIDFromString("123") + + makeTestTraces := func() ptrace.Traces { + traces := ptrace.NewTraces() + resources := traces.ResourceSpans().AppendEmpty() + scopes := resources.ScopeSpans().AppendEmpty() + span := scopes.Spans().AppendEmpty() + span.SetName("test-span") + span.SetTraceID(traceId.ToOTELTraceID()) + span.SetSpanID(pcommon.SpanID{1}) + return traces + } + testCases := []struct { name string params map[string]string - expectedQuery spanstore.GetTraceParameters + expectedQuery tracestore.GetTraceParams }{ { name: "TestGetTrace", params: map[string]string{}, - expectedQuery: spanstore.GetTraceParameters{ - TraceID: traceId, + expectedQuery: tracestore.GetTraceParams{ + TraceID: traceId.ToOTELTraceID(), }, }, { @@ -111,10 +126,10 @@ func TestHTTPGatewayGetTrace(t *testing.T) { "start_time": "2000-01-02T12:30:08.999999998Z", "end_time": "2000-04-05T21:55:16.999999992+08:00", }, - expectedQuery: spanstore.GetTraceParameters{ - TraceID: traceId, - StartTime: time.Date(2000, time.January, 0o2, 12, 30, 8, 999999998, time.UTC), - EndTime: time.Date(2000, time.April, 0o5, 13, 55, 16, 999999992, time.UTC), + expectedQuery: tracestore.GetTraceParams{ + TraceID: traceId.ToOTELTraceID(), + Start: time.Date(2000, time.January, 0o2, 12, 30, 8, 999999998, time.UTC), + End: time.Date(2000, time.April, 0o5, 13, 55, 16, 999999992, time.UTC), }, }, } @@ -124,9 +139,12 @@ func TestHTTPGatewayGetTrace(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { gw := setupHTTPGatewayNoServer(t, "") + gw.reader. - On("GetTrace", matchContext, tc.expectedQuery). - Return(&model.Trace{}, nil).Once() + On("GetTraces", matchContext, tc.expectedQuery). + Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { + yield([]ptrace.Traces{makeTestTraces()}, nil) + })).Once() q := url.Values{} for k, v := range tc.params { @@ -141,7 +159,7 @@ func TestHTTPGatewayGetTrace(t *testing.T) { require.NoError(t, err) w := httptest.NewRecorder() gw.router.ServeHTTP(w, r) - gw.reader.AssertCalled(t, "GetTrace", matchContext, tc.expectedQuery) + gw.reader.AssertCalled(t, "GetTraces", matchContext, tc.expectedQuery) }) } } From 6595cc3885af9596df05f942600dc597461b88db Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 2 Jan 2025 20:25:18 -0500 Subject: [PATCH 05/10] Remove Aggregation And Return Not Found Error Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/http_gateway.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go index 2d76402bcb8..2cb77005281 100644 --- a/cmd/query/app/apiv3/http_gateway.go +++ b/cmd/query/app/apiv3/http_gateway.go @@ -21,7 +21,6 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" - "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/internal/proto/api_v3" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/iter" @@ -162,14 +161,20 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) { request.RawTraces = rawTraces } getTracesIter := h.QueryService.GetTraces(r.Context(), request) - aggrTracesIter := jptrace.AggregateTraces(getTracesIter) - trc, err := iter.CollectWithErrors(aggrTracesIter) + trc, err := iter.FlattenWithErrors(getTracesIter) if h.tryHandleError(w, err, http.StatusInternalServerError) { return } if len(trc) == 0 { - // TODO: should we return 404 if trace not found? + errorResponse := api_v3.GRPCGatewayError{ + Error: &api_v3.GRPCGatewayError_GRPCGatewayErrorDetails{ + HttpCode: http.StatusNotFound, + Message: err.Error(), + }, + } + resp, _ := json.Marshal(&errorResponse) + http.Error(w, string(resp), http.StatusNotFound) return } h.returnTrace(trc[0], w) @@ -182,8 +187,7 @@ func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) { } findTracesIter := h.QueryService.FindTraces(r.Context(), *queryParams) - aggrTracesIter := jptrace.AggregateTraces(findTracesIter) - traces, err := iter.CollectWithErrors(aggrTracesIter) + traces, err := iter.FlattenWithErrors(findTracesIter) // TODO how do we distinguish internal error from bad parameters for FindTrace? if h.tryHandleError(w, err, http.StatusInternalServerError) { return From 48b851c618d4bb68d1a2b56ce424da0f643c26fc Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 2 Jan 2025 21:42:53 -0500 Subject: [PATCH 06/10] Fix Test Expectations Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/gateway_test.go | 34 +++++++------ cmd/query/app/apiv3/http_gateway_test.go | 61 ++++++++++-------------- 2 files changed, 44 insertions(+), 51 deletions(-) diff --git a/cmd/query/app/apiv3/gateway_test.go b/cmd/query/app/apiv3/gateway_test.go index abed204e4dc..8bd2bf93be2 100644 --- a/cmd/query/app/apiv3/gateway_test.go +++ b/cmd/query/app/apiv3/gateway_test.go @@ -24,7 +24,9 @@ import ( "github.com/jaegertracing/jaeger/internal/proto/api_v3" "github.com/jaegertracing/jaeger/model" _ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration + "github.com/jaegertracing/jaeger/pkg/iter" "github.com/jaegertracing/jaeger/storage/spanstore" + "github.com/jaegertracing/jaeger/storage_v2/tracestore" tracestoremocks "github.com/jaegertracing/jaeger/storage_v2/tracestore/mocks" ) @@ -112,9 +114,9 @@ func makeTestTraceV2() ptrace.Traces { spanA := scopes.Spans().AppendEmpty() spanA.SetName("foobar") spanA.SetTraceID(traceID) - spanA.SetSpanID(pcommon.SpanID([8]byte{180})) - spanA.SetKind(ptrace.SpanKindServer) - spanA.Attributes().PutBool("error", true) + spanA.SetSpanID(pcommon.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 2})) + // spanA.SetKind(ptrace.SpanKindServer) + // spanA.Attributes().PutBool("error", true) return trace } @@ -145,10 +147,10 @@ func (gw *testGateway) runGatewayGetServices(t *testing.T) { } func (gw *testGateway) runGatewayGetOperations(t *testing.T) { - qp := spanstore.OperationQueryParameters{ServiceName: "foo", SpanKind: "server"} + qp := tracestore.OperationQueryParams{ServiceName: "foo", SpanKind: "server"} gw.reader. On("GetOperations", matchContext, qp). - Return([]spanstore.Operation{{Name: "get_users", SpanKind: "server"}}, nil).Once() + Return([]tracestore.Operation{{Name: "get_users", SpanKind: "server"}}, nil).Once() body, statusCode := gw.execRequest(t, "/api/v3/operations?service=foo&span_kind=server") require.Equal(t, http.StatusOK, statusCode) @@ -162,21 +164,25 @@ func (gw *testGateway) runGatewayGetOperations(t *testing.T) { } func (gw *testGateway) runGatewayGetTrace(t *testing.T) { - trace, query := makeTestTrace() - gw.reader.On("GetTrace", matchContext, query).Return(trace, nil).Once() - gw.getTracesAndVerify(t, "/api/v3/traces/"+query.TraceID.String(), query.TraceID) + query := tracestore.GetTraceParams{TraceID: traceID} + gw.reader. + On("GetTraces", matchContext, query). + Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { + yield([]ptrace.Traces{makeTestTraceV2()}, nil) + })).Once() + gw.getTracesAndVerify(t, "/api/v3/traces/1", traceID) } func (gw *testGateway) runGatewayFindTraces(t *testing.T) { - trace, query := makeTestTrace() q, qp := mockFindQueries() - gw.reader. - On("FindTraces", matchContext, qp). - Return([]*model.Trace{trace}, nil).Once() - gw.getTracesAndVerify(t, "/api/v3/traces?"+q.Encode(), query.TraceID) + gw.reader.On("FindTraces", matchContext, qp). + Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { + yield([]ptrace.Traces{makeTestTraceV2()}, nil) + })).Once() + gw.getTracesAndVerify(t, "/api/v3/traces?"+q.Encode(), traceID) } -func (gw *testGateway) getTracesAndVerify(t *testing.T, url string, expectedTraceID model.TraceID) { +func (gw *testGateway) getTracesAndVerify(t *testing.T, url string, expectedTraceID pcommon.TraceID) { body, statusCode := gw.execRequest(t, url) require.Equal(t, http.StatusOK, statusCode, "response=%s", string(body)) body = gw.verifySnapshot(t, body) diff --git a/cmd/query/app/apiv3/http_gateway_test.go b/cmd/query/app/apiv3/http_gateway_test.go index 6bcafd5037f..59707b73b2d 100644 --- a/cmd/query/app/apiv3/http_gateway_test.go +++ b/cmd/query/app/apiv3/http_gateway_test.go @@ -14,13 +14,12 @@ import ( "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" - "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/iter" "github.com/jaegertracing/jaeger/pkg/jtracer" "github.com/jaegertracing/jaeger/pkg/testutils" @@ -95,19 +94,6 @@ func TestHTTPGatewayTryHandleError(t *testing.T) { } func TestHTTPGatewayGetTrace(t *testing.T) { - traceId, _ := model.TraceIDFromString("123") - - makeTestTraces := func() ptrace.Traces { - traces := ptrace.NewTraces() - resources := traces.ResourceSpans().AppendEmpty() - scopes := resources.ScopeSpans().AppendEmpty() - span := scopes.Spans().AppendEmpty() - span.SetName("test-span") - span.SetTraceID(traceId.ToOTELTraceID()) - span.SetSpanID(pcommon.SpanID{1}) - return traces - } - testCases := []struct { name string params map[string]string @@ -117,7 +103,7 @@ func TestHTTPGatewayGetTrace(t *testing.T) { name: "TestGetTrace", params: map[string]string{}, expectedQuery: tracestore.GetTraceParams{ - TraceID: traceId.ToOTELTraceID(), + TraceID: traceID, }, }, { @@ -127,14 +113,14 @@ func TestHTTPGatewayGetTrace(t *testing.T) { "end_time": "2000-04-05T21:55:16.999999992+08:00", }, expectedQuery: tracestore.GetTraceParams{ - TraceID: traceId.ToOTELTraceID(), + TraceID: traceID, Start: time.Date(2000, time.January, 0o2, 12, 30, 8, 999999998, time.UTC), End: time.Date(2000, time.April, 0o5, 13, 55, 16, 999999992, time.UTC), }, }, } - testUri := "/api/v3/traces/123" + testUri := "/api/v3/traces/1" for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -143,7 +129,7 @@ func TestHTTPGatewayGetTrace(t *testing.T) { gw.reader. On("GetTraces", matchContext, tc.expectedQuery). Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { - yield([]ptrace.Traces{makeTestTraces()}, nil) + yield([]ptrace.Traces{makeTestTraceV2()}, nil) })).Once() q := url.Values{} @@ -195,9 +181,10 @@ func TestHTTPGatewayGetTraceMalformedInputErrors(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { gw := setupHTTPGatewayNoServer(t, "") - gw.reader. - On("GetTrace", matchContext, matchGetTraceParameters). - Return(&model.Trace{}, nil).Once() + gw.reader.On("GetTraces", matchContext, mock.AnythingOfType("tracestore.GetTraceParams")). + Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { + yield([]ptrace.Traces{}, nil) + })).Once() r, err := http.NewRequest(http.MethodGet, tc.requestUrl, nil) require.NoError(t, err) @@ -210,9 +197,10 @@ func TestHTTPGatewayGetTraceMalformedInputErrors(t *testing.T) { func TestHTTPGatewayGetTraceInternalErrors(t *testing.T) { gw := setupHTTPGatewayNoServer(t, "") - gw.reader. - On("GetTrace", matchContext, matchGetTraceParameters). - Return(nil, assert.AnError).Once() + gw.reader.On("GetTraces", matchContext, mock.AnythingOfType("tracestore.GetTraceParams")). + Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { + yield([]ptrace.Traces{}, assert.AnError) + })).Once() r, err := http.NewRequest(http.MethodGet, "/api/v3/traces/123", nil) require.NoError(t, err) @@ -221,7 +209,7 @@ func TestHTTPGatewayGetTraceInternalErrors(t *testing.T) { assert.Contains(t, w.Body.String(), assert.AnError.Error()) } -func mockFindQueries() (url.Values, *spanstore.TraceQueryParameters) { +func mockFindQueries() (url.Values, tracestore.TraceQueryParams) { // mock performs deep comparison of the timestamps and can fail // if they are different in the timezone or the monotonic clocks. // To void that we truncate monotonic clock and force UTC timezone. @@ -236,7 +224,7 @@ func mockFindQueries() (url.Values, *spanstore.TraceQueryParameters) { q.Set(paramDurationMax, "2s") q.Set(paramNumTraces, "10") - return q, &spanstore.TraceQueryParameters{ + return q, tracestore.TraceQueryParams{ ServiceName: "foo", OperationName: "bar", StartTimeMin: time1, @@ -324,7 +312,6 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) { } t.Run("span reader error", func(t *testing.T) { q, qp := mockFindQueries() - const simErr = "simulated error" r, err := http.NewRequest(http.MethodGet, "/api/v3/traces?"+q.Encode(), nil) require.NoError(t, err) w := httptest.NewRecorder() @@ -332,40 +319,40 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) { gw := setupHTTPGatewayNoServer(t, "") gw.reader. On("FindTraces", matchContext, qp). - Return(nil, errors.New(simErr)).Once() + Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { + yield(nil, assert.AnError) + })).Once() gw.router.ServeHTTP(w, r) - assert.Contains(t, w.Body.String(), simErr) + assert.Contains(t, w.Body.String(), assert.AnError.Error()) }) } func TestHTTPGatewayGetServicesErrors(t *testing.T) { gw := setupHTTPGatewayNoServer(t, "") - const simErr = "simulated error" gw.reader. On("GetServices", matchContext). - Return(nil, errors.New(simErr)).Once() + Return(nil, assert.AnError).Once() r, err := http.NewRequest(http.MethodGet, "/api/v3/services", nil) require.NoError(t, err) w := httptest.NewRecorder() gw.router.ServeHTTP(w, r) - assert.Contains(t, w.Body.String(), simErr) + assert.Contains(t, w.Body.String(), assert.AnError.Error()) } func TestHTTPGatewayGetOperationsErrors(t *testing.T) { gw := setupHTTPGatewayNoServer(t, "") - qp := spanstore.OperationQueryParameters{ServiceName: "foo", SpanKind: "server"} - const simErr = "simulated error" + qp := tracestore.OperationQueryParams{ServiceName: "foo", SpanKind: "server"} gw.reader. On("GetOperations", matchContext, qp). - Return(nil, errors.New(simErr)).Once() + Return(nil, assert.AnError).Once() r, err := http.NewRequest(http.MethodGet, "/api/v3/operations?service=foo&span_kind=server", nil) require.NoError(t, err) w := httptest.NewRecorder() gw.router.ServeHTTP(w, r) - assert.Contains(t, w.Body.String(), simErr) + assert.Contains(t, w.Body.String(), assert.AnError.Error()) } From 39c435bd2e724e02cbd71b64711aa4e293940c0f Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 2 Jan 2025 22:04:36 -0500 Subject: [PATCH 07/10] Fix Tests And Regenerate Snapshots Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/gateway_test.go | 25 ++----------------- cmd/query/app/apiv3/grpc_handler_test.go | 5 +--- cmd/query/app/apiv3/snapshots/FindTraces.json | 6 ++--- cmd/query/app/apiv3/snapshots/GetTrace.json | 6 ++--- 4 files changed, 7 insertions(+), 35 deletions(-) diff --git a/cmd/query/app/apiv3/gateway_test.go b/cmd/query/app/apiv3/gateway_test.go index 8bd2bf93be2..27e992a6b73 100644 --- a/cmd/query/app/apiv3/gateway_test.go +++ b/cmd/query/app/apiv3/gateway_test.go @@ -22,10 +22,8 @@ import ( "go.opentelemetry.io/collector/pdata/ptrace" "github.com/jaegertracing/jaeger/internal/proto/api_v3" - "github.com/jaegertracing/jaeger/model" _ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration "github.com/jaegertracing/jaeger/pkg/iter" - "github.com/jaegertracing/jaeger/storage/spanstore" "github.com/jaegertracing/jaeger/storage_v2/tracestore" tracestoremocks "github.com/jaegertracing/jaeger/storage_v2/tracestore/mocks" ) @@ -87,25 +85,6 @@ func parseResponse(t *testing.T, body []byte, obj gogoproto.Message) { require.NoError(t, gogojsonpb.Unmarshal(bytes.NewBuffer(body), obj)) } -func makeTestTrace() (*model.Trace, spanstore.GetTraceParameters) { - traceID := model.NewTraceID(150, 160) - query := spanstore.GetTraceParameters{TraceID: traceID} - return &model.Trace{ - Spans: []*model.Span{ - { - TraceID: traceID, - SpanID: model.NewSpanID(180), - OperationName: "foobar", - Tags: []model.KeyValue{ - model.SpanKindTag(model.SpanKindServer), - model.Bool("error", true), - }, - Process: &model.Process{}, - }, - }, - }, query -} - func makeTestTraceV2() ptrace.Traces { trace := ptrace.NewTraces() resources := trace.ResourceSpans().AppendEmpty() @@ -115,8 +94,8 @@ func makeTestTraceV2() ptrace.Traces { spanA.SetName("foobar") spanA.SetTraceID(traceID) spanA.SetSpanID(pcommon.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 2})) - // spanA.SetKind(ptrace.SpanKindServer) - // spanA.Attributes().PutBool("error", true) + spanA.SetKind(ptrace.SpanKindServer) + spanA.Status().SetCode(ptrace.StatusCodeError) return trace } diff --git a/cmd/query/app/apiv3/grpc_handler_test.go b/cmd/query/app/apiv3/grpc_handler_test.go index df2b5952bc7..f37f0420fad 100644 --- a/cmd/query/app/apiv3/grpc_handler_test.go +++ b/cmd/query/app/apiv3/grpc_handler_test.go @@ -25,10 +25,7 @@ import ( tracestoremocks "github.com/jaegertracing/jaeger/storage_v2/tracestore/mocks" ) -var ( - matchContext = mock.AnythingOfType("*context.valueCtx") - matchGetTraceParameters = mock.AnythingOfType("spanstore.GetTraceParameters") -) +var matchContext = mock.AnythingOfType("*context.valueCtx") func newGrpcServer(t *testing.T, handler *Handler) (*grpc.Server, net.Addr) { server := grpc.NewServer() diff --git a/cmd/query/app/apiv3/snapshots/FindTraces.json b/cmd/query/app/apiv3/snapshots/FindTraces.json index f9676a145f0..423bb446eeb 100644 --- a/cmd/query/app/apiv3/snapshots/FindTraces.json +++ b/cmd/query/app/apiv3/snapshots/FindTraces.json @@ -8,16 +8,14 @@ "scope": {}, "spans": [ { - "endTimeUnixNano": "11651379494838206464", "kind": 2, "name": "foobar", "parentSpanId": "", - "spanId": "00000000000000b4", - "startTimeUnixNano": "11651379494838206464", + "spanId": "0000000000000002", "status": { "code": 2 }, - "traceId": "000000000000009600000000000000a0" + "traceId": "00000000000000000000000000000001" } ] } diff --git a/cmd/query/app/apiv3/snapshots/GetTrace.json b/cmd/query/app/apiv3/snapshots/GetTrace.json index f9676a145f0..423bb446eeb 100644 --- a/cmd/query/app/apiv3/snapshots/GetTrace.json +++ b/cmd/query/app/apiv3/snapshots/GetTrace.json @@ -8,16 +8,14 @@ "scope": {}, "spans": [ { - "endTimeUnixNano": "11651379494838206464", "kind": 2, "name": "foobar", "parentSpanId": "", - "spanId": "00000000000000b4", - "startTimeUnixNano": "11651379494838206464", + "spanId": "0000000000000002", "status": { "code": 2 }, - "traceId": "000000000000009600000000000000a0" + "traceId": "00000000000000000000000000000001" } ] } From d5c5a58670176f7f2bbab72524c924efd3b4e4b9 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 2 Jan 2025 22:11:45 -0500 Subject: [PATCH 08/10] Move Common Logic To Helper Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/http_gateway.go | 51 +++++++++++------------------ 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go index 2cb77005281..172bb846a81 100644 --- a/cmd/query/app/apiv3/http_gateway.go +++ b/cmd/query/app/apiv3/http_gateway.go @@ -110,14 +110,30 @@ func (h *HTTPGateway) tryParamError(w http.ResponseWriter, err error, paramName return h.tryHandleError(w, fmt.Errorf("malformed parameter %s: %w", paramName, err), http.StatusBadRequest) } -func (h *HTTPGateway) returnTrace(t ptrace.Traces, w http.ResponseWriter) { - tracesData := api_v3.TracesData(t) +func (h *HTTPGateway) returnTrace(td ptrace.Traces, w http.ResponseWriter) { + tracesData := api_v3.TracesData(td) response := &api_v3.GRPCGatewayWrapper{ Result: &tracesData, } h.marshalResponse(response, w) } +func (h *HTTPGateway) returnTraces(traces []ptrace.Traces, err error, w http.ResponseWriter) { + // TODO how do we distinguish internal error from bad parameters? + if h.tryHandleError(w, err, http.StatusInternalServerError) { + return + } + combinedTrace := ptrace.NewTraces() + for _, t := range traces { + resources := t.ResourceSpans() + for i := 0; i < resources.Len(); i++ { + resource := resources.At(i) + resource.CopyTo(combinedTrace.ResourceSpans().AppendEmpty()) + } + } + h.returnTrace(combinedTrace, w) +} + func (*HTTPGateway) marshalResponse(response proto.Message, w http.ResponseWriter) { _ = new(jsonpb.Marshaler).Marshal(w, response) } @@ -162,22 +178,7 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) { } getTracesIter := h.QueryService.GetTraces(r.Context(), request) trc, err := iter.FlattenWithErrors(getTracesIter) - - if h.tryHandleError(w, err, http.StatusInternalServerError) { - return - } - if len(trc) == 0 { - errorResponse := api_v3.GRPCGatewayError{ - Error: &api_v3.GRPCGatewayError_GRPCGatewayErrorDetails{ - HttpCode: http.StatusNotFound, - Message: err.Error(), - }, - } - resp, _ := json.Marshal(&errorResponse) - http.Error(w, string(resp), http.StatusNotFound) - return - } - h.returnTrace(trc[0], w) + h.returnTraces(trc, err, w) } func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) { @@ -188,19 +189,7 @@ func (h *HTTPGateway) findTraces(w http.ResponseWriter, r *http.Request) { findTracesIter := h.QueryService.FindTraces(r.Context(), *queryParams) traces, err := iter.FlattenWithErrors(findTracesIter) - // TODO how do we distinguish internal error from bad parameters for FindTrace? - if h.tryHandleError(w, err, http.StatusInternalServerError) { - return - } - combinedTrace := ptrace.NewTraces() - for _, t := range traces { - resources := t.ResourceSpans() - for i := 0; i < resources.Len(); i++ { - resource := resources.At(i) - resource.CopyTo(combinedTrace.ResourceSpans().AppendEmpty()) - } - } - h.returnTrace(combinedTrace, w) + h.returnTraces(traces, err, w) } func (h *HTTPGateway) parseFindTracesQuery(q url.Values, w http.ResponseWriter) (*querysvc.TraceQueryParams, bool) { From a7eb11593313c5fa067b942ad50e61a4aa8bc21e Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 2 Jan 2025 22:13:42 -0500 Subject: [PATCH 09/10] Add TODO Comment Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/http_gateway.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go index 172bb846a81..6672de22917 100644 --- a/cmd/query/app/apiv3/http_gateway.go +++ b/cmd/query/app/apiv3/http_gateway.go @@ -123,6 +123,8 @@ func (h *HTTPGateway) returnTraces(traces []ptrace.Traces, err error, w http.Res if h.tryHandleError(w, err, http.StatusInternalServerError) { return } + // TODO: the response should be streamed back to the client + // https://github.com/jaegertracing/jaeger/issues/6467 combinedTrace := ptrace.NewTraces() for _, t := range traces { resources := t.ResourceSpans() From a56567ba12cab629f1365b988ab194ecc62c1649 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 2 Jan 2025 22:25:44 -0500 Subject: [PATCH 10/10] Fix Missing Code Coverage Signed-off-by: Mahad Zaryab --- cmd/query/app/apiv3/http_gateway.go | 11 +++++++ cmd/query/app/apiv3/http_gateway_test.go | 41 +++++++++++++++++++++--- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go index 6672de22917..0470cf8b60b 100644 --- a/cmd/query/app/apiv3/http_gateway.go +++ b/cmd/query/app/apiv3/http_gateway.go @@ -123,6 +123,17 @@ func (h *HTTPGateway) returnTraces(traces []ptrace.Traces, err error, w http.Res if h.tryHandleError(w, err, http.StatusInternalServerError) { return } + if len(traces) == 0 { + errorResponse := api_v3.GRPCGatewayError{ + Error: &api_v3.GRPCGatewayError_GRPCGatewayErrorDetails{ + HttpCode: http.StatusNotFound, + Message: "No traces found", + }, + } + resp, _ := json.Marshal(&errorResponse) + http.Error(w, string(resp), http.StatusNotFound) + return + } // TODO: the response should be streamed back to the client // https://github.com/jaegertracing/jaeger/issues/6467 combinedTrace := ptrace.NewTraces() diff --git a/cmd/query/app/apiv3/http_gateway_test.go b/cmd/query/app/apiv3/http_gateway_test.go index 59707b73b2d..c42e8118d20 100644 --- a/cmd/query/app/apiv3/http_gateway_test.go +++ b/cmd/query/app/apiv3/http_gateway_test.go @@ -150,6 +150,39 @@ func TestHTTPGatewayGetTrace(t *testing.T) { } } +func TestHTTPGatewayGetTraceEmptyResponse(t *testing.T) { + gw := setupHTTPGatewayNoServer(t, "") + gw.reader.On("GetTraces", matchContext, mock.AnythingOfType("tracestore.GetTraceParams")). + Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { + yield([]ptrace.Traces{}, nil) + })).Once() + + r, err := http.NewRequest(http.MethodGet, "/api/v3/traces/1", nil) + require.NoError(t, err) + w := httptest.NewRecorder() + gw.router.ServeHTTP(w, r) + assert.Equal(t, http.StatusNotFound, w.Code) + assert.Contains(t, w.Body.String(), "No traces found") +} + +func TestHTTPGatewayFindTracesEmptyResponse(t *testing.T) { + q, qp := mockFindQueries() + r, err := http.NewRequest(http.MethodGet, "/api/v3/traces?"+q.Encode(), nil) + require.NoError(t, err) + w := httptest.NewRecorder() + + gw := setupHTTPGatewayNoServer(t, "") + gw.reader. + On("FindTraces", matchContext, qp). + Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) { + yield([]ptrace.Traces{}, nil) + })).Once() + + gw.router.ServeHTTP(w, r) + assert.Equal(t, http.StatusNotFound, w.Code) + assert.Contains(t, w.Body.String(), "No traces found") +} + func TestHTTPGatewayGetTraceMalformedInputErrors(t *testing.T) { testCases := []struct { name string @@ -163,17 +196,17 @@ func TestHTTPGatewayGetTraceMalformedInputErrors(t *testing.T) { }, { name: "TestGetTraceWithInvalidStartTime", - requestUrl: "/api/v3/traces/123?start_time=abc", + requestUrl: "/api/v3/traces/1?start_time=abc", expectedError: "malformed parameter start_time", }, { name: "TestGetTraceWithInvalidEndTime", - requestUrl: "/api/v3/traces/123?end_time=xyz", + requestUrl: "/api/v3/traces/1?end_time=xyz", expectedError: "malformed parameter end_time", }, { name: "TestGetTraceWithInvalidRawTraces", - requestUrl: "/api/v3/traces/123?raw_traces=foobar", + requestUrl: "/api/v3/traces/1?raw_traces=foobar", expectedError: "malformed parameter raw_traces", }, } @@ -202,7 +235,7 @@ func TestHTTPGatewayGetTraceInternalErrors(t *testing.T) { yield([]ptrace.Traces{}, assert.AnError) })).Once() - r, err := http.NewRequest(http.MethodGet, "/api/v3/traces/123", nil) + r, err := http.NewRequest(http.MethodGet, "/api/v3/traces/1", nil) require.NoError(t, err) w := httptest.NewRecorder() gw.router.ServeHTTP(w, r)