Skip to content

Fix OTLP HTTP/protobuf export failures and improve OTLP integration test reliability #8449

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.

Sign up for GitHub

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

Merged
bouwkast merged 10 commits into from
Apr 21, 2026
Merged

Fix OTLP HTTP/protobuf export failures and improve OTLP integration test reliability #8449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
883d2b6
Remove forced HTTP/2 from OTLP exporter HttpClients
bouwkast Apr 15, 2026
193934c
Fix HttpClient leak in OtlpSubmissionLogSink
bouwkast Apr 15, 2026
8fcf313
Fix OTLP integration test reliability
bouwkast Apr 14, 2026
0a35130
Fix OTLP metric/log protocol settings to fallback correctly
bouwkast Apr 15, 2026
9129db4
Attempt to give more time for flushing logs
bouwkast Apr 17, 2026
e102afe
More attempts at fixing the tests
bouwkast Apr 20, 2026
cffedfa
Fix dupe metrics export
bouwkast Apr 20, 2026
6b4a632
More progress on the tests
bouwkast Apr 20, 2026
32d2ea4
Remove unused timeout parameter from OTLP Shutdown methods
bouwkast Apr 21, 2026
e80e6ec
Replace OTLP log ForceFlush with explicit Shutdown
bouwkast Apr 21, 2026
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
1 change: 0 additions & 1 deletion tracer/src/Datadog.Trace.Trimming/build/Datadog.Trace.Trimming.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@
<type fullname="System.Net.DnsEndPoint" />
<type fullname="System.Net.EndPoint" />
<type fullname="System.Net.HttpStatusCode" />
<type fullname="System.Net.HttpVersion" />
Copy link
Copy Markdown
Collaborator Author

@bouwkast bouwkast Apr 14, 2026

Choose a reason for hiding this comment

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

The removal of System.Net.HttpVersion appears to be due to the removal of DefaultRequestVersion = HttpVersion.Version20 and DefaultVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher

<type fullname="System.Net.ICredentials" />
<type fullname="System.Net.IPAddress" />
<type fullname="System.Net.IPEndPoint" />
Expand Down
11 changes: 9 additions & 2 deletions tracer/src/Datadog.Trace/Configuration/TracerSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,17 @@ not null when string.Equals(x, "http/protobuf", StringComparison.OrdinalIgnoreCa
},
validator: null);

var otlpGeneralProtocolName = OtlpGeneralProtocol switch
{
OtlpProtocol.Grpc => "grpc",
OtlpProtocol.HttpProtobuf => "http/protobuf",
_ => "grpc",
};
Comment on lines +253 to +258
Copy link
Copy Markdown
Collaborator Author

@bouwkast bouwkast Apr 21, 2026

Choose a reason for hiding this comment

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

Unsure if these changes are needed TBH will double check again

Copy link
Copy Markdown
Collaborator Author

@bouwkast bouwkast Apr 21, 2026

Choose a reason for hiding this comment

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

Ah yeah so the issue here was that in tests we only set SetEnvironmentVariable("OTEL_EXPORTER_OTLP_PROTOCOL", protocol); (protocol was http/protobuf)

and that we weren't setting the metric/logs specific protocol so that when we ran the tests instead of running with http/protobuf they actually ran grpc

Also the OpenTelemetry spec says (right at the top):

The following configuration options MUST be available to configure the OTLP exporter. Each configuration option MUST be overridable by a signal specific option.

https://opentelemetry.io/docs/specs/otel/protocol/exporter/#configuration-options

Copy link
Copy Markdown
Contributor

@zacharycmontoya zacharycmontoya Apr 21, 2026

Choose a reason for hiding this comment

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

Can confirm, this new logic is correct 👍🏼


