Skip to content

Small perf tweaks for AspNetCoreDiagnosticObserver #7963

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
andrewlock merged 5 commits into from
Dec 24, 2025
Merged

Small perf tweaks for AspNetCoreDiagnosticObserver #7963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
68d9276
Use HttpContext.Items instead of HttpContext.Features (as slightly fa…
andrewlock Dec 17, 2025
d91bf77
Force providing the inputs to diagnostic observer (except where we cu…
andrewlock Dec 17, 2025
f122712
Inline checks for AddHeaderTagsToSpan
andrewlock Dec 17, 2025
ba93347
Should -> Enabled
andrewlock Dec 23, 2025
383da45
Inline extension method
andrewlock Dec 23, 2025
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
6 changes: 5 additions & 1 deletion tracer/src/Datadog.Trace/ClrProfiler/Instrumentation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,11 @@ private static void StartDiagnosticManager()
}
else
{
observers.Add(new AspNetCoreDiagnosticObserver());
// Tracer, Security, should both have been initialized by now.
// Iast hasn't yet, but doing it now is fine
// span origins is _not_ initialized yet, and we can't guarantee it will be
// so just be lazy instead
observers.Add(new AspNetCoreDiagnosticObserver(Tracer.Instance, Security.Instance, Iast.Iast.Instance, spanCodeOrigin: null));
Copy link
Copy Markdown
Collaborator

@NachoEchevarria NachoEchevarria Dec 23, 2025

Choose a reason for hiding this comment

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

This forces the creation of IAST and security instances even if not enabled. On the other hand, we are calling Security.Instance and Iast.Instance in many other places, so, in practical terms, we are going to create the instances anyway. Actually, we need to call Instance to read the IAST and ASM settings. Just wondering if at some point we could separate the security settings and access them without actually creating a (not needed mostly) instance of Security or IAST. We are creating instances just to read the settings. Still, a sensitive change (we need to be careful with RC, for instance)

Anyway, definitely, totally out of the scope of this PR. The performance optimization here is sound given the current architecture.

Copy link
Copy Markdown
Member Author

@andrewlock andrewlock Dec 24, 2025

Choose a reason for hiding this comment

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

This forces the creation of IAST and security instances even if not enabled

So these are actually already created automatically 😅 e.g. we explicitly do _ = Security.Instance; just above.

Actually, we need to call Instance to read the IAST and ASM settings. Just wondering if at some point we could separate the security settings and access them without actually creating a (not needed mostly) instance of Security or IAST.

💯 This is something I'd definitely like to do, and I think is worthwhile, but for the exact reason you state "we need to be careful with RC, for instance", I suspect we will generally need to do a certain amount of initialization no matter what, so I don't think it's a massive issue. And like you say, outside of the scope for this PR, but definitely something for us to think about! 🙂

observers.Add(new QuartzDiagnosticObserver());
}

Expand Down
85 changes: 33 additions & 52 deletions tracer/src/Datadog.Trace/DiagnosticListeners/AspNetCoreDiagnosticObserver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,6 @@ internal sealed class AspNetCoreDiagnosticObserver : DiagnosticObserver
private string _hostingHttpRequestInStopEventKey;
private string _routingEndpointMatchedKey;

public AspNetCoreDiagnosticObserver()
: this(null, null, null, null)
{
}

public AspNetCoreDiagnosticObserver(Tracer tracer, Security security, Iast.Iast iast, SpanCodeOrigin spanCodeOrigin)
{
_tracer = tracer;
Expand All @@ -85,12 +80,8 @@ public AspNetCoreDiagnosticObserver(Tracer tracer, Security security, Iast.Iast

protected override string ListenerName => DiagnosticListenerName;

private Tracer CurrentTracer => _tracer ?? Tracer.Instance;

private Security CurrentSecurity => _security ?? Security.Instance;

private Iast.Iast CurrentIast => _iast ?? Iast.Iast.Instance;

// TODO: Once SpanCodeOrigin initialization is synchronous
// just set this on startup instead of having the properties
private SpanCodeOrigin CurrentCodeOrigin => _spanCodeOrigin ?? DebuggerManager.Instance.CodeOrigin;

#if NETCOREAPP
Expand Down Expand Up @@ -421,50 +412,46 @@ private static Span StartMvcCoreSpan(

private void OnHostingHttpRequestInStart(object arg)
{
var tracer = CurrentTracer;
var security = CurrentSecurity;
var shouldTrace = tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId);
var shouldSecure = security.AppsecEnabled;
var integrationEnabled = _tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId);
var appsecEnabled = _security.AppsecEnabled;

if (!shouldTrace && !shouldSecure)
if (!integrationEnabled && !appsecEnabled)
{
return;
}

if (arg.TryDuckCast<HttpRequestInStartStruct>(out var requestStruct))
{
var httpContext = requestStruct.HttpContext;
if (shouldTrace)
if (integrationEnabled)
{
// Use an empty resource name here, as we will likely replace it as part of the request
// If we don't, update it in OnHostingHttpRequestInStop or OnHostingUnhandledException
// If the app is using resource-based sampling rules, then we need to set a resource straight
// away, so force that by using null.
var resourceName = tracer.CurrentTraceSettings.HasResourceBasedSamplingRule ? null : string.Empty;
var scope = AspNetCoreRequestHandler.StartAspNetCorePipelineScope(tracer, CurrentSecurity, CurrentIast, httpContext, resourceName);
if (shouldSecure)
var resourceName = _tracer.CurrentTraceSettings.HasResourceBasedSamplingRule ? null : string.Empty;
var scope = AspNetCoreRequestHandler.StartAspNetCorePipelineScope(_tracer, _security, _iast, httpContext, resourceName);
if (appsecEnabled)
{
CoreHttpContextStore.Instance.Set(httpContext);
var securityReporter = new SecurityReporter(scope.Span, new SecurityCoordinator.HttpTransport(httpContext));
securityReporter.ReportWafInitInfoOnce(security.WafInitResult);
securityReporter.ReportWafInitInfoOnce(_security.WafInitResult);
}
}
}
}

private void OnRoutingEndpointMatched(object arg)
{
var tracer = CurrentTracer;

if (!tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId) ||
!tracer.Settings.RouteTemplateResourceNamesEnabled)
if (!_tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId) ||
!_tracer.Settings.RouteTemplateResourceNamesEnabled)
{
return;
}

if (arg.TryDuckCast<HttpRequestInEndpointMatchedStruct>(out var typedArg)
&& typedArg.HttpContext is { } httpContext
&& httpContext.Features.Get<AspNetCoreHttpRequestHandler.RequestTrackingFeature>() is { RootScope.Span: { } rootSpan } trackingFeature)
&& httpContext.Items[AspNetCoreHttpRequestHandler.HttpContextItemsKey] is AspNetCoreHttpRequestHandler.RequestTrackingFeature { RootScope.Span: { } rootSpan } trackingFeature)
{
if (rootSpan.Tags is not AspNetCoreEndpointTags tags)
{
Expand Down Expand Up @@ -574,7 +561,7 @@ private void OnRoutingEndpointMatched(object arg)
areaName: areaName,
controllerName: controllerName,
actionName: actionName,
tracer.Settings.ExpandRouteTemplatesEnabled);
_tracer.Settings.ExpandRouteTemplatesEnabled);

