-
Notifications
You must be signed in to change notification settings - Fork 530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add HTML pages to view tenants with overrides #3332
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc update looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested most of this locally and LGTM. One thing I noticed is that when browsing to a non-existent tenant like /status/overrides/doesnotexist
it prints the inherited settings from tenant *
as expected - but is there a way to indicate that it's using the defaults and not configured specifically? Not a blocker, just curious. Also left a few nits for things that I think would improve readability. Non-blocking.
for _, tenant := range userConfigurableOverridesManager.GetTenantIDs() { | ||
_, hasRuntimeOverrides := tenants[tenant] | ||
|
||
tenants[tenant] = tenantsPageTenant{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Can this be simplified? Either by using *tenantsPageTenant
or making use of how tenants[tenant]
will return an uninitialized struct (with HasRunTimeOverrides=false)
I gave this a shot and it gets a bit complicated because a tenant can be using its own runtime overrides, the wildcard ( tempo/modules/overrides/runtime_config_overrides.go Lines 484 to 498 in 21fde18
I added a text line that tells you where it found the overrides, I hope this is clear enough. If a tenant has its own runtime overrides: If a tenant does not have its own runtime overrides, but we have If neither is set and we fall back to the |
What this PR does:
Changes
/status/overrides
and/status/overrides/{tenant}
to HTML page which are nicer and easier to use.Examples:
/status/overrides
with user-configurable overrides disabled/status/overrides/{tenant}
with user-configurable overrides disabled/status/overrides
with user-configurable overrides enabled/status/overrides/{tenant}
with user-configurable overrides enabledWhich issue(s) this PR fixes:
Fixes #Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]