Repository navigation
[Configuration] Ignore unsupported OTEL_PROPAGATORS values with a warning #9515
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
base: master
Are you sure you want to change the base?
Changes from all commits
aafd374
308c6a8
15bfd58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -529,9 +529,34 @@ not null when string.Equals(value, "otlp", StringComparison.OrdinalIgnoreCase) = | |
| var otelPropagation = config | ||
| .WithKeys(ConfigurationKeys.OpenTelemetry.Propagators) | ||
| .GetAsClassResult( | ||
| validator: injectionValidator, // invalid individual values are rejected later | ||
| validator: styles => styles.Any(IsSupportedOtelPropagator), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With the default composite configuration source and only OTEL_PROPAGATORS=jaeger configured, this validator rejects the value. CompositeConfigurationSource.GetAs discards invalid results and returns NotFound when no source validates, so the new IsPresent branch never emits the promised warning or OpenTelemetryConfigInvalid metric. Propagation falls back to defaults silently. Preserve invalid presence when no source validates while retaining fallback to valid lower-priority sources, and cover the unsupported-only composite case. This requires changes beyond a localized replacement in this diff. React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it What each reaction means
🤖 Bits Code Review · Open Bits AI session
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This issue seems to be gue to composite config, fix proposed in #9522 |
||
| converter: otelConverter); | ||
|
|
||
| // Filter the selected value, rather than in the converter, which runs once per source | ||
| if (otelPropagation.ConfigurationResult is { Result: { } otelPropagators, IsValid: true }) | ||
|
MilanGarnier marked this conversation as resolved.
|
||
| { | ||
| var propagatorsBySupport = otelPropagators.ToLookup(IsSupportedOtelPropagator); | ||
|
MilanGarnier marked this conversation as resolved.
|
||
| if (propagatorsBySupport.Contains(false)) | ||
| { | ||
| ErrorLog.LogUnsupportedValues(otelPropagation.Key, [..propagatorsBySupport[false]]); | ||
| otelPropagation = new( | ||
| otelPropagation.Telemetry, | ||
| otelPropagation.Key, | ||
| otelPropagation.RecordValue, | ||
| ConfigurationResult<string[]>.Valid([..propagatorsBySupport[true]], otelPropagation.ConfigurationResult.TelemetryOverride)); | ||
| } | ||
| } | ||
| else if (otelPropagation.ConfigurationResult.IsPresent) | ||
| { | ||
| // Report once, instead of once per OverrideWith call (inject and extract) | ||
| ErrorLog.LogInvalidConfiguration(otelPropagation.Key); | ||
| otelPropagation = new( | ||
| otelPropagation.Telemetry, | ||
| otelPropagation.Key, | ||
| otelPropagation.RecordValue, | ||
| ConfigurationResult<string[]>.NotFound()); | ||
| } | ||
|
|
||
| PropagationStyleInject = config | ||
| .WithKeys(ConfigurationKeys.PropagationStyleInject) | ||
| .GetAsClassResult( | ||
|
|
@@ -1676,6 +1701,10 @@ internal string GetDefaultHttpClientExclusions() | |
| } | ||
| } | ||
|
|
||
| private static bool IsSupportedOtelPropagator(string propagator) | ||
| => SpanContextPropagatorFactory.GetPropagator(propagator) is not null | ||
| || string.Equals(propagator, "none", StringComparison.OrdinalIgnoreCase); | ||
|
|
||
| private static ParsingResult<OtlpProtocol> UnsupportedOtlpProtocol(string inputValue) | ||
| { | ||
| Log.Warning("Unsupported OTLP protocol '{Protocol}'. Supported values are 'grpc', 'http/protobuf' and 'http/json'. Using default: http/protobuf", inputValue); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.