From 57a5f8365dacbb9455e0384cfe55ff7630f3b2d4 Mon Sep 17 00:00:00 2001 From: Andreas Gerstmayr Date: Tue, 22 Sep 2026 16:19:22 +0200 Subject: [PATCH] traces/rbac: use gogo/protobuf to match Tempo marshalling lib The response RBAC modifier unmarshals a Tempo API response, filters it, and re-marshals it. To stay compliant with Tempo's API, that round-trip must produce output identical to what Tempo sends, which means it has to (un)marshal with the same library Tempo uses: gogo/protobuf. This code used golang/protobuf's jsonpb instead, which disagrees with gogo on empty repeated fields (gogo emits "[]", golang/protobuf omits them). A zero-result /api/search therefore came back without the "traces" field instead of Tempo's {"traces":[],...}. Switch to gogo's jsonpb/proto so the modifier's output matches Tempo marshalling and "traces" field is always present. Co-Authored-By: Claude Opus 4.8 --- api/traces/v1/trace_rbac.go | 4 ++-- api/traces/v1/trace_rbac_test.go | 21 ++++++++++++++++++++- go.mod | 2 +- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/api/traces/v1/trace_rbac.go b/api/traces/v1/trace_rbac.go index 2554446c0..1b95af0e0 100644 --- a/api/traces/v1/trace_rbac.go +++ b/api/traces/v1/trace_rbac.go @@ -11,8 +11,8 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" - "github.com/golang/protobuf/jsonpb" // nolint:staticcheck - "github.com/golang/protobuf/proto" //nolint:staticcheck + "github.com/gogo/protobuf/jsonpb" + "github.com/gogo/protobuf/proto" "github.com/grafana/tempo/pkg/tempopb" commonv1 "github.com/grafana/tempo/pkg/tempopb/common/v1" tracev1 "github.com/grafana/tempo/pkg/tempopb/trace/v1" diff --git a/api/traces/v1/trace_rbac_test.go b/api/traces/v1/trace_rbac_test.go index a277b5bea..3d84ce31b 100644 --- a/api/traces/v1/trace_rbac_test.go +++ b/api/traces/v1/trace_rbac_test.go @@ -702,7 +702,7 @@ func TestResponseRBACModifier(t *testing.T) { ] }, "scopeSpans": [ - {"scope": {}, "spans": [{}]} + {"scope": {"attributes": []}, "spans": [{"attributes": [], "events": []}]} ] } ] @@ -756,6 +756,25 @@ func TestResponseRBACModifier(t *testing.T) { }`, string(body)) }) + t.Run("search endpoint preserves empty traces array", func(t *testing.T) { + // A zero-result search response from Tempo includes an empty (but + // present) "traces" array. The RBAC modifier must not drop that field, + // so clients can reliably rely on it being present. + resp := makeResponse(ctx, http.StatusOK, "/api/search", `{ + "traces": [], + "metrics": {"inspectedBytes": "1868359", "completedJobs": 3, "totalJobs": 3} +}`, nil) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.JSONEq(t, `{ + "traces": [], + "metrics": {"inspectedBytes": "1868359", "completedJobs": 3, "totalJobs": 3} +}`, string(body)) + assert.Contains(t, string(body), `"traces":[]`, "traces field must be present as an empty array") + }) + t.Run("search tag values endpoint is not modified", func(t *testing.T) { resp := makeResponse(ctx, http.StatusOK, "/api/v2/search/tag/resource.service.name/values", `{ "tagValues": [ diff --git a/go.mod b/go.mod index 9c7b3efe1..1ce64f663 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/go-kit/log v0.2.1 github.com/go-openapi/runtime v0.32.6 github.com/go-openapi/strfmt v0.27.0 + github.com/gogo/protobuf v1.3.2 github.com/golang-jwt/jwt/v4 v4.5.2 github.com/golang/protobuf v1.5.4 github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 @@ -94,7 +95,6 @@ require ( github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/gobwas/glob v0.2.3 // indirect github.com/goccy/go-json v0.10.6 // indirect - github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect