fix: preserve empty traces/resourceSpans arrays in RBAC-filtered trace responses

[IshwarKanse]

Summary

/api/search and /api/v2/traces/{id} responses lose their "traces" or "resourceSpans" key entirely when query RBAC is enabled (WithTraceQLNamespaceSelectAndForbidOtherAPIs/responseRBACModifier), instead of correctly returning an empty array. This crashes downstream consumers that assume the field is always present when parsing a JSON response — we hit this via the OpenShift Distributed Tracing console plugin (using the Grafana/Perses Tempo datasource), reported as TRACING-6841.

Root cause

responseRBACModifier's JSON path unmarshals and re-marshals the response using github.com/golang/protobuf/jsonpb, a thin wrapper around protojson. tempopb.SearchResponse and tempopb.TraceByIDResponse are both gogo/protobuf-generated types (like tempopb.Trace, which already gets special-cased via tempopb.MarshalToJSONV1/UnmarshalFromJSONV1 for exactly this reason — see the comments already in this file at the routeQueryV1 case). The two jsonpb implementations disagree on whether a populated-but-empty repeated field is distinct from an absent one:

• github.com/golang/protobuf/jsonpb (via protojson) collapses an explicit empty array to a nil Go slice on unmarshal, and omits a nil-or-zero-length slice on marshal regardless of how it got that way — so the key never makes it back out, even once.
• github.com/gogo/protobuf/jsonpb preserves the nil vs. populated-but-empty distinction on both ends.

Verified empirically by round-tripping raw empty-array payloads through all four combinations of {gogo, golang/protobuf} x {unmarshal, marshal}: only gogo-unmarshal -> gogo-marshal preserves the empty array in the output — every combination touching golang/protobuf on either end drops the key.

Fix

Switch the shared unmarshal/marshal helpers to github.com/gogo/protobuf/jsonpb for the JSON content-type path, since every tempopb.* type they handle (SearchResponse, TraceByIDResponse) is gogo/protobuf-generated — matching the existing tempopb.Trace special-casing already in this file. This fixes both /api/search and /api/v2/traces/{id} with one change. The protobuf content-type path (added in #924 for the Grafana client) is unchanged and unaffected — the wire format has no separate representation for "empty but present" vs. "absent" to begin with, so proto.Marshal/proto.Unmarshal round-trip correctly there regardless of library.

Testing

Unit tests:

• TestSearchResponseEmptyTracesPreserved and TestTraceByIDResponseEmptyResourceSpansPreserved: direct round-trip tests of unmarshal/marshal against explicit empty-array payloads for both response types.
• TestResponseRBACModifier/search_endpoint_with_zero_results_keeps_traces_as_an_empty_array: end-to-end test through the actual HTTP response modifier with a realistic zero-result Tempo response body.
• TestResponseRBACModifier/v2_trace_endpoint: updated to assert the fixed (non-omitted) redacted-attribute shape.
• Existing TestRBACSearchResult, TestResponseRBACModifier (search endpoint, non-empty), and TestResponseRBACModifierProtobuf (search endpoint protobuf) all still pass unmodified — RBAC filtering behavior and the protobuf path are unaffected.

Real OpenShift cluster validation:

• Built this branch into an observatorium-api image and ran it as the Tempo Operator Gateway's image (TempoStack.spec.images.tempoGateway) on a real OpenShift 4.22 cluster, alongside the OpenShift Distributed Tracing console plugin.
• Ran the console plugin's full Cypress e2e suite against it: all 14 applicable tests passed, including a zero-result TraceQL search through this exact Gateway-enabled TempoStack — the precise scenario that previously crashed — and the RBAC/namespace-redaction test.
• Re-ran the same suite with the stock, unpatched console plugin image (no plugin-side defensive fix) and only this fix applied to the Gateway: still 14/14 passing, confirming the fix alone resolves the crash for consumers going through query RBAC, independent of any client-side workaround.
• Confirmed via tempo-operator's own Gateway e2e/chainsaw tests (multitenancy-rbac) that RBAC-filtered trace generation, viewing, and redaction all work correctly with this fix in place.
• Checked for regressions against tempo-operator's Gateway e2e/chainsaw tests: none assert on raw JSON shape for /api/search (all use jq ".traces | length", which evaluates identically whether the key is missing or an empty array), so this is not a breaking change for existing consumers.

Other checks:

• go build ./..., go vet ./..., gofmt -l, and the full go test ./... suite pass.
• go mod tidy produces no diff.

Note on CI: ci/circleci: test flaked on this PR a couple of times on TestWithSharedRateLimiter/one_rate_limiter_with_additional_retry_after_logic in the unrelated ratelimit package (confirmed via two separate CI runs producing two different scrambled Retry-After orderings, and a zero diff between this branch and main for ratelimit/). Root cause and fix are in #936.

Update: per review feedback, removed the two test/e2e RBAC tests — the unit test coverage above plus the real OpenShift cluster validation already exercise this change thoroughly, so the extra real-Tempo e2e tests weren't worth their added CI time.

[andreasgerstmayr]

imo the e2e test is overkill for this change, as the unit test already covers it - otherwise LGTM 👍

[IshwarKanse]

@andreasgerstmayr agreed — removed both test/e2e RBAC tests (TestTracesTempoSearchZeroResultsWithQueryRBAC and its non-empty companion) in 500a61e. The unit tests in trace_rbac_test.go already cover the round-trip logic through the actual responseRBACModifier/unmarshalSearchResponse/marshalSearchResponse code paths, and this was validated further via a full OpenShift cluster + Tempo Operator Gateway + console plugin Cypress suite as noted in the PR description, so the extra e2e coverage wasn't pulling its weight against the added CI time.