feat(otel): Emit OpenTelemetry HTTP semantic conventions on HTTP server spans (ASP.NET Core) - #8995
Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8995) and master. ✅ No regressions detected |
BenchmarksBenchmark execution time: 2026-09-01 18:59:26 Comparing candidate commit 9f89fbe in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 72 metrics, 0 unstable metrics, 67 known flaky benchmarks, 59 flaky benchmarks without significant changes.
|
f1664a2 to
30e3052
Compare
94908cc to
f13c1e3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f043b02dc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Snapshots difference summaryThe following differences have been observed in committed snapshots. It is meant to help the reviewer. 2 occurrences of : - http.request.method: GET,
|
22472cf to
3649dcd
Compare
5c28a20 to
9d37363
Compare
bouwkast
left a comment
There was a problem hiding this comment.
LGTM I just have a few comments on semantics mainly
Diff the new OtlpAspNetCoreMinimalApisTests.DatadogSemantics.* snapshots with the corresponding OtlpAspNetCoreMinimalApisTests.OtelSemantics file to see the difference in output when each feature flag is turned on. Notably, the OTLP output is NOT affected by the DD feature flags. At a high-level, the diff from Datadog semantics <> OTel semantics is the following. Modified (resource) names. Removed attributes: - component - http.request.headers.host - operation.name - resource.name - runtime-id - service.name - span.kind - span.type Renamed attributes: - http.method => http.request.method - http.url => url.scheme + server.address + server.port + url.path + url.query - http.status_code => http.response.status_code - http.useragent => user_agent.original Added attributes: - http.request.method_original - http.route
Diff with AspNetCoreMvc21Tests.cs To see how this OTLP test setup differs from the previous DD MsgPack tests
…etCore* test classes
…bled AspNetCore instrumentation only emits Datadog HTTP semantics (instead of both Datadog and OpenTelemetry)
…-Session-Token header for communicating with the dd apm test agent for each individual test case to prevent multiple concurrent XUnit sessions from colliding
…s-only, and its OpenTelemetry-semantics equivalent is HttpSemanticConventions.SetHttpServerRequestValues.
- Improve logic for getting the http.request.method and http.request.method_original attribute values - Update OpenTelemetrySdkTests to use dd-apm-test-agent sessions - Use file scoped namespaces (applied to OtlpTestAgentSession.cs) - Remove the XUnit Test Collection TestAgentOtlpCollection since now different classes can run concurrently because they submit OTLP traces by sessions (and tests under Datadog.Trace.ClrProfiler.IntegrationTests already have parallelization disabled by our CustomTestFramework)
…iving from OtlpAspNetCoreTestBase. This is accomplished by doing the following: - At the end of each test case execution, the OTLP traces for this test case are cleared, by calling Fixture.OtlpSession.ClearSessionAsync() in OtlpAspNetCoreTestBase.DisposeAsync. - At the start of each test case execution, invoke the test Fixture with a cached initialization result, similar to our common Fixture.TryStartApp, by calling ixture.EnsureInitializedAsync in OtlpAspNetCoreTestBase.InitializeAsync. Rather than use the pre-existing EnsureServerStarted, we pass our own delegate to cache. - Now we can remove the unnecessary polling so it's done at the start of each test class rather than each test case. - Re-insert the OTLP start time assertion in OtlpSnapshotHelper.NormalizeSpans now that traces are properly isolated per test case
- Force SingleSpanAspNetCoreEnabled to true when OpenTelemetry semantic mode is enabled - Force RouteTemplateResourceNamesEnabled to true when OpenTelemetry semantic mode is enabled - Update AspNetCoreHttpRequestHandler.IsKnownWebTag to only filter the OpenTelemetry attribute names when the span is using OpenTelemetry semantics, since that's the only time we're trying to deduplicate span attributes. - Remove unused regexes in VerifyHelper - Add explicit unit tests to cover the behavior of SpanExtensions.GetHttpMethod() to only search the tag for the span's semantic mode, rather than both DD and OTel keys
…osable and its consumer classes implement IAsyncLifetime, to make sure we start/clear sessions in a consistent manner
- Small changes to comments in AspNetCoreDiagnosticObserver and OtlpAspNetCoreTestBase to note that we're targeting the OTel spec first, not the OpenTelemetry ASP.NET Core instrumentation
- Optimize WebTags storage so we don't allocate additional fields for the default Datadog semantics
…P.NET Core: - Report http.route exactly as-is (not lowercased), or "/" if empty. This is handled by a new helper method HttpSemanticConventions.GetHttpRoute - Update our OtlpAspNetCoreTestBase with two more routes to execute some ASP.NET Core edge cases: "GET /" and "GET /rewrite-me"
…metry tags in KnownWebTags
- Fix WebTags unit tests regression by allowing the user to set OTel tags in the WebTags object, but still allocate the OtelTags object lazily - Update AspNetCore snapshots with route fix - Update Ocelot snapshot with updates to IsKnownWebTag implementation - Update OtlpTestAgentSession.WaitForSpansAsync and its invocation in OtlpAspNetCoreTestBase so we filter out any spans whose time stamp is older than the request start time
…spNetCoreTestBase with "GET /path-base/api/delay/0" With OTel semantics, the http.route is reported without the PathBase, so http.route reports the same as "GET /api/delay/0"
…/path-base/api/delay" call so we snapshot test it correctly
- Simplify the InitializeAsync / DisposeAsync for OpenTelemetryHttpClientTests and OpenTelemetryWebRequestTests after the OTLP agent session refactoring - Remove the need to expose internal types to the Datadog.Trace.ClrProfiler.IntegrationTests by making types public and using the actual Newtonsoft.Json package - Update TracerSettings when OTel semantics is enabled to only force enable SingleSpanAspNetCoreEnabled on .NET 6+ (where it's supported) and force disable ExpandRouteTemplatesEnabled
7c26fbb to
9f89fbe
Compare
Summary of changes
Updates the server spans produced by the ASP.NET Core automatic instrumentation so that their span name and request attributes match the OpenTelemetry HTTP server span specification when
DD_TRACE_OTEL_SEMANTICS_ENABLED=true, mirroring the HTTP client work from #8975. Also adds an OTLP-based ASP.NET Core test harness so the new attribute set can be snapshot-tested end-to-end, alongside the existing msgpack-based Datadog snapshot tests.Reason for change
When
DD_TRACE_OTEL_SEMANTICS_ENABLED=true, the SDK should produce spans that align with the OpenTelemetry semantic conventions across every span kind, not just HTTP clients. Today an ASP.NET Core server span emits the Datadog attribute set (http.method,http.url,http.request.headers.host) and a Datadog resource name built from various algorithms. After these changes, a user who has opted into OpenTelemetry semantics will get a server span whose name and attributes align with the OpenTelemetry HTTP server span specification. Ideally, users running the latest .NET OpenTelemetry ASP.NET Core instrumentation would be able to switch freely between the two without needing to update dashboards, monitors, or downstream tooling, but these changes will prefer adhering to the specification if any differences arise.Implementation details
WebTagschangesFollowing the same pattern established for
HttpTagsin #8975,WebTagsnow carries first-class properties for OpenTelemetry concepts, in addition to the existing Datadog ones. When a property is represented in both semantics, the corresponding tag name is emitted based onSpan.OpenTelemetrySemanticsEnabled:HttpMethodhttp.methodhttp.request.methodHttpUserAgenthttp.useragentuser_agent.originalHttpClientIphttp.client_ipclient.addressNetworkClientIpnetwork.client.ipnetwork.peer.addressHttpStatusCodehttp.status_codehttp.response.status_codeHttpRequestHeadersHosthttp.request.headers.hostHttpUrlhttp.urlHttpRequestMethodOriginalhttp.request.method_originalUrlSchemeurl.schemeUrlPathurl.pathUrlQueryurl.queryServerAddressserver.addressServerPortserver.portHttpUrl/HttpRequestHeadersHosthave no OpenTelemetry equivalent because the spec splits that single concept intourl.scheme/url.path/url.queryandserver.address/server.port.Conversely,
HttpRequestMethodOriginaland theurl.*/server.*properties are OpenTelemetry-only and are never populated under Datadog semantics.HttpStatusCodeandServerPortare backed by anint?and serialized as integer attributes over OTLP.Datadog-specific attributes (
aspnet_core.route,aspnet_core.endpoint,http.endpoint) are retained as we aim to be a superset of attributes, but we may consider removing them at a later time.HttpSemanticConventionsAdds
HttpSemanticConventions.SetHttpServerUrlTags(tags, scheme, host, port, pathBase, path, queryString, queryStringManager), the server-side counterpart to the client entry point from #8975. It builds theurl.*/server.*attributes directly from the individual ASP.NET Core request components and runs the query string through the sameQueryStringManagertruncation/obfuscation used elsewhere.GetRequestMethodOriginal(originalMethod, normalizedMethod)is extracted out of the client-only method-normalization logic added in #8975 so both the client and server code paths share it.AspNetCoreHttpRequestHandlerGetDefaultResourceNameuses the newotelSemanticsEnabledparameter to quickly return the{method}since nohttp.routewas identified.StartAspNetCorePipelineScopegenerates an ASP.NET Core span with correct naming/attributes and callsSetHttpServerUrlTagswhen OpenTelemetry semantics are enabled.RouteTemplateResourceNamesEnabledis now implicitly forced on byOtelSemanticsEnabled, because the spec requires the low-cardinality route inhttp.route, which Datadog only tracks when that feature is on.IsKnownWebTagis now extended to also recognize the new OpenTelemetry tag names, so the ASP.NET CoreActivity(which sets some of the same OpenTelemetry attributes independently) can't clobber the values this instrumentation already set. This allows us to continue using the backingWebTags, even though the attribute values should be identical.AspNetCoreDiagnosticObserver/SingleSpanAspNetCoreDiagnosticObserver{method} {http.route}using the route verbatim (lower-cased, unsimplified, reported exactly as-is or/if empty) instead of the Datadog "simplified route template" logic.StartMvcCoreSpanno longer creates theaspnet_core_mvc.requestchild span since OpenTelemetry requires a single span. Instead, the route/resource name is written onto the root span directly.SingleSpanAspNetCoreDiagnosticObserveralready did for its own single-span mode).TracerSettingsWhen
OtelSemanticsEnabledis true, three related feature flags are now force enabled/disabled so the tracing output aligns with the spec (and the final config value is recorded to telemetry asCalculated):RouteTemplateResourceNamesEnabled→ forcedtrue(spec requires the low-cardinality route).SingleSpanAspNetCoreEnabled→ forcedtrue, but only on .NET 6+, since the single-span observer isn't supported on older TFMs. On pre-.NET 6 runtimes,AspNetCoreDiagnosticObserver's existing OTel-semantics branches (e.g. skipping the MVC child span) remain the fallback.ExpandRouteTemplatesEnabled→ forcedfalse(spec requires the unexpanded route template, not one with parameter values substituted in).Span tag lookup refactor
To account for other internal call sites that request HTTP information via
span.GetTag, the following helpers were added:Tagging/IHasHttpMethod— implemented byHttpTags,WebTags,AwsSdkTags, andInferredProxyTagsto enable more optimized access to the HttpMethodSpanExtensions.GetHttpMethod(span)— reads the new strongly-typed property when available and otherwise falls back toGetTagSpanExtensions.GetHttpClientIp(span)andSpanExtensions.GetNetworkClientIp(span)— used by AppSec'sSecurityCoordinatorandSecurityReporterand byApiSecurity/IastModule/StatsAggregator.Test coverage
Unit tests
TagsListTests— New coverage asserting that eachWebTags1:1 rename resolves throughGetTag/SetTagunder either name, that exactly one wire name is enumerated per semantics mode, and that the OpenTelemetry-only attributes (url.*,server.*,http.request.method_original) are emitted under their own name and omitted under Datadog semantics.Integration tests — new OTLP test harness
VerifyHelper— Scrubs values reported under OpenTelemetry-specified span attributes so snapshots stay stable.OtlpTestAgentSession— Consolidates shared logic for getting OTLP from the ddapm test-agent, keyed by a uniqueX-Datadog-Test-Session-Tokenheader per test case so concurrent XUnit sessions don't collide. This also introducesClearSessionWhenQuietAsyncandCheckAvailabilityAsyncfor long-lived servers (IIS/Kestrel) that outlive a single test case and need their own health-check bypassed once OTLP export takes over from the mock DD agent, and filters out any spans whose timestamp predates the request start so leftover spans from a prior test don't leak into a snapshot.OtlpAspNetCoreTestBase— The OTLP parallel toAspNetCoreMvcTestBasewhich creates the AspNetCoreTestFixture and identifies shared test input routes.OtlpAspNetCoreMvc31Tests,OtlpAspNetCoreMinimalApisTests,OtlpAspNetCoreMvc21Tests— New end-to-end snapshot tests to verify OTLP export + OTel semantics for the test applicationsSamples.AspNetCoreMvc31,Samples.AspNetCoreMinimalApis, andSamples.AspNetCoreMvc21respectively.Understanding the snapshots
To understand how OTel semantics are reported based on the actual HTTP requests/responses, I recommend observing the diffs between the following snapshots:
http.routereported without the request'sPathBasehttp.request.method_originalis reportedAlso, I ran all of the following configurations so we could see the difference in all scenarios, but we can reduce the tested configurations as needed:
DD_TRACE_ROUTE_TEMPLATE_RESOURCE_NAMES_ENABLED=trueDD_TRACE_SINGLE_SPAN_ASPNETCORE_ENABLED=true(.NET 6+ only)DD_TRACE_OTEL_SEMANTICS_ENABLED=trueOther details
JIRA: APMAPI-2054
Follow-up work
network.protocol.name,network.protocol.version) are still unset on server spans.