OtlpMetricsProtocol = config
.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsProtocol)
.GetAs(
defaultValue: new(OtlpProtocol.Grpc, "grpc"),
defaultValue: new(OtlpGeneralProtocol, otlpGeneralProtocolName),
converter: x => x switch
{
not null when string.Equals(x, "grpc", StringComparison.OrdinalIgnoreCase) => OtlpProtocol.Grpc,
Expand Down Expand Up @@ -312,7 +319,7 @@ not null when string.Equals(x, "lowmemory", StringComparison.OrdinalIgnoreCase)
OtlpLogsProtocol = config
.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsProtocol)
.GetAs(
defaultValue: new(OtlpProtocol.Grpc, "grpc"),
defaultValue: new(OtlpGeneralProtocol, otlpGeneralProtocolName),
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 21, 2026

Choose a reason for hiding this comment

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

P2 Badge Propagate the general http/json protocol

For users who set only OTEL_EXPORTER_OTLP_PROTOCOL=http/json, this new fallback still resolves logs to the default gRPC protocol because OtlpGeneralProtocol does not parse http/json even though the logs-specific converter below accepts it and the configuration docs list it as valid. In that configuration logs will be sent to the gRPC default endpoint instead of the HTTP exporter unless OTEL_EXPORTER_OTLP_LOGS_PROTOCOL is also set explicitly.

Useful? React with 👍 / 👎.

converter: x => x switch
{
not null when string.Equals(x, "grpc", StringComparison.OrdinalIgnoreCase) => OtlpProtocol.Grpc,
Expand Down
8 changes: 7 additions & 1 deletion tracer/src/Datadog.Trace/Logging/DirectSubmission/Sink/OtlpSubmissionLogSink.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@ protected override void DelayEvents(TimeSpan delayUntilNextFlush)

public override async Task DisposeAsync()
{
await DisposeAsync(true).ConfigureAwait(false);
// Final flush is awaited by base.DisposeAsync() and bounded by the HTTP
// client timeout; Shutdown() just releases the HTTP client resources.
await base.DisposeAsync().ConfigureAwait(false);
if (!_otlpExporter.Shutdown())
{
_logger.Warning("OTLP exporter shutdown did not complete successfully.");
}
}
}
#endif
7 changes: 4 additions & 3 deletions tracer/src/Datadog.Trace/OpenTelemetry/Logs/IOtlpExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ internal interface IOtlpExporter
Task<ExportResult> ExportAsync(IReadOnlyList<LogPoint> logs);

/// <summary>
/// Shuts down the exporter and ensures all pending exports complete.
/// Releases the exporter's HTTP resources. Does not wait on pending exports:
/// the final flush runs synchronously in the sink's DisposeAsync before this is
/// called, bounded by the HTTP client timeout (OTEL_EXPORTER_OTLP_TIMEOUT).
/// </summary>
/// <param name="timeoutMilliseconds">Maximum time to wait for shutdown</param>
/// <returns>True if shutdown completed successfully</returns>
bool Shutdown(int timeoutMilliseconds);
bool Shutdown();
}
#endif
10 changes: 4 additions & 6 deletions tracer/src/Datadog.Trace/OpenTelemetry/Logs/OtlpExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,12 @@ public async Task<ExportResult> ExportAsync(IReadOnlyList<LogPoint> logs)
}