var resourceName = $"{tags.HttpMethod} {request.PathBase.ToUriComponent()}{resourcePathName}";

Expand All @@ -591,9 +578,9 @@ private void OnRoutingEndpointMatched(object arg)
tags.HttpRoute = normalizedRoute;
}

CurrentSecurity.CheckPathParamsAndSessionId(httpContext, rootSpan, routeValues);
_security.CheckPathParamsAndSessionId(httpContext, rootSpan, routeValues);

if (CurrentIast.Settings.Enabled)
if (_iast.Settings.Enabled)
{
rootSpan.Context?.TraceContext?.IastRequestContext?.AddRequestData(httpContext.Request, routeValues);
}
Expand All @@ -602,37 +589,35 @@ private void OnRoutingEndpointMatched(object arg)

private void OnMvcBeforeAction(object arg)
{
var tracer = CurrentTracer;
var security = CurrentSecurity;
var shouldTrace = tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId);
var shouldSecure = security.AppsecEnabled;
var shouldUseIast = CurrentIast.Settings.Enabled;
var integrationEnabled = _tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId);
var appsecEnabled = _security.AppsecEnabled;
var iastEnabled = _iast.Settings.Enabled;
var isCodeOriginEnabled = CurrentCodeOrigin is { Settings.CodeOriginForSpansEnabled: true };

if (!shouldTrace && !shouldSecure && !shouldUseIast && !isCodeOriginEnabled)
if (!integrationEnabled && !appsecEnabled && !iastEnabled && !isCodeOriginEnabled)
{
return;
}

