tempo-cli: rewrite migrate overrides-config and add per-tenant migration command#6793
Merged
electron0zero merged 5 commits intografana:mainfrom Mar 30, 2026
Conversation
The command was abusing WriteStatusRuntimeConfig to extract migrated overrides, which dumped all defaults making the output unusable. Use Config.UnmarshalYAML which already handles legacy conversion, and diff against defaults to output only user-set values.
Add a new CLI command to migrate per-tenant override files from the legacy flat format to the new scoped format. Export UnmarshalPerTenantOverrides in the overrides package to avoid duplicating the legacy conversion logic. Add omitempty to generate_native_histograms and ingestion_time_range_slack yaml tags to prevent zero values leaking into output.
electron0zero
requested review from
carles-grafana,
ie-pham,
javiermolinar,
mattdurham,
oleg-kozlyuk-grafana,
ruslan-mikhailov,
stoewer and
zalegrala
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates tempo-cli migration tooling to support the new scoped overrides format by improving how overrides are parsed and re-emitted, and by adding a new per-tenant migration command.
Changes:
- Reworks
tempo-cli migrate overrides-configto unmarshal the overrides config directly and emit only values that differ from defaults. - Adds
tempo-cli migrate overrides-per-tenantto migrate legacy per-tenant override files into the new scoped format. - Exposes
UnmarshalPerTenantOverridesfrom the overrides module and adjusts YAML tags to avoid emitting zero values for specific fields.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
modules/overrides/runtime_config_overrides.go |
Exports a strict per-tenant overrides unmarshaller for reuse by tempo-cli. |
modules/overrides/config.go |
Adds omitempty to specific YAML tags to prevent zero values appearing in output. |
cmd/tempo-cli/main.go |
Wires new/updated migrate subcommands and help text into the CLI. |
cmd/tempo-cli/cmd-migrate-overrides-config.go |
Reimplements overrides-config migration via YAML round-trip + diff against defaults. |
cmd/tempo-cli/cmd-migrate-overrides-config_test.go |
Adds tests validating legacy/new-format config migration output. |
cmd/tempo-cli/cmd-migrate-overrides-per-tenant.go |
Adds per-tenant overrides migration command using exported unmarshal helper. |
cmd/tempo-cli/cmd-migrate-overrides-per-tenant_test.go |
Adds tests validating legacy/new-format per-tenant migration output. |
cmd/tempo-cli/test-data/migrate-overrides-*/*.yaml |
Adds fixtures for legacy/new input and expected migrated output. |
3 tasks
Update the existing migrate overrides-config command docs to reflect the rewritten implementation and add docs for the new migrate overrides-per-tenant command.
- Fix struct field alignment in main.go (check-fmt) - Regenerate manifest after omitempty tag changes (generate-manifest) - Use separate config instance for defaults to avoid shared pointers
Member
Author
|
@codex review please |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
3 tasks
mapno
approved these changes
Mar 30, 2026
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:
Follow-up to #6741 (disable legacy overrides by default).
migrate overrides-configto useConfig.UnmarshalYAMLdirectly instead ofWriteStatusRuntimeConfig, which dumped all defaults making the output unusable. Now diffs against defaults to output only user-set values.migrate overrides-per-tenantcommand for migrating per-tenant override files from legacy flat format to the new scoped format.UnmarshalPerTenantOverridesin the overrides package to reuse existing legacy conversion logic.omitemptytogenerate_native_histogramsandingestion_time_range_slackyaml tags to prevent zero values leaking into output.Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]