/// <summary>
/// Shuts down the exporter and ensures all pending exports complete.
/// Releases the exporter's HTTP resources. The final flush already ran
/// synchronously in the sink's DisposeAsync (bounded by the HTTP client
/// timeout, OTEL_EXPORTER_OTLP_TIMEOUT), so there is nothing further to wait on.
/// </summary>
/// <param name="timeoutMilliseconds">Maximum time to wait for shutdown</param>
/// <returns>True if shutdown completed successfully</returns>
public bool Shutdown(int timeoutMilliseconds)
public bool Shutdown()
{
try
{
Expand All @@ -158,14 +159,11 @@ private HttpClient CreateHttpClient()
var httpClient = new HttpClient(tcpHandler)
{
Timeout = TimeSpan.FromMilliseconds(_timeoutMs),
DefaultRequestVersion = HttpVersion.Version20,
DefaultVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher
};
#else
var httpClient = new HttpClient
{
Timeout = TimeSpan.FromMilliseconds(_timeoutMs),
DefaultRequestVersion = HttpVersion.Version20,
};
#endif

Expand Down
2 changes: 1 addition & 1 deletion tracer/src/Datadog.Trace/OpenTelemetry/Metrics/InMemoryExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public override Task<ExportResult> ExportAsync(IReadOnlyList<MetricPoint> metric
return Task.FromResult(result);
}

public override bool Shutdown(int timeoutMilliseconds)
public override bool Shutdown()
{
return true;
}
Expand Down
6 changes: 4 additions & 2 deletions tracer/src/Datadog.Trace/OpenTelemetry/Metrics/MetricExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ internal abstract class MetricExporter
public abstract Task<ExportResult> ExportAsync(IReadOnlyList<MetricPoint> metrics);

/// <summary>
/// Shuts down the exporter.
/// Releases exporter resources. The final flush is driven by the caller
/// (MetricReader.StopAsync) and bounded by the export path's own timeout;
/// this method is not expected to block.
/// </summary>
public abstract bool Shutdown(int timeoutMilliseconds);
public abstract bool Shutdown();
}
}
#endif
4 changes: 1 addition & 3 deletions tracer/src/Datadog.Trace/OpenTelemetry/Metrics/MetricReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ internal sealed class MetricReader
{
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(MetricReader));
private readonly int _exportIntervalMs;
private readonly int _timeoutMs;
private readonly MetricReaderHandler _handler;
private readonly MetricExporter _exporter;
private MeterListener? _listener;
Expand All @@ -33,7 +32,6 @@ internal sealed class MetricReader
public MetricReader(TracerSettings settings, MetricReaderHandler handler, MetricExporter exporter)
{
_exportIntervalMs = settings.OtelMetricExportIntervalMs;
_timeoutMs = settings.OtlpMetricsTimeoutMs;
_handler = handler;
_exporter = exporter;
}
Expand Down Expand Up @@ -95,7 +93,7 @@ public async Task StopAsync()
}
finally
{
_exporter.Shutdown(_timeoutMs);
_exporter.Shutdown();
_listener?.Dispose();
_listener = null;
Log.Debug("MetricReader stopped");
Expand Down
22 changes: 8 additions & 14 deletions tracer/src/Datadog.Trace/OpenTelemetry/Metrics/OtlpExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,13 @@ public override async Task<ExportResult> ExportAsync(IReadOnlyList<MetricPoint>
}

/// <summary>
/// Shuts down the exporter and ensures all pending exports complete.
/// Releases the exporter's HTTP resources. The final export already ran
/// synchronously in MetricReader.StopAsync and is bounded by the HTTP
/// request timeout (OTEL_EXPORTER_OTLP_METRICS_TIMEOUT), so there is
/// nothing further to wait on here.
/// </summary>
/// <param name="timeoutMilliseconds">Maximum time to wait for shutdown</param>
/// <returns>True if shutdown completed successfully, false otherwise</returns>
public override bool Shutdown(int timeoutMilliseconds)
public override bool Shutdown()
{
try
{
Expand All @@ -146,7 +148,7 @@ public override bool Shutdown(int timeoutMilliseconds)

/// <summary>
/// Creates an HttpClient with Unix Domain Socket support if the endpoint uses unix:// scheme.
/// For TCP/IP endpoints (http:// or https://), creates a standard HttpClient with HTTP/2.
/// For TCP/IP endpoints (http:// or https://), creates a standard HttpClient.
/// </summary>
private static HttpClient CreateHttpClient(Uri endpoint)
{
Expand All @@ -168,11 +170,7 @@ private static HttpClient CreateHttpClient(Uri endpoint)
}
};

return new HttpClient(handler)
{
DefaultRequestVersion = HttpVersion.Version20,
DefaultVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher
};
return new HttpClient(handler);
Copy link
Copy Markdown
Collaborator Author

@bouwkast bouwkast Apr 14, 2026

Choose a reason for hiding this comment

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

I couldn't find any documentation / reason why it had to be HTTP/2 so I think this is fine, but feel free to request it to be reverted.

Copy link
Copy Markdown
Member

@andrewlock andrewlock Apr 21, 2026

Choose a reason for hiding this comment

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

I think this could be because gRPC requires HTTP/2 🤔 But standard HTTP certainly shouldn't. So maybe we need to condition the client on that? (assuming this client is actually used in the grpc code path)

Copy link
Copy Markdown
Collaborator Author

@bouwkast bouwkast Apr 21, 2026

Choose a reason for hiding this comment

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

Yeah gRPC pins to 2 here I've assumed that takes affect as there aren't test failures there so I think that is good enough

}

// Standard TCP/IP endpoint
Expand All @@ -181,11 +179,7 @@ private static HttpClient CreateHttpClient(Uri endpoint)
AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate
};

return new HttpClient(tcpHandler)
{
DefaultRequestVersion = HttpVersion.Version20,
DefaultVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher
};
return new HttpClient(tcpHandler);
}

private async Task<bool> SendOtlpRequest(IReadOnlyList<MetricPoint> metrics)
Expand Down
Loading
Loading