if (arg.TryDuckCast<BeforeActionStruct>(out var typedArg)
&& typedArg.HttpContext is { } httpContext
&& httpContext.Features.Get<AspNetCoreHttpRequestHandler.RequestTrackingFeature>() is { RootScope.Span: { } rootSpan } trackingFeature)
&& httpContext.Items[AspNetCoreHttpRequestHandler.HttpContextItemsKey] is AspNetCoreHttpRequestHandler.RequestTrackingFeature { RootScope.Span: { } rootSpan } trackingFeature)
{
HttpRequest request = httpContext.Request;

// NOTE: This event is the start of the action pipeline. The action has been selected, the route
// has been selected but no filters have run and model binding hasn't occurred.
Span span = null;
if (shouldTrace)
if (integrationEnabled)
{
if (!tracer.Settings.RouteTemplateResourceNamesEnabled)
if (!_tracer.Settings.RouteTemplateResourceNamesEnabled)
{
// override the parent's resource name with the simplified MVC route template
rootSpan.ResourceName = GetLegacyResourceName(typedArg);
}
else
{
span = StartMvcCoreSpan(tracer, trackingFeature, typedArg, httpContext, request);
span = StartMvcCoreSpan(_tracer, trackingFeature, typedArg, httpContext, request);
}
}

Expand All @@ -650,10 +635,10 @@ private void OnMvcBeforeAction(object arg)
}
}

CurrentSecurity.CheckPathParamsFromAction(httpContext, span, typedArg.ActionDescriptor?.Parameters, typedArg.RouteData.Values);
_security.CheckPathParamsFromAction(httpContext, span, typedArg.ActionDescriptor?.Parameters, typedArg.RouteData.Values);
}

if (shouldUseIast)
if (iastEnabled)
{
rootSpan.Context?.TraceContext?.IastRequestContext?.AddRequestData(request, typedArg.RouteData?.Values);
}
Expand Down Expand Up @@ -705,7 +690,7 @@ private bool TryGetTypeAndMethod(BeforeActionStruct beforeAction, out Type type,

private void OnMvcAfterAction(object arg)
{
var tracer = CurrentTracer;
var tracer = _tracer;

if (!tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId) ||
!tracer.Settings.RouteTemplateResourceNamesEnabled)
Expand Down Expand Up @@ -747,17 +732,15 @@ private void OnMvcAfterAction(object arg)

private void OnHostingHttpRequestInStop(object arg)
{
var tracer = CurrentTracer;

if (!tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId))
if (!_tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId))
{
return;
}

if (arg.DuckCast<HttpRequestInStopStruct>().HttpContext is { } httpContext
&& httpContext.Features.Get<AspNetCoreHttpRequestHandler.RequestTrackingFeature>() is { RootScope: { } rootScope })
&& httpContext.Items[AspNetCoreHttpRequestHandler.HttpContextItemsKey] is AspNetCoreHttpRequestHandler.RequestTrackingFeature { RootScope: { } rootScope })
{
AspNetCoreRequestHandler.StopAspNetCorePipelineScope(tracer, CurrentSecurity, rootScope, httpContext);
AspNetCoreRequestHandler.StopAspNetCorePipelineScope(_tracer, _security, rootScope, httpContext);
}

CoreHttpContextStore.Instance.Remove();
Expand All @@ -766,18 +749,16 @@ private void OnHostingHttpRequestInStop(object arg)

private void OnHostingUnhandledException(object arg)
{
var tracer = CurrentTracer;

if (!tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId))
if (!_tracer.CurrentTraceSettings.Settings.IsIntegrationEnabled(IntegrationId))
{
return;
}

if (arg.TryDuckCast<UnhandledExceptionStruct>(out var unhandledStruct)
&& unhandledStruct.HttpContext is { } httpContext
&& httpContext.Features.Get<AspNetCoreHttpRequestHandler.RequestTrackingFeature>() is { RootScope.Span: { } rootSpan })
&& httpContext.Items[AspNetCoreHttpRequestHandler.HttpContextItemsKey] is AspNetCoreHttpRequestHandler.RequestTrackingFeature { RootScope.Span: { } rootSpan })
{
AspNetCoreRequestHandler.HandleAspNetCoreException(tracer, CurrentSecurity, rootSpan, httpContext, unhandledStruct.Exception);
AspNetCoreRequestHandler.HandleAspNetCoreException(_tracer, _security, rootSpan, httpContext, unhandledStruct.Exception);
}

