refactor(config): add cross-product gate to prevent conflicting programmatic overrides#4640
refactor(config): add cross-product gate to prevent conflicting programmatic overrides#4640
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files
🚀 New features to boost your workflow:
|
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 4d980f7 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback! |
BenchmarksBenchmark execution time: 2026-04-09 20:39:13 Comparing candidate commit 4d980f7 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 215 metrics, 8 unstable metrics.
|
| if c.serviceMappings == nil { | ||
| c.serviceMappings = make(map[string]string) | ||
| } | ||
| c.serviceMappings[from] = to |
There was a problem hiding this comment.
Is there a reason why we don't need to check for a "conflict" here too?
There was a problem hiding this comment.
| func (c *Config) SetFeatureFlags(features []string, origin telemetry.Origin) { | ||
| func (c *Config) SetFeatureFlags(features []string, origin telemetry.Origin, product ...Product) { | ||
| c.mu.Lock() | ||
| if c.featureFlags == nil { |
There was a problem hiding this comment.
Similar question/confirmation as below: what if c.featureFlags is not nil and contains some of the same keys as what we're adding? Since we're not checking for existing flags, it'll prioritize flags that come later?
There was a problem hiding this comment.
This is a good catch, but the behavior is intentional.
The cross-product gate prevents two products from disagreeing on a single, conflicting value (e.g., service name). By contrast, feature flags and service mappings are "additive" configs — each call adds entries to a shared collection rather than replacing a value, so there's no conflict to guard against.
I've added tests to showcase this behavior in 5733188
| return false | ||
| } | ||
| p := product[0] | ||
| if prev, exists := c.overrides[field]; exists && prev.product != p { |
There was a problem hiding this comment.
Can c.overrides be nil? Should we also check and initialize it to an empty map like we are elsewhere?
There was a problem hiding this comment.
It can never be nil because I initialize it to an empty map in the Config initialization ;)
internal/config/config.go
Outdated
| // The product parameter is variadic for ergonomic pass-through from Set* methods; | ||
| // only the first value is used and callers should pass at most one. | ||
| // Must be called while c.mu is held. | ||
| func (c *Config) productConflict(field string, origin telemetry.Origin, value any, product ...Product) bool { |
There was a problem hiding this comment.
I think it reflects more correctly what this function does.
| func (c *Config) productConflict(field string, origin telemetry.Origin, value any, product ...Product) bool { | |
| func (c *Config) checkProductConflict(field string, origin telemetry.Origin, value any, product ...Product) bool { |
What does this PR do?
Adds a cross-product gate to internal/config that prevents different products (tracer, profiler, etc.) from setting conflicting values for the same configuration field via programmatic APIs. When a conflict is attempted, a new telemetry metric
config.product_conflictis emitted.Every
Set*method now accepts an optional...Productparameter. When a product passes its identity (e.g.ProductTracer), the gate records that claim. If a different product later tries to set the same field, the call is rejected (first-in-wins) and a warning is logged.Env vars, defaults, and non-programmatic origins bypass the gate entirely.
Tests and integrations omit the product parameter and are unaffected.
Motivation
Previously, each product (tracer, profiler, etc) loaded configuration independently from all sources and stored its own local copy. Programmatic APIs like
tracer.WithServiceorprofiler.WithEnvwould overwrite only that product's local copy. As we consolidate into a single shared Config instance in internal/config, programmatic APIs from different products now write to the same fields. The gate ensures these writes don't silently conflict: the first product to claim a field via programmatic API owns it, and attempts from other products are rejected with a warning.Reviewer's Checklist
make lintlocally.make testlocally.make generatelocally.make fix-moduleslocally.Unsure? Have a question? Request a review!