Skip to content

fix: merge per-tenant overrides with defaults for unset fields#6592

Draft
electron0zero wants to merge 8 commits intografana:mainfrom
electron0zero:overrides_fall_chain_bug
Draft

fix: merge per-tenant overrides with defaults for unset fields#6592
electron0zero wants to merge 8 commits intografana:mainfrom
electron0zero:overrides_fall_chain_bug

Conversation

@electron0zero
Copy link
Member

@electron0zero electron0zero commented Mar 2, 2026

What this PR does:

Fixes a bug where per-tenant and wildcard overrides that only set a few fields cause all other fields to resolve to Go zero values instead of falling back to static defaults.

Bug: When a tenant has an override entry that sets only a subset of fields (e.g., just max_cardinality_per_label: 100), all other fields resolve to Go zero values instead of the static defaults. This is because the override struct is returned as-is without merging in the defaults for unset fields.

Fix: Each override entry is now field-level merged with static defaults at config load time using reflection. Unset fields fall back to defaults instead of being zero values. Merged views are pre-computed at load/reload - zero cost per request.

Fields where zero is semantically meaningful (e.g., 0 = "no limit", false = "disabled") are migrated to pointer types so the merge can distinguish "not set" (nil) from "explicitly zero" (*0).

This is needed because the merge uses "is this field zero?" to decide whether a field was set in the override file.

For most fields this works fine - a zero value means "not set". But some fields use zero as a real value: max_bytes_per_trace: 0 means "no limit", span_multiplier_key: "" means "disable multiplier".

Without pointers, the merge would treat these as "not set" and overwrite them with the default, making it impossible for operators to explicitly set these values.

Pointer types solve this because we can check nil and assign it to "not set, fall back to default", non-nil zero = "explicitly set to this value".

Behavior before this PR:

  • Lookup returns the first matching override struct whole (per-tenant OR wildcard OR defaults)
  • If a per-tenant override exists, it is returned as-is - fields not set in the override file resolve to Go zero values, not the static defaults
  • If a wildcard tenant override exists, it is returned as-is - fields not set in the override file resolve to Go zero values, not the static defaults
  • if both per tenant and wildcard tenant override doesn't exist, returns default

Behavior after this PR:

  • Lookup order is unchanged, it's still per-tenant OR wildcard OR defaults
  • The matched override entry is now field-level merged with static defaults - fields not set in the override file correctly fall back to the static defaults

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@electron0zero electron0zero changed the title WIP: test for the overrrides chain bug fix(overrides): field-level fallback chain for wildcard overrides Mar 2, 2026
@electron0zero electron0zero force-pushed the overrides_fall_chain_bug branch 3 times, most recently from 10fb6e7 to 098a9fe Compare March 10, 2026 20:04
@electron0zero
Copy link
Member Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 098a9fedcc

ℹ️ 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".

@electron0zero
Copy link
Member Author

@codex review again

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe6f310425

ℹ️ 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".

@electron0zero electron0zero force-pushed the overrides_fall_chain_bug branch 3 times, most recently from 670a862 to ec68bd7 Compare March 12, 2026 20:40
@electron0zero electron0zero force-pushed the overrides_fall_chain_bug branch from ec68bd7 to 6865ad0 Compare March 13, 2026 21:48
@electron0zero electron0zero changed the title fix(overrides): field-level fallback chain for wildcard overrides fix: merge per-tenant overrides with defaults overrides Mar 13, 2026
@electron0zero electron0zero changed the title fix: merge per-tenant overrides with defaults overrides fix: per-tenant overrides return zero instead of defaults for unset fields Mar 13, 2026
@electron0zero electron0zero changed the title fix: per-tenant overrides return zero instead of defaults for unset fields fix: merge per-tenant overrides with defaults for unset fields Mar 13, 2026
@electron0zero electron0zero force-pushed the overrides_fall_chain_bug branch from 682c9dd to fb79118 Compare March 13, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant