From aa0b033798fa2dd1757e260bde04c54c6cce575a Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 27 Dec 2023 17:14:26 -0500 Subject: [PATCH 1/6] Add grpc-gateway tests for all APIv3 methods Signed-off-by: Yuri Shkuro --- cmd/query/app/apiv3/grpc_gateway_test.go | 202 +++++++++++++++--- ...CGatewayWithBasePathAndTLS_FindTraces.json | 24 +++ ...tewayWithBasePathAndTLS_GetOperations.json | 8 + ...GatewayWithBasePathAndTLS_GetServices.json | 5 + ...RPCGatewayWithBasePathAndTLS_GetTrace.json | 24 +++ ...TestGRPCGatewayWithTenancy_FindTraces.json | 24 +++ ...tGRPCGatewayWithTenancy_GetOperations.json | 8 + ...estGRPCGatewayWithTenancy_GetServices.json | 5 + .../TestGRPCGatewayWithTenancy_GetTrace.json | 24 +++ .../snapshots/TestGRPCGateway_FindTraces.json | 24 +++ .../TestGRPCGateway_GetOperations.json | 8 + .../TestGRPCGateway_GetServices.json | 5 + .../snapshots/TestGRPCGateway_GetTrace.json | 24 +++ 13 files changed, 351 insertions(+), 34 deletions(-) create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_FindTraces.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetOperations.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetServices.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetTrace.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_FindTraces.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetOperations.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetServices.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetTrace.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGateway_FindTraces.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetOperations.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetServices.json create mode 100644 cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetTrace.json diff --git a/cmd/query/app/apiv3/grpc_gateway_test.go b/cmd/query/app/apiv3/grpc_gateway_test.go index e39c25e81be..118b412cb21 100644 --- a/cmd/query/app/apiv3/grpc_gateway_test.go +++ b/cmd/query/app/apiv3/grpc_gateway_test.go @@ -21,12 +21,17 @@ import ( "io" "net" "net/http" + "net/url" + "os" + "path/filepath" "strings" "testing" + "time" "github.com/gorilla/mux" "github.com/grpc-ecosystem/grpc-gateway/runtime" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/zap" "google.golang.org/grpc" @@ -39,23 +44,33 @@ import ( "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/proto-gen/api_v3" dependencyStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" + "github.com/jaegertracing/jaeger/storage/spanstore" spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) -var testCertKeyLocation = "../../../../pkg/config/tlscfg/testdata/" +const ( + testCertKeyLocation = "../../../../pkg/config/tlscfg/testdata/" + snapshotLocation = "./snapshots/" +) + +var regenerateSnapshots = os.Getenv("REGENERATE_SNAPSHOTS") == "true" type testGateway struct { reader *spanstoremocks.Reader url string } +type gatewayRequest struct { + url string + setupRequest func(*http.Request) +} + func setupGRPCGateway( t *testing.T, basePath string, serverTLS, clientTLS *tlscfg.Options, tenancyOptions tenancy.Options, ) *testGateway { - // *spanstoremocks.Reader, net.Listener, *grpc.Server, context.CancelFunc, *http.Server gw := &testGateway{ reader: &spanstoremocks.Reader{}, } @@ -123,6 +138,66 @@ func setupGRPCGateway( return gw } +func (gw *testGateway) execRequest(t *testing.T, gwReq *gatewayRequest) ([]byte, int) { + req, err := http.NewRequest(http.MethodGet, gw.url+gwReq.url, nil) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + gwReq.setupRequest(req) + response, err := http.DefaultClient.Do(req) + require.NoError(t, err) + body, err := io.ReadAll(response.Body) + require.NoError(t, err) + require.NoError(t, response.Body.Close()) + return body, response.StatusCode +} + +func verifySnapshot(t *testing.T, body []byte) []byte { + // reformat JSON body with indentation, to make diffing easier + var data interface{} + require.NoError(t, json.Unmarshal(body, &data)) + body, err := json.MarshalIndent(data, "", " ") + require.NoError(t, err) + + snapshotFile := filepath.Join(snapshotLocation, strings.ReplaceAll(t.Name(), "/", "_")+".json") + if regenerateSnapshots { + os.WriteFile(snapshotFile, body, 0o644) + } + snapshot, err := os.ReadFile(snapshotFile) + require.NoError(t, err) + assert.Equal(t, string(snapshot), string(body), "comparing against stored snapshot. Use REGENERATE_SNAPSHOTS=true to rebuild snapshots.") + return body +} + +func parseResponse(t *testing.T, body []byte, obj interface{}) { + // TODO can we use gogo marshaler here instead of grcp-gateway? + jsonpb := &runtime.JSONPb{} + require.NoError(t, jsonpb.Unmarshal(body, obj)) +} + +func parseChunkResponse(t *testing.T, body []byte, obj interface{}) { + // Unwrap the 'result' container generated by the gateway. + // See https://github.com/grpc-ecosystem/grpc-gateway/issues/2189 + type resultWrapper struct { + Result json.RawMessage `json:"result"` + } + var result resultWrapper + require.NoError(t, json.Unmarshal(body, &result)) + parseResponse(t, result.Result, obj) +} + +func makeTestTrace() (*model.Trace, model.TraceID) { + traceID := model.NewTraceID(150, 160) + return &model.Trace{ + Spans: []*model.Span{ + { + TraceID: traceID, + SpanID: model.NewSpanID(180), + OperationName: "foobar", + }, + }, + }, traceID +} + func testGRPCGateway( t *testing.T, basePath string, serverTLS, clientTLS *tlscfg.Options, @@ -144,36 +219,100 @@ func testGRPCGatewayWithTenancy( setupRequest func(*http.Request), ) { gw := setupGRPCGateway(t, basePath, serverTLS, clientTLS, tenancyOptions) + t.Run("GetServices", func(t *testing.T) { + runGatewayGetServices(t, gw, setupRequest) + }) + t.Run("GetOperations", func(t *testing.T) { + runGatewayGetOperations(t, gw, setupRequest) + }) + t.Run("GetTrace", func(t *testing.T) { + runGatewayGetTrace(t, gw, setupRequest) + }) + t.Run("FindTraces", func(t *testing.T) { + runGatewayFindTraces(t, gw, setupRequest) + }) +} - traceID := model.NewTraceID(150, 160) - gw.reader.On("GetTrace", matchContext, matchTraceID).Return( - &model.Trace{ - Spans: []*model.Span{ - { - TraceID: traceID, - SpanID: model.NewSpanID(180), - OperationName: "foobar", - }, - }, - }, nil).Once() +func runGatewayGetServices(t *testing.T, gw *testGateway, setupRequest func(*http.Request)) { + gw.reader.On("GetServices", matchContext).Return([]string{"foo"}, nil).Once() - req, err := http.NewRequest(http.MethodGet, gw.url+"/api/v3/traces/123", nil) - require.NoError(t, err) - req.Header.Set("Content-Type", "application/json") - setupRequest(req) - response, err := http.DefaultClient.Do(req) - require.NoError(t, err) - body, err := io.ReadAll(response.Body) - require.NoError(t, err) - require.NoError(t, response.Body.Close()) + body, statusCode := gw.execRequest(t, &gatewayRequest{ + url: "/api/v3/services", + setupRequest: setupRequest, + }) + require.Equal(t, http.StatusOK, statusCode) + body = verifySnapshot(t, body) + + var response struct { + Services []string `json:"services"` + } + parseResponse(t, body, &response) + assert.Equal(t, []string{"foo"}, response.Services) +} + +func runGatewayGetOperations(t *testing.T, gw *testGateway, setupRequest func(*http.Request)) { + gw.reader. + On("GetOperations", matchContext, mock.AnythingOfType("spanstore.OperationQueryParameters")). + Return([]spanstore.Operation{{Name: "get_users", SpanKind: "server"}}, nil).Once() + + body, statusCode := gw.execRequest(t, &gatewayRequest{ + url: "/api/v3/operations", + setupRequest: setupRequest, + }) + require.Equal(t, http.StatusOK, statusCode) + body = verifySnapshot(t, body) + + var response struct { + Operations []struct { + Name string `json:"name"` + SpanKind string `json:"spanKind"` + } `json:"operations"` + } + parseResponse(t, body, &response) + require.Len(t, response.Operations, 1) + assert.Equal(t, "get_users", response.Operations[0].Name) + assert.Equal(t, "server", response.Operations[0].SpanKind) +} + +func runGatewayGetTrace(t *testing.T, gw *testGateway, setupRequest func(*http.Request)) { + trace, traceID := makeTestTrace() + gw.reader.On("GetTrace", matchContext, matchTraceID).Return(trace, nil).Once() + + body, statusCode := gw.execRequest(t, &gatewayRequest{ + url: "/api/v3/traces/123", + setupRequest: setupRequest, + }) + require.Equal(t, http.StatusOK, statusCode) + body = verifySnapshot(t, body) - jsonpb := &runtime.JSONPb{} - var envelope envelope - err = json.Unmarshal(body, &envelope) - require.NoError(t, err) var spansResponse api_v3.SpansResponseChunk - err = jsonpb.Unmarshal(envelope.Result, &spansResponse) - require.NoError(t, err) + parseChunkResponse(t, body, &spansResponse) + + assert.Len(t, spansResponse.GetResourceSpans(), 1) + assert.Equal(t, bytesOfTraceID(t, traceID.High, traceID.Low), spansResponse.GetResourceSpans()[0].GetScopeSpans()[0].GetSpans()[0].GetTraceId()) +} + +func runGatewayFindTraces(t *testing.T, gw *testGateway, setupRequest func(*http.Request)) { + trace, traceID := makeTestTrace() + gw.reader. + On("FindTraces", matchContext, mock.AnythingOfType("*spanstore.TraceQueryParameters")). + Return([]*model.Trace{trace}, nil).Once() + + q := url.Values{} + q.Set("query.service_name", "foobar") + q.Set("query.start_time_min", time.Now().Format(time.RFC3339)) + q.Set("query.start_time_max", time.Now().Format(time.RFC3339)) + + body, statusCode := gw.execRequest(t, &gatewayRequest{ + url: "/api/v3/traces?" + q.Encode(), + setupRequest: setupRequest, + }) + require.Equal(t, http.StatusOK, statusCode, "response=%s", string(body)) + body = verifySnapshot(t, body) + + var spansResponse api_v3.SpansResponseChunk + parseChunkResponse(t, body, &spansResponse) + assert.Len(t, spansResponse.GetResourceSpans(), 1) assert.Equal(t, bytesOfTraceID(t, traceID.High, traceID.Low), spansResponse.GetResourceSpans()[0].GetScopeSpans()[0].GetSpans()[0].GetTraceId()) } @@ -207,11 +346,6 @@ func TestGRPCGatewayWithBasePathAndTLS(t *testing.T) { testGRPCGateway(t, "/jaeger", serverTLS, clientTLS) } -// For more details why this is needed see https://github.com/grpc-ecosystem/grpc-gateway/issues/2189 -type envelope struct { - Result json.RawMessage `json:"result"` -} - func TestGRPCGatewayWithTenancy(t *testing.T) { tenancyOptions := tenancy.Options{ Enabled: true, @@ -226,7 +360,7 @@ func TestGRPCGatewayWithTenancy(t *testing.T) { }) } -func TestTenancyGRPCRejection(t *testing.T) { +func TestGRPCGatewayTenancyRejection(t *testing.T) { basePath := "/" tenancyOptions := tenancy.Options{Enabled: true} gw := setupGRPCGateway(t, diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_FindTraces.json b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_FindTraces.json new file mode 100644 index 00000000000..5883e50eb1f --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_FindTraces.json @@ -0,0 +1,24 @@ +{ + "result": { + "resourceSpans": [ + { + "resource": {}, + "scopeSpans": [ + { + "scope": {}, + "spans": [ + { + "endTimeUnixNano": "11651379494838206464", + "name": "foobar", + "spanId": "AAAAAAAAALQ=", + "startTimeUnixNano": "11651379494838206464", + "status": {}, + "traceId": "AAAAAAAAAJYAAAAAAAAAoA==" + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetOperations.json b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetOperations.json new file mode 100644 index 00000000000..f56d8454189 --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetOperations.json @@ -0,0 +1,8 @@ +{ + "operations": [ + { + "name": "get_users", + "spanKind": "server" + } + ] +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetServices.json b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetServices.json new file mode 100644 index 00000000000..6a631d75f06 --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetServices.json @@ -0,0 +1,5 @@ +{ + "services": [ + "foo" + ] +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetTrace.json b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetTrace.json new file mode 100644 index 00000000000..5883e50eb1f --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithBasePathAndTLS_GetTrace.json @@ -0,0 +1,24 @@ +{ + "result": { + "resourceSpans": [ + { + "resource": {}, + "scopeSpans": [ + { + "scope": {}, + "spans": [ + { + "endTimeUnixNano": "11651379494838206464", + "name": "foobar", + "spanId": "AAAAAAAAALQ=", + "startTimeUnixNano": "11651379494838206464", + "status": {}, + "traceId": "AAAAAAAAAJYAAAAAAAAAoA==" + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_FindTraces.json b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_FindTraces.json new file mode 100644 index 00000000000..5883e50eb1f --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_FindTraces.json @@ -0,0 +1,24 @@ +{ + "result": { + "resourceSpans": [ + { + "resource": {}, + "scopeSpans": [ + { + "scope": {}, + "spans": [ + { + "endTimeUnixNano": "11651379494838206464", + "name": "foobar", + "spanId": "AAAAAAAAALQ=", + "startTimeUnixNano": "11651379494838206464", + "status": {}, + "traceId": "AAAAAAAAAJYAAAAAAAAAoA==" + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetOperations.json b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetOperations.json new file mode 100644 index 00000000000..f56d8454189 --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetOperations.json @@ -0,0 +1,8 @@ +{ + "operations": [ + { + "name": "get_users", + "spanKind": "server" + } + ] +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetServices.json b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetServices.json new file mode 100644 index 00000000000..6a631d75f06 --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetServices.json @@ -0,0 +1,5 @@ +{ + "services": [ + "foo" + ] +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetTrace.json b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetTrace.json new file mode 100644 index 00000000000..5883e50eb1f --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGatewayWithTenancy_GetTrace.json @@ -0,0 +1,24 @@ +{ + "result": { + "resourceSpans": [ + { + "resource": {}, + "scopeSpans": [ + { + "scope": {}, + "spans": [ + { + "endTimeUnixNano": "11651379494838206464", + "name": "foobar", + "spanId": "AAAAAAAAALQ=", + "startTimeUnixNano": "11651379494838206464", + "status": {}, + "traceId": "AAAAAAAAAJYAAAAAAAAAoA==" + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGateway_FindTraces.json b/cmd/query/app/apiv3/snapshots/TestGRPCGateway_FindTraces.json new file mode 100644 index 00000000000..5883e50eb1f --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGateway_FindTraces.json @@ -0,0 +1,24 @@ +{ + "result": { + "resourceSpans": [ + { + "resource": {}, + "scopeSpans": [ + { + "scope": {}, + "spans": [ + { + "endTimeUnixNano": "11651379494838206464", + "name": "foobar", + "spanId": "AAAAAAAAALQ=", + "startTimeUnixNano": "11651379494838206464", + "status": {}, + "traceId": "AAAAAAAAAJYAAAAAAAAAoA==" + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetOperations.json b/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetOperations.json new file mode 100644 index 00000000000..f56d8454189 --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetOperations.json @@ -0,0 +1,8 @@ +{ + "operations": [ + { + "name": "get_users", + "spanKind": "server" + } + ] +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetServices.json b/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetServices.json new file mode 100644 index 00000000000..6a631d75f06 --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetServices.json @@ -0,0 +1,5 @@ +{ + "services": [ + "foo" + ] +} \ No newline at end of file diff --git a/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetTrace.json b/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetTrace.json new file mode 100644 index 00000000000..5883e50eb1f --- /dev/null +++ b/cmd/query/app/apiv3/snapshots/TestGRPCGateway_GetTrace.json @@ -0,0 +1,24 @@ +{ + "result": { + "resourceSpans": [ + { + "resource": {}, + "scopeSpans": [ + { + "scope": {}, + "spans": [ + { + "endTimeUnixNano": "11651379494838206464", + "name": "foobar", + "spanId": "AAAAAAAAALQ=", + "startTimeUnixNano": "11651379494838206464", + "status": {}, + "traceId": "AAAAAAAAAJYAAAAAAAAAoA==" + } + ] + } + ] + } + ] + } +} \ No newline at end of file From 161e6a6163bb33d1e631d3221dbb28ffdc64f9f0 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 27 Dec 2023 17:15:57 -0500 Subject: [PATCH 2/6] comment Signed-off-by: Yuri Shkuro --- cmd/query/app/apiv3/grpc_gateway_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/query/app/apiv3/grpc_gateway_test.go b/cmd/query/app/apiv3/grpc_gateway_test.go index 118b412cb21..9a4220d83e8 100644 --- a/cmd/query/app/apiv3/grpc_gateway_test.go +++ b/cmd/query/app/apiv3/grpc_gateway_test.go @@ -53,6 +53,9 @@ const ( snapshotLocation = "./snapshots/" ) +// Snapshots can be regenerated via: +// +// REGENERATE_SNAPSHOTS=true go test -v ./cmd/query/app/apiv3/... var regenerateSnapshots = os.Getenv("REGENERATE_SNAPSHOTS") == "true" type testGateway struct { From 5a8a42da3d4745288d7f9ce6fb490cd07b1ebb48 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 27 Dec 2023 19:34:25 -0500 Subject: [PATCH 3/6] use gogo marshaler Signed-off-by: Yuri Shkuro --- cmd/query/app/apiv3/grpc_gateway_test.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/cmd/query/app/apiv3/grpc_gateway_test.go b/cmd/query/app/apiv3/grpc_gateway_test.go index 9a4220d83e8..7e3752bba17 100644 --- a/cmd/query/app/apiv3/grpc_gateway_test.go +++ b/cmd/query/app/apiv3/grpc_gateway_test.go @@ -15,6 +15,7 @@ package apiv3 import ( + "bytes" "context" "encoding/json" "fmt" @@ -29,7 +30,6 @@ import ( "time" "github.com/gorilla/mux" - "github.com/grpc-ecosystem/grpc-gateway/runtime" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -37,6 +37,8 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials" + gogojsonpb "github.com/gogo/protobuf/jsonpb" + gogoproto "github.com/gogo/protobuf/proto" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" @@ -171,13 +173,11 @@ func verifySnapshot(t *testing.T, body []byte) []byte { return body } -func parseResponse(t *testing.T, body []byte, obj interface{}) { - // TODO can we use gogo marshaler here instead of grcp-gateway? - jsonpb := &runtime.JSONPb{} - require.NoError(t, jsonpb.Unmarshal(body, obj)) +func parseResponse(t *testing.T, body []byte, obj gogoproto.Message) { + require.NoError(t, gogojsonpb.Unmarshal(bytes.NewBuffer(body), obj)) } -func parseChunkResponse(t *testing.T, body []byte, obj interface{}) { +func parseChunkResponse(t *testing.T, body []byte, obj gogoproto.Message) { // Unwrap the 'result' container generated by the gateway. // See https://github.com/grpc-ecosystem/grpc-gateway/issues/2189 type resultWrapper struct { @@ -246,9 +246,7 @@ func runGatewayGetServices(t *testing.T, gw *testGateway, setupRequest func(*htt require.Equal(t, http.StatusOK, statusCode) body = verifySnapshot(t, body) - var response struct { - Services []string `json:"services"` - } + var response api_v3.GetServicesResponse parseResponse(t, body, &response) assert.Equal(t, []string{"foo"}, response.Services) } @@ -265,12 +263,7 @@ func runGatewayGetOperations(t *testing.T, gw *testGateway, setupRequest func(*h require.Equal(t, http.StatusOK, statusCode) body = verifySnapshot(t, body) - var response struct { - Operations []struct { - Name string `json:"name"` - SpanKind string `json:"spanKind"` - } `json:"operations"` - } + var response api_v3.GetOperationsResponse parseResponse(t, body, &response) require.Len(t, response.Operations, 1) assert.Equal(t, "get_users", response.Operations[0].Name) From d9351bb946e383618155ca0c7cb48140e30beed2 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 27 Dec 2023 20:17:51 -0500 Subject: [PATCH 4/6] Verify parameter passing Signed-off-by: Yuri Shkuro --- cmd/query/app/apiv3/grpc_gateway_test.go | 9 +++++---- cmd/query/app/http_handler.go | 9 ++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/query/app/apiv3/grpc_gateway_test.go b/cmd/query/app/apiv3/grpc_gateway_test.go index 7e3752bba17..6cec8300b99 100644 --- a/cmd/query/app/apiv3/grpc_gateway_test.go +++ b/cmd/query/app/apiv3/grpc_gateway_test.go @@ -252,12 +252,13 @@ func runGatewayGetServices(t *testing.T, gw *testGateway, setupRequest func(*htt } func runGatewayGetOperations(t *testing.T, gw *testGateway, setupRequest func(*http.Request)) { + qp := spanstore.OperationQueryParameters{ServiceName: "foo", SpanKind: "server"} gw.reader. - On("GetOperations", matchContext, mock.AnythingOfType("spanstore.OperationQueryParameters")). + On("GetOperations", matchContext, qp). Return([]spanstore.Operation{{Name: "get_users", SpanKind: "server"}}, nil).Once() body, statusCode := gw.execRequest(t, &gatewayRequest{ - url: "/api/v3/operations", + url: "/api/v3/operations?service=foo&span_kind=server", setupRequest: setupRequest, }) require.Equal(t, http.StatusOK, statusCode) @@ -272,10 +273,10 @@ func runGatewayGetOperations(t *testing.T, gw *testGateway, setupRequest func(*h func runGatewayGetTrace(t *testing.T, gw *testGateway, setupRequest func(*http.Request)) { trace, traceID := makeTestTrace() - gw.reader.On("GetTrace", matchContext, matchTraceID).Return(trace, nil).Once() + gw.reader.On("GetTrace", matchContext, traceID).Return(trace, nil).Once() body, statusCode := gw.execRequest(t, &gatewayRequest{ - url: "/api/v3/traces/123", + url: "/api/v3/traces/" + traceID.String(), // hex string setupRequest: setupRequest, }) require.Equal(t, http.StatusOK, statusCode) diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 90d01a77cd0..46f6589ea31 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -138,12 +138,11 @@ func (aH *APIHandler) RegisterRoutes(router *mux.Router) { func (aH *APIHandler) handleFunc( router *mux.Router, f func(http.ResponseWriter, *http.Request), - route string, + routeFmt string, args ...interface{}, ) *mux.Route { - route = aH.route(route, args...) - var handler http.Handler - handler = http.HandlerFunc(f) + route := aH.formatRoute(routeFmt, args...) + var handler http.Handler = http.HandlerFunc(f) if aH.tenancyMgr.Enabled { handler = tenancy.ExtractTenantHTTPHandler(aH.tenancyMgr, handler) } @@ -154,7 +153,7 @@ func (aH *APIHandler) handleFunc( return router.HandleFunc(route, traceMiddleware.ServeHTTP) } -func (aH *APIHandler) route(route string, args ...interface{}) string { +func (aH *APIHandler) formatRoute(route string, args ...interface{}) string { args = append([]interface{}{aH.apiPrefix}, args...) return fmt.Sprintf("/%s"+route, args...) } From b2ed34047d6a227f4a3e4205911a688e4c3b7348 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 27 Dec 2023 20:51:06 -0500 Subject: [PATCH 5/6] better output Signed-off-by: Yuri Shkuro --- cmd/query/app/apiv3/grpc_gateway_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/query/app/apiv3/grpc_gateway_test.go b/cmd/query/app/apiv3/grpc_gateway_test.go index 6cec8300b99..e4c3b620df2 100644 --- a/cmd/query/app/apiv3/grpc_gateway_test.go +++ b/cmd/query/app/apiv3/grpc_gateway_test.go @@ -279,7 +279,7 @@ func runGatewayGetTrace(t *testing.T, gw *testGateway, setupRequest func(*http.R url: "/api/v3/traces/" + traceID.String(), // hex string setupRequest: setupRequest, }) - require.Equal(t, http.StatusOK, statusCode) + require.Equal(t, http.StatusOK, statusCode, "response=%s", string(body)) body = verifySnapshot(t, body) var spansResponse api_v3.SpansResponseChunk From 4ac994ba5c4f35240f75be3d89fbd1a47788670e Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 27 Dec 2023 20:53:32 -0500 Subject: [PATCH 6/6] fmt Signed-off-by: Yuri Shkuro --- cmd/query/app/apiv3/grpc_gateway_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/query/app/apiv3/grpc_gateway_test.go b/cmd/query/app/apiv3/grpc_gateway_test.go index e4c3b620df2..673d7912304 100644 --- a/cmd/query/app/apiv3/grpc_gateway_test.go +++ b/cmd/query/app/apiv3/grpc_gateway_test.go @@ -29,6 +29,8 @@ import ( "testing" "time" + gogojsonpb "github.com/gogo/protobuf/jsonpb" + gogoproto "github.com/gogo/protobuf/proto" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -37,8 +39,6 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials" - gogojsonpb "github.com/gogo/protobuf/jsonpb" - gogoproto "github.com/gogo/protobuf/proto" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/config/tlscfg"