fix(overrides): fall back to cluster default for unset retry_info_enabled

[zhxiaogg]

What this PR does:

RetryInfoEnabled is a plain bool, so a per-tenant override that never mentions retry_info_enabled is indistinguishable from one that explicitly sets it to false — both unmarshal to the zero value. Since getOverridesForUser returns a tenant's override struct as-is with no per-field merge against Defaults, any tenant with any override at all silently gets RetryInfoEnabled=false, regardless of the cluster-level default (true).

This changes the field to *bool, mirroring the existing ArtificialDelay *time.Duration pattern in the same struct, so nil means "not set" and the code falls back to the cluster default in that case. An explicit true/false from the tenant still takes priority.

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated (chloggen entry added)

[Copilot]

Pull request overview

This PR fixes an overrides-merging bug where a tenant that has any per-tenant override configured would unintentionally get ingestion.retry_info_enabled=false whenever they didn’t explicitly set retry_info_enabled, even if the cluster default is true. It does this by making RetryInfoEnabled a *bool so “unset” can be distinguished from “explicitly false”, and by updating the runtime overrides accessor to fall back to the cluster default when the per-tenant value is nil.

Changes:

• Change IngestionOverrides.RetryInfoEnabled (and legacy equivalent) from bool to *bool to preserve “unset” semantics.
• Update IngestionRetryInfoEnabled() to return the tenant override when set, otherwise fall back to the cluster default.
• Update/extend tests and add a .chloggen bugfix entry.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
modules/overrides/user_configurable_overrides_test.go	Removes a now-redundant local boolPtr helper (moved to package scope).
modules/overrides/runtime_config_overrides.go	Implements nil-aware fallback logic for RetryInfoEnabled.
modules/overrides/runtime_config_overrides_test.go	Adds coverage for default vs explicit tenant override vs “tenant override exists but field unset”.
modules/overrides/config.go	Changes RetryInfoEnabled to *bool, adds boolPtr helper, and sets the default to boolPtr(true).
modules/overrides/config_test.go	Updates legacy override test data for the *bool field.
modules/overrides/config_legacy.go	Updates legacy override struct field type and conversion mapping to new overrides.
modules/distributor/distributor_test.go	Updates struct literals to use *bool and adds a local boolPtr for tests.
.chloggen/retry-info-enabled-default-merge.yaml	Adds changelog entry for the bugfix.

[mapno]

LGTM. But I'd prefer to use the built-in new().

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (3)

modules/overrides/runtime_config_overrides_test.go:704

• CRITICAL: new(true) is invalid here (Go's new takes a type, not a value), so this test won't compile. Use the existing boolPtr(true) helper.

				Ingestion: IngestionOverrides{RetryInfoEnabled: new(true)},

modules/overrides/runtime_config_overrides_test.go:708

• CRITICAL: new(false) is invalid here (Go's new takes a type, not a value), so this test won't compile. Use the existing boolPtr(false) helper.

					"user1": {Ingestion: IngestionOverrides{RetryInfoEnabled: new(false)}},

modules/overrides/runtime_config_overrides_test.go:716

• CRITICAL: new(true) is invalid here (Go's new takes a type, not a value), so this test won't compile. Use the existing boolPtr(true) helper.

				Ingestion: IngestionOverrides{RetryInfoEnabled: new(true)},

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

modules/overrides/config.go:386

• CRITICAL: new(true) is not valid Go (the built-in new takes a type, not a value). This will not compile; use a bool variable and take its address (or a dedicated bool pointer helper).

	// Distributor LegacyOverrides
	// enabled in overrides by default, only takes effect when
	// distributor.retry_after_on_resource_exhausted is greater than 0.cluster level default is 5s.
	c.Defaults.Ingestion.RetryInfoEnabled = new(true)
	f.StringVar(&c.Defaults.Ingestion.RateStrategy, "distributor.rate-limit-strategy", "local", "Whether the various ingestion rate limits should be applied individually to each distributor instance (local), or evenly shared across the cluster (global).")

modules/overrides/runtime_config_overrides_test.go:699

• CRITICAL: new(true)/new(false) is not valid Go (the built-in new takes a type). Use an addressable bool (e.g., boolPtr(true)/boolPtr(false)) for these test fixtures; the same fix applies to the other new(...) occurrences in this test.

			name: "no tenant override: cluster default wins",
			defaultLimits: Overrides{
				Ingestion: IngestionOverrides{RetryInfoEnabled: new(true)},
			},
			expected: true,

modules/overrides/config_test.go:399

• CRITICAL: new(true) is not valid Go here; this test fixture will not compile. Use boolPtr(true) (already used elsewhere in this file) or take the address of a local bool variable.

		IngestionTenantShardSize:   3,
		IngestionMaxAttributeBytes: 1000,
		IngestionArtificialDelay:   durationPtr(5 * time.Minute),
		IngestionRetryInfoEnabled:  new(true),

modules/distributor/distributor_test.go:2194

• CRITICAL: new(tt.overrideRetryInfoEnabled) is not valid Go (the built-in new takes a type). You can create a *bool inline via a tiny helper closure so this test compiles.

				Defaults: overrides.Overrides{
					Ingestion: overrides.IngestionOverrides{
						RetryInfoEnabled: new(tt.overrideRetryInfoEnabled),
					},