fix: processor-level config takes precedence over top-level settings - #984
fix: processor-level config takes precedence over top-level settings#984lazureykis wants to merge 1 commit into
Conversation
|
Sorry for the delay and thanks for this. I'm trying to be certain about this. Another issue is sadly another way of configuring things needs to get in soon and will complicate this even more. But I'll likely merge this today because off the top of my head it seems right. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #984 +/- ##
=======================================
Coverage 17.59% 17.59%
=======================================
Files 24 24
Lines 722 722
=======================================
Hits 127 127
Misses 595 595
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@tsloughter Friendly ping — I think this is safe to merge independently of #769: declarative config ignores env/ |
Previously, explicitly setting `traces_exporter` or `bsp_*` options at the top level would silently override the same keys in processor-level config. This meant a user who configured `exporter` directly in their processor opts could have it replaced by a top-level `traces_exporter` value without warning. The fix ensures processor opts always take precedence: top-level and environment variable settings only fill in keys that are absent from the processor config. The `span_processor` shorthand (`batch`/`simple`) now starts with empty opts instead of pre-populated defaults, since defaults are already applied via `maps:merge` in `merge_processor_config`. Closes open-telemetry#976
4337f68 to
069800d
Compare
Summary
Fixes #976.
When both
traces_exporter(orbsp_*settings) and processor-level config are set, the processor-level config was silently overridden by the top-level value. For example:The batch processor would end up using
other_exporterinstead ofmy_exporter.Changes
merge_processor_config_/4: Simplified to always respect processor-level config — if a key already exists in processor opts, top-level/env values don't override it.transform(span_processor, batch|simple): Returns empty#{}instead ofBATCH_PROCESSOR_DEFAULTS/SIMPLE_PROCESSOR_DEFAULTS. Defaults are already applied viamaps:mergeinmerge_processor_config/4, so pre-populating was redundant and prevented the merge logic from distinguishing user-set values from defaults.is_default/3helper.New precedence order
processor opts > top-level/env config > BATCH_PROCESSOR_DEFAULTS
Tests
Added
processor_exporter_not_overridden(12 assertions) andprocessor_config_precedence_with_os_env(4 assertions) covering:traces_exporternoneexporter preservedbsp_*/ssp_*settings don't override processor-level equivalentsbsp_*/ssp_*still apply when keys are absent from processor optsOTEL_TRACES_EXPORTER) doesn't override processor-level exporter