Add extension mechanism for per-tenant Overrides#6758
Merged
stoewer merged 30 commits intografana:mainfrom Mar 31, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an extension mechanism to Tempo’s per-tenant overrides so downstream projects that vendor Tempo can register and decode their own override blocks/keys alongside built-in limits.
Changes:
- Introduces an extension registry (
RegisterExtension) and processing logic for YAML/JSON (new-format nested keys and legacy flat keys). - Refactors
Overrides/LegacyOverridesto embed “base” structs and implement custom marshal/unmarshal to support extensions while preserving legacy conversions. - Updates runtime-config and user-configurable override tests to account for the new embedded-struct layout and extension handling.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/overrides/extension.go | Adds extension interface, registry, and processing helpers. |
| modules/overrides/extension_test.go | Adds test coverage for extension registration and JSON/YAML + legacy/new round trips. |
| modules/overrides/config.go | Refactors Overrides and config unmarshalling to support extensions. |
| modules/overrides/config_legacy.go | Refactors LegacyOverrides and legacy conversion/marshal/unmarshal to support extensions. |
| modules/overrides/runtime_config_overrides.go | Updates per-tenant runtime overrides YAML decoding and legacy conversion flow. |
| modules/overrides/config_test.go | Updates config/tag tests and adds tests asserting unknown legacy keys are rejected. |
| modules/overrides/runtime_config_overrides_test.go | Adjusts tests for embedded baseOverrides initialization. |
| modules/overrides/interface_test.go | Adjusts tests for embedded baseOverrides initialization. |
| modules/overrides/user_configurable_overrides_test.go | Adjusts tests for embedded baseOverrides initialization. |
| modules/overrides/userconfigurable/api/api_test.go | Adjusts defaults setup for the refactored Overrides struct. |
| modules/overrides/userconfigurable/api/limits_test.go | Adjusts defaults setup for the refactored Overrides struct. |
stoewer
force-pushed
the
extend-tempo-config
branch
from
08bb725 to
5d0f993
Compare
stoewer
changed the title
stoewer
changed the title
Comment on lines
+141
to
+146
| // Already a typed Extension (set programmatically or after legacy conversion) | ||
| if ext, alreadyTyped := raw.(Extension); alreadyTyped { | ||
| if err := ext.Validate(); err != nil { | ||
| return &extensionError{fmt.Errorf("extension %q: %w", key, err)} | ||
| } | ||
| continue |
There was a problem hiding this comment.
In processExtensions, the "already typed" fast-path validates the Extension but does not verify that the map key matches the extension's declared Key(). If a caller programmatically stores a typed instance under the wrong key, it will validate successfully but later marshal/unmarshal behavior will be inconsistent and hard to debug.
Consider rejecting this case by checking ext.Key() == key and returning an error (wrapped as extensionError) when it doesn't match.
Contributor
Author
There was a problem hiding this comment.
This is negligible in my opinion
stoewer
requested review from
carles-grafana,
electron0zero,
ie-pham,
javiermolinar,
mapno,
mattdurham,
oleg-kozlyuk-grafana,
ruslan-mikhailov,
yvrhdn and
zalegrala
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36bde18a28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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:
Add extension mechanism for Temp's per-tenant Overrides. This enables applications who vendor tempo to add their own Overrides.
Which issue(s) this PR fixes:
Contributes to grafana/cloud-traces#270
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]