Extend TraceRedactor interface and add ErrTraceHidden - #6811
Merged
Conversation
stoewer
force-pushed
the
extend-trace-redactor-interface
branch
2 times, most recently
from
March 27, 2026 11:42
0b62d10 to
bee854b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces two main enhancements: (1) extends the frontend TraceRedactor hook to return errors (including a sentinel ErrTraceHidden to translate to 404/NotFound), and (2) adds a typed “extensions” mechanism to the per-tenant overrides configuration with registry-backed validation and legacy flat-key compatibility.
Changes:
- Update
TraceRedactor.RedactTraceAttributesto returnerror, addErrTraceHidden, and translate it to HTTP 404 / gRPC NotFound in combiners. - Add an overrides extension registry (
RegisterExtension) plus decoding/validation logic for YAML/JSON and legacy flat-key conversion. - Update config/runtime-config unmarshalling behavior to properly distinguish new-format extension errors vs legacy fallback, and add extensive tests for overrides extensions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/overrides/runtime_config_overrides.go | Adjust legacy-fallback logic to avoid swallowing extension-related errors. |
| modules/overrides/extension.go | Introduce extension registry + typed decode/validate + legacy flat-key processing. |
| modules/overrides/extension_test.go | Add coverage for extension registration, JSON/YAML marshal/unmarshal, and legacy conversions. |
| modules/overrides/config.go | Add Overrides.Extensions support; JSON/YAML marshal/unmarshal hooks; legacy fallback adjustments. |
| modules/overrides/config_legacy.go | Add LegacyOverrides.Extensions and JSON/YAML flattening/unflattening for legacy wire format. |
| modules/overrides/config_test.go | Update config tag tests and field-population helper for new Extensions field. |
| modules/frontend/combiner/common.go | Add ErrTraceHidden, update TraceRedactor interface, and map hidden traces to NotFound. |
| modules/frontend/combiner/trace_by_id.go | Handle ErrTraceHidden as HTTP 404 for v1 trace-by-id responses. |
| modules/frontend/combiner/trace_by_id_v2.go | Propagate redactor errors so genericCombiner can translate ErrTraceHidden. |
| CHANGELOG.md | Add changelog entry for overrides extension mechanism. |
stoewer
force-pushed
the
extend-trace-redactor-interface
branch
from
March 31, 2026 09:56
bee854b to
cb2600e
Compare
stoewer
force-pushed
the
extend-trace-redactor-interface
branch
from
March 31, 2026 10:52
73b201c to
f19bbf0
Compare
stoewer
requested review from
carles-grafana,
electron0zero,
ie-pham,
javiermolinar,
mattdurham,
oleg-kozlyuk-grafana,
ruslan-mikhailov,
yvrhdn and
zalegrala
as code owners
March 31, 2026 12:14
Comment on lines
+132
to
+140
| err := c.traceRedactor.RedactTraceAttributes(traceResult) | ||
| if err != nil { | ||
| if errors.Is(err, ErrTraceHidden) { | ||
| return &http.Response{ | ||
| StatusCode: http.StatusNotFound, | ||
| Body: http.NoBody, | ||
| Header: http.Header{}, | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
When ErrTraceHidden is returned, HTTPFinal returns a 404 response but doesn't update the combiner's internal c.code / c.statusMessage. That means StatusCode() can still report 200 after a hidden-trace response, which is inconsistent with genericCombiner (it sets httpStatusCode = 404) and may confuse any callers relying on StatusCode() for metrics/control flow. Consider setting c.code = http.StatusNotFound (and clearing c.statusMessage) before returning the 404 response here.
ie-pham
approved these changes
Mar 31, 2026
mattdurham
pushed a commit
to mattdurham/tempo
that referenced
this pull request
Jun 18, 2026
* Add ErrTraceHidden to combiner and handle it correctly * Reuse ErrTraceHidden text * Use http.NoBody for empty responses * Set internal httpStatusCode consistently in combiner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does:
The method
TraceRedactor.RedactTraceAttributes()can now return an error. When the error isErrTraceHidden, this signals that the trace should not be returned. Endpoints should return 404 (not found) instead.This allows
TraceRedactorimplementation to support a wider range of redaction modes.Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]