// If we don't have a span, no need to call Handle exception
Expand Down
47 changes: 25 additions & 22 deletions tracer/src/Datadog.Trace/PlatformHelpers/AspNetCoreHttpRequestHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#if !NETFRAMEWORK
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using Datadog.Trace.AppSec;
using Datadog.Trace.AppSec.Coordinator;
Expand All @@ -27,6 +28,7 @@ namespace Datadog.Trace.PlatformHelpers
{
internal sealed class AspNetCoreHttpRequestHandler
{
internal const string HttpContextItemsKey = "__Datadog.AspNetCoreHttpRequestHandler.Tracking";
private readonly IDatadogLogger _log;
private readonly IntegrationId _integrationId;
private readonly string _requestInOperationName;
Expand Down Expand Up @@ -73,29 +75,24 @@ private PropagationContext ExtractPropagatedContext(Tracer tracer, HttpRequest r
return default;
}

private void AddHeaderTagsToSpan(ISpan span, HttpRequest request, Tracer tracer)
private void AddHeaderTagsToSpan(ISpan span, HttpRequest request, Tracer tracer, ReadOnlyDictionary<string, string> headerTagsInternal)
{
var headerTagsInternal = tracer.CurrentTraceSettings.Settings.HeaderTags;

if (!headerTagsInternal.IsNullOrEmpty())
try
{
try
{
// extract propagation details from http headers
if (request.Headers is { } requestHeaders)
{
tracer.TracerManager.SpanContextPropagator.AddHeadersToSpanAsTags(
span,
new HeadersCollectionAdapter(requestHeaders),
headerTagsInternal,
defaultTagPrefix: SpanContextPropagator.HttpRequestHeadersTagPrefix);
}
}
catch (Exception ex)
// extract propagation details from http headers
if (request.Headers is { } requestHeaders)
{
_log.Error(ex, "Error extracting propagated HTTP headers.");
tracer.TracerManager.SpanContextPropagator.AddHeadersToSpanAsTags(
span,
new HeadersCollectionAdapter(requestHeaders),
headerTagsInternal,
defaultTagPrefix: SpanContextPropagator.HttpRequestHeadersTagPrefix);
}
}
catch (Exception ex)
{
_log.Error(ex, "Error extracting propagated HTTP headers.");
}
}

public Scope StartAspNetCorePipelineScope(Tracer tracer, Security security, Iast.Iast iast, HttpContext httpContext, string resourceName)
Expand Down Expand Up @@ -124,7 +121,13 @@ public Scope StartAspNetCorePipelineScope(Tracer tracer, Security security, Iast

var scope = tracer.StartActiveInternal(_requestInOperationName, extractedContext.SpanContext, tags: tags, links: extractedContext.Links);
scope.Span.DecorateWebServerSpan(resourceName, httpMethod, host, url, userAgent, tags);
AddHeaderTagsToSpan(scope.Span, request, tracer);

var headerTagsInternal = tracer.CurrentTraceSettings.Settings.HeaderTags;
if (headerTagsInternal.Count != 0)
{
AddHeaderTagsToSpan(scope.Span, request, tracer, headerTagsInternal);
}

tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(scope.Span, extractedContext.Baggage, tracer.Settings.BaggageTagKeys);

var originalPath = request.PathBase.HasValue ? request.PathBase.Add(request.Path) : request.Path;
Expand All @@ -135,7 +138,7 @@ public Scope StartAspNetCorePipelineScope(Tracer tracer, Security security, Iast
requestTrackingFeature.ProxyScope = proxyScope;
}

httpContext.Features.Set(requestTrackingFeature);
httpContext.Items[HttpContextItemsKey] = requestTrackingFeature;

if (tracer.Settings.IpHeaderEnabled || security.AppsecEnabled)
{
Expand All @@ -160,7 +163,7 @@ public void StopAspNetCorePipelineScope(Tracer tracer, Security security, Scope
{
if (rootScope != null)
{
var requestFeature = httpContext.Features.Get<RequestTrackingFeature>();
var requestFeature = httpContext.Items[HttpContextItemsKey] as RequestTrackingFeature;
var proxyScope = requestFeature?.ProxyScope;
// We may need to update the resource name if none of the routing/mvc events updated it.
// If we had an unhandled exception, the status code will already be updated correctly,
Expand Down Expand Up @@ -226,7 +229,7 @@ public void HandleAspNetCoreException(Tracer tracer, Security security, Span roo
// Generic unhandled exceptions are converted to 500 errors by Kestrel
rootSpan.SetHttpStatusCode(statusCode: statusCode, isServer: true, tracer.CurrentTraceSettings.Settings);

var requestFeature = httpContext.Features.Get<RequestTrackingFeature>();
var requestFeature = httpContext.Items[HttpContextItemsKey] as RequestTrackingFeature;
var proxyScope = requestFeature?.ProxyScope;
if (proxyScope?.Span != null)
{
Expand Down
Loading