Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,20 @@ public void LogUnsupportedConfiguration(string otelKey, string otelValue, string
});
}

public void LogUnsupportedValues(string otelKey, string[] unsupportedValues)
{
EnqueueAction(
(log, metrics) =>
{
log.Warning(
"OpenTelemetry configuration {OpenTelemetryConfiguration} contains unsupported values '{UnsupportedValues}', which will be ignored.",
otelKey,
string.Join(",", unsupportedValues));
OpenTelemetryHelpers.GetConfigurationMetricTags(otelKey, out var openTelemetryConfig, out var datadogConfig);
metrics.RecordCountOpenTelemetryConfigInvalid(datadogConfig, openTelemetryConfig);
});
}

bool IConfigurationOverrideHandler.TryHandleOverrides<T>(
string datadogKey,
ConfigurationResult<T> datadogConfigResult,
Expand Down
31 changes: 30 additions & 1 deletion tracer/src/Datadog.Trace/Configuration/TracerSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Comment thread
MilanGarnier marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Preserve invalid propagators through composite source resolution

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
  • 👍 Useful: correct, and you're fixing it or will.
  • 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
  • 😕 Unclear: you can't tell what it means or why it matters.
  • 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

🤖 Bits Code Review · Open Bits AI session

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 })
Comment thread
MilanGarnier marked this conversation as resolved.
{
var propagatorsBySupport = otelPropagators.ToLookup(IsSupportedOtelPropagator);
Comment thread
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(
Expand Down Expand Up @@ -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);
Expand Down
106 changes: 90 additions & 16 deletions tracer/test/Datadog.Trace.Tests/Configuration/TracerSettingsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@
using Datadog.Trace.Configuration;
using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
using Datadog.Trace.Configuration.Telemetry;
using Datadog.Trace.Logging;
using Datadog.Trace.Sampling;
using Datadog.Trace.Tagging;
using Datadog.Trace.Telemetry;
using Datadog.Trace.Telemetry.Metrics;
using Datadog.Trace.TestHelpers;
using FluentAssertions;
using FluentAssertions.Execution;
using Moq;
using Xunit;

namespace Datadog.Trace.Tests.Configuration
Expand Down Expand Up @@ -650,6 +652,12 @@ public void IsActivityListenerEnabled(string value, string fallbackValue, string
[InlineData(null, null, null, "tracecontext,b3,b3multi", new[] { "tracecontext", "b3 single header", "b3multi" })]
[InlineData(null, null, null, null, new[] { "Datadog", "tracecontext", "baggage" })]
[InlineData(null, null, null, ",", new[] { "Datadog", "tracecontext", "baggage" })]
[InlineData(null, null, null, "jaeger", new[] { "Datadog", "tracecontext", "baggage" })]
[InlineData(null, null, null, "tracecontext,baggage,jaeger,xray", new[] { "tracecontext", "baggage" })]
[InlineData(null, null, null, "b3,ottrace", new[] { "b3 single header" })]
[InlineData(null, null, null, "TRACEContext,B3MULTI", new[] { "TRACEContext", "B3MULTI" })]
[InlineData(null, null, null, "none", new[] { "none" })]
[InlineData(null, null, "tracecontext", "tracecontext,jaeger", new[] { "tracecontext" })]
public void PropagationStyleInject(string value, string legacyValue, string fallbackValue, string otelValue, string[] expected)
{
const string legacyKey = "DD_PROPAGATION_STYLE_INJECT";
Expand All @@ -669,14 +677,7 @@ public void PropagationStyleInject(string value, string legacyValue, string fall

settings.PropagationStyleInject.Should().BeEquivalentTo(expected);

Count? metric = (value ?? legacyValue ?? fallbackValue, otelValue) switch
{
(null, ",") => Count.OpenTelemetryConfigInvalid,
(not null, not null) => Count.OpenTelemetryConfigHiddenByDatadogConfig,
_ => null,
};

errorLog.ShouldHaveExpectedOtelMetric(metric, ConfigurationKeys.OpenTelemetry.Propagators.ToLowerInvariant(), "DD_TRACE_PROPAGATION_STYLE".ToLowerInvariant());
errorLog.ShouldHaveExpectedOtelMetric(GetExpectedOtelPropagationMetrics(value ?? legacyValue ?? fallbackValue, otelValue));
}
}

Expand All @@ -690,6 +691,12 @@ public void PropagationStyleInject(string value, string legacyValue, string fall
[InlineData(null, null, null, "tracecontext,b3,b3multi", new[] { "tracecontext", "b3 single header", "b3multi" })]
[InlineData(null, null, null, null, new[] { "Datadog", "tracecontext", "baggage" })]
[InlineData(null, null, null, ",", new[] { "Datadog", "tracecontext", "baggage" })]
[InlineData(null, null, null, "jaeger", new[] { "Datadog", "tracecontext", "baggage" })]
[InlineData(null, null, null, "tracecontext,baggage,jaeger,xray", new[] { "tracecontext", "baggage" })]
[InlineData(null, null, null, "b3,ottrace", new[] { "b3 single header" })]
[InlineData(null, null, null, "TRACEContext,B3MULTI", new[] { "TRACEContext", "B3MULTI" })]
[InlineData(null, null, null, "none", new[] { "none" })]
[InlineData(null, null, "tracecontext", "tracecontext,jaeger", new[] { "tracecontext" })]
public void PropagationStyleExtract(string value, string legacyValue, string fallbackValue, string otelValue, string[] expected)
{
const string legacyKey = "DD_PROPAGATION_STYLE_EXTRACT";
Expand All @@ -709,17 +716,43 @@ public void PropagationStyleExtract(string value, string legacyValue, string fal

settings.PropagationStyleExtract.Should().BeEquivalentTo(expected);

Count? metric = (value ?? legacyValue ?? fallbackValue, otelValue) switch
{
(null, ",") => Count.OpenTelemetryConfigInvalid,
(not null, not null) => Count.OpenTelemetryConfigHiddenByDatadogConfig,
_ => null,
};

errorLog.ShouldHaveExpectedOtelMetric(metric, ConfigurationKeys.OpenTelemetry.Propagators.ToLowerInvariant(), "DD_TRACE_PROPAGATION_STYLE".ToLowerInvariant());
errorLog.ShouldHaveExpectedOtelMetric(GetExpectedOtelPropagationMetrics(value ?? legacyValue ?? fallbackValue, otelValue));
}
}

[Theory]
// Values with no supported propagator are reported as invalid instead
[InlineData(null, "not-a-propagator", null)]
[InlineData(null, "jaeger,ottrace", null)]
[InlineData(null, "tracecontext,baggage,Jaeger,b3,XRay", "Jaeger,XRay")]
[InlineData(null, "tracecontext,none", null)]
[InlineData(null, "tracecontext,baggage", null)]
[InlineData("tracecontext", "tracecontext,jaeger", "jaeger")]
public void PropagationStyleUnsupportedOtelPropagatorsWarning(string fallbackValue, string otelValue, string expectedWarningValue)
{
var source = CreateConfigurationSource(
("DD_TRACE_PROPAGATION_STYLE", fallbackValue),
(ConfigurationKeys.OpenTelemetry.Propagators, otelValue));

ShouldWarnAboutUnsupportedOtelPropagators(source, expectedWarningValue);
}

[Fact]
public void PropagationStyleUnsupportedOtelPropagatorsWarningUsesSelectedSource()
{
// The highest-priority source has no supported propagator, so the lower-priority one is used
var source = new CompositeConfigurationSource
{
new NameValueConfigurationSource(new() { { ConfigurationKeys.OpenTelemetry.Propagators, "jaeger" } }),
new NameValueConfigurationSource(new() { { ConfigurationKeys.OpenTelemetry.Propagators, "tracecontext,xray" } }),
};

var settings = ShouldWarnAboutUnsupportedOtelPropagators(source, "xray");

settings.PropagationStyleInject.Should().BeEquivalentTo("tracecontext");
settings.PropagationStyleExtract.Should().BeEquivalentTo("tracecontext");
}

[Theory]
[MemberData(nameof(StringTestCases), "", Strings.AllowEmpty)]
public void TraceMethods(string value, string expected)
Expand Down Expand Up @@ -1366,5 +1399,46 @@ public void ExperimentalFeaturesEnabled_ParsesAndTrimsEntries(string value, stri

settings.ExperimentalFeaturesEnabled.Should().BeEquivalentTo(expected);
}

private static IEnumerable<(Count? Metric, string OtelSetting, string DdSetting)> GetExpectedOtelPropagationMetrics(string ddValue, string otelValue)
{
var otelSetting = ConfigurationKeys.OpenTelemetry.Propagators.ToLowerInvariant();
var ddSetting = "DD_TRACE_PROPAGATION_STYLE".ToLowerInvariant();

if (ddValue is not null && otelValue is not null)
{
yield return (Count.OpenTelemetryConfigHiddenByDatadogConfig, otelSetting, ddSetting);
}
else if (otelValue is "," or "jaeger")
{
yield return (Count.OpenTelemetryConfigInvalid, otelSetting, ddSetting);
}

// Values mixing supported and unsupported propagators
if (otelValue is "tracecontext,baggage,jaeger,xray" or "b3,ottrace" or "tracecontext,jaeger")
{
yield return (Count.OpenTelemetryConfigInvalid, otelSetting, ddSetting);
}
}

private static TracerSettings ShouldWarnAboutUnsupportedOtelPropagators(IConfigurationSource source, string expectedWarningValue)
{
var errorLog = new OverrideErrorLog();
var settings = new TracerSettings(source, NullConfigurationTelemetry.Instance, errorLog);

var log = new Mock<IDatadogLogger>();
errorLog.ProcessAndClearActions(log.Object, new MetricsTelemetryCollector());

log.Verify(
l => l.Warning(
It.Is<string>(s => s.Contains("unsupported values")),
ConfigurationKeys.OpenTelemetry.Propagators,
It.Is<string>(v => expectedWarningValue == null || v == expectedWarningValue),
It.IsAny<int>(),
It.IsAny<string>()),
expectedWarningValue is null ? Times.Never() : Times.Once());

return settings;
}
}
}
Loading