Skip to content

Commit 7d328b2

Browse files
zacharycmontoyazacharycmontoya
authored
Implement OTel HTTP server semantics for AspNet integrations
1 parent 0e2fce6 commit 7d328b2

10 files changed

Lines changed: 562 additions & 80 deletions

‎tracer/src/Datadog.Trace/AppSec/ControllerContextExtensions.Framework.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ internal static void MonitorBodyAndPathParams(this IControllerContext controller
3232
return;
3333
}
3434

35-
var scope = SharedItems.TryPeekScope(context, peekScopeKey);
35+
var scope = SharedItems.TryPeekScopeOrServerScope(context, peekScopeKey);
3636
if (scope == null)
3737
{
3838
return;

‎tracer/src/Datadog.Trace/AspNet/SharedItems.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,26 @@ internal static void PushScope(HttpContext? context, string key, Scope item)
4747

4848
internal static Scope? TryPeekScope(HttpContext? context, string key) => ExtractScope(context, key, Peek);
4949

50+
/// <summary>
51+
/// Gets the scope an AppSec check should report against. With OpenTelemetry semantics the MVC
52+
/// and Web API integrations don't create a span of their own -- a request has a single HTTP
53+
/// server span -- so there is nothing under <paramref name="key"/> and the active span, which is
54+
/// that server span, is used instead.
55+
/// </summary>
56+
/// <param name="context">The context of the current request</param>
57+
/// <param name="key">The <see cref="HttpContext.Items"/> key the integration pushes its scope under</param>
58+
internal static Scope? TryPeekScopeOrServerScope(HttpContext? context, string key)
59+
{
60+
var scope = TryPeekScope(context, key);
61+
if (scope is not null)
62+
{
63+
return scope;
64+
}
65+
66+
var tracer = Tracer.Instance;
67+
return tracer.Settings.OtelSemanticsEnabled ? tracer.InternalActiveScope : null;
68+
}
69+
5070
private static Scope? ExtractScope(HttpContext? context, string key, Func<Stack<Scope>, Scope> getter)
5171
{
5272
var item = context?.Items[key];

‎tracer/src/Datadog.Trace/AspNet/TracingHttpModule.cs‎

Lines changed: 133 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
using Datadog.Trace.Headers;
1919
using Datadog.Trace.Iast;
2020
using Datadog.Trace.Logging;
21+
using Datadog.Trace.OpenTelemetry;
2122
using Datadog.Trace.Propagators;
2223
using Datadog.Trace.Sampling;
2324
using Datadog.Trace.Tagging;
@@ -82,8 +83,54 @@ public void Dispose()
8283
{
8384
}
8485

86+
/// <summary>
87+
/// Runs the AppSec and IAST request hooks against the span tracking the request. Shared by the
88+
/// usual path and by a transferred request, which reuses the span of the request it came from.
89+
/// </summary>
90+
private static void ReportToSecurityAndIast(Scope scope, HttpContext httpContext, HttpRequest httpRequest)
91+
{
92+
var security = Security.Instance;
93+
if (security.AppsecEnabled)
94+
{
95+
var securityCoordinator = SecurityCoordinator.Get(security, scope.Span, httpContext);
96+
securityCoordinator.Reporter.ReportWafInitInfoOnce(security.WafInitResult);
97+
98+
// request args
99+
var args = securityCoordinator.GetBasicRequestArgsForWaf();
100+
101+
// body args
102+
if (httpRequest.ContentType?.IndexOf("application/x-www-form-urlencoded", StringComparison.InvariantCultureIgnoreCase) >= 0)
103+
{
104+
var bodyArgs = securityCoordinator.GetBodyFromRequest();
105+
if (bodyArgs is not null)
106+
{
107+
args.Add(AddressesConstants.RequestBody, bodyArgs);
108+
}
109+
}
110+
111+
securityCoordinator.BlockAndReport(args, isInHttpTracingModule: true);
112+
}
113+
114+
var iastInstance = Iast.Iast.Instance;
115+
if (iastInstance.Settings.Enabled && iastInstance.OverheadController.AcquireRequest())
116+
{
117+
var traceContext = scope.Span?.Context?.TraceContext;
118+
traceContext?.EnableIastInRequest();
119+
traceContext?.IastRequestContext?.AddRequestData(httpRequest);
120+
}
121+
}
122+
85123
private static string BuildResourceName(Tracer tracer, HttpRequest httpRequest)
86124
{
125+
if (tracer.Settings.OtelSemanticsEnabled)
126+
{
127+
// The OpenTelemetry HTTP span specification requires the span name to be
128+
// "{method} {http.route}", or just "{method}" when no route is available. Falling back
129+
// to the URI path is explicitly not allowed, as it makes the name high-cardinality.
130+
// The route is added later by the MVC / Web API integrations, if the request matched one.
131+
return HttpSemanticConventions.GetServerResourceNameFromRawMethod(httpRequest.HttpMethod, route: null);
132+
}
133+
87134
var url = tracer.Settings.BypassHttpRequestUrlCachingEnabled
88135
? RequestDataHelper.BuildUrl(httpRequest)
89136
: RequestDataHelper.GetUrl(httpRequest);
@@ -180,21 +227,68 @@ private void OnBeginRequest(object sender, EventArgs eventArgs)
180227
}
181228
}
182229

183-
string host = requestHeaders.Get("Host");
230+
var otelSemanticsEnabled = tracer.Settings.OtelSemanticsEnabled;
231+
232+
// HttpServerUtility.TransferRequest re-runs the pipeline with a fresh HttpContext but
233+
// the same ExecutionContext, so the span the original request started is still active.
234+
// The OpenTelemetry conventions describe a single HTTP server span per inbound request,
235+
// so track the transferred request against that span rather than nesting a second
236+
// server span inside it. This pipeline still produces the response the client sees, and
237+
// its EndRequest runs first, so it is the one that stamps the status code onto the span.
238+
var reusedScope = otelSemanticsEnabled ? HttpSemanticConventions.GetActiveHttpServerScope(tracer) : null;
239+
240+
if (reusedScope is not null)
241+
{
242+
httpContext.Items[_httpContextScopeKey] = new ScopeContainer(reusedScope, proxyScope: null, ownsScope: false);
243+
shouldDisposeScope = false;
244+
ReportToSecurityAndIast(reusedScope, httpContext, httpRequest);
245+
return;
246+
}
247+
248+
var hostHeader = requestHeaders.Get("Host");
184249
var userAgent = requestHeaders.Get(HttpHeaderNames.UserAgent);
185-
string httpMethod = httpRequest.HttpMethod.ToUpperInvariant();
186-
var url = httpContext.Request.GetUrlForSpan(tracer.TracerManager.QueryStringManager, tracer.Settings.BypassHttpRequestUrlCachingEnabled);
187250
var tags = new WebTags();
188251
// FIXME: InstrumentationName should be added to InstrumentationTags
189252
tags.SetTag("component", "aspnet");
253+
254+
string host;
255+
string url;
256+
string httpMethod;
257+
258+
if (otelSemanticsEnabled)
259+
{
260+
// OpenTelemetry reports the Host header as server.address/server.port and the URL as
261+
// url.scheme/url.path/url.query, so neither the Host header nor the formatted absolute
262+
// URL is used as-is.
263+
host = null;
264+
url = null;
265+
266+
var requestUri = tracer.Settings.BypassHttpRequestUrlCachingEnabled
267+
? RequestDataHelper.BuildUrl(httpRequest)
268+
: RequestDataHelper.GetUrl(httpRequest);
269+
270+
httpMethod = HttpSemanticConventions.SetHttpServerRequestValues(
271+
tags,
272+
httpRequest.HttpMethod,
273+
requestUri,
274+
hostHeader,
275+
tracer.TracerManager.QueryStringManager);
276+
}
277+
else
278+
{
279+
host = hostHeader;
280+
url = httpContext.Request.GetUrlForSpan(tracer.TracerManager.QueryStringManager, tracer.Settings.BypassHttpRequestUrlCachingEnabled);
281+
httpMethod = httpRequest.HttpMethod.ToUpperInvariant();
282+
}
283+
190284
scope = tracer.StartActiveInternal(_requestOperationName, extractedContext.SpanContext, tags: tags);
191285
// Attempt to set Resource Name to something that will be close to what is expected
192286
// Note: we will go and re-do it in OnEndRequest, but doing it here will allow for resource-based sampling
193287
// this likely won't be perfect - but we need something to try and allow resource-based sampling to function
194288
var resourceName = tracer.CurrentTraceSettings.HasResourceBasedSamplingRule
195289
? BuildResourceName(tracer, httpRequest)
196290
: null;
197-
scope.Span.DecorateWebServerSpan(resourceName: resourceName, httpMethod, host, url, userAgent, tags);
291+
scope.Span.DecorateWebServerSpan(resourceName: resourceName, httpMethod, host, url, userAgent, tags, otelSemanticsEnabled);
198292
tracer.TracerManager.SpanContextPropagator.AddHeadersToSpanAsTags(scope.Span, headers, tracer.CurrentTraceSettings.Settings.HeaderTags, defaultTagPrefix: SpanContextPropagator.HttpRequestHeadersTagPrefix);
199293
tracer.TracerManager.SpanContextPropagator.AddSecurityTestingHeadersAsTags(scope.Span, headers);
200294
if (inferredProxyScope?.Span is { } proxySpan)
@@ -237,35 +331,7 @@ private void OnBeginRequest(object sender, EventArgs eventArgs)
237331

238332
tracer.TracerManager.Telemetry.IntegrationGeneratedSpan(IntegrationId);
239333

240-
var security = Security.Instance;
241-
if (security.AppsecEnabled)
242-
{
243-
var securityCoordinator = SecurityCoordinator.Get(security, scope.Span, httpContext);
244-
securityCoordinator.Reporter.ReportWafInitInfoOnce(security.WafInitResult);
245-
246-
// request args
247-
var args = securityCoordinator.GetBasicRequestArgsForWaf();
248-
249-
// body args
250-
if (httpRequest.ContentType?.IndexOf("application/x-www-form-urlencoded", StringComparison.InvariantCultureIgnoreCase) >= 0)
251-
{
252-
var bodyArgs = securityCoordinator.GetBodyFromRequest();
253-
if (bodyArgs is not null)
254-
{
255-
args.Add(AddressesConstants.RequestBody, bodyArgs);
256-
}
257-
}
258-
259-
securityCoordinator.BlockAndReport(args, isInHttpTracingModule: true);
260-
}
261-
262-
var iastInstance = Iast.Iast.Instance;
263-
if (iastInstance.Settings.Enabled && iastInstance.OverheadController.AcquireRequest())
264-
{
265-
var traceContext = scope.Span?.Context?.TraceContext;
266-
traceContext?.EnableIastInRequest();
267-
traceContext?.IastRequestContext?.AddRequestData(httpRequest);
268-
}
334+
ReportToSecurityAndIast(scope, httpContext, httpRequest);
269335
}
270336
catch (Exception ex)
271337
{
@@ -427,31 +493,40 @@ private void OnEndRequest(object sender, EventArgs eventArgs)
427493
AddHeaderTagsFromHttpResponse(app.Context, proxyScope);
428494
}
429495

430-
if (app.Context.Items[SharedItems.HttpContextPropagatedResourceNameKey] is string resourceName
431-
&& !string.IsNullOrEmpty(resourceName))
496+
// A transferred request must not rename the span of the request that
497+
// transferred to it: the name describes the request the client made.
498+
if (container.OwnsScope)
432499
{
433-
currentSpan.ResourceName = resourceName;
434-
}
435-
else
436-
{
437-
currentSpan.ResourceName = BuildResourceName(tracer, app.Request);
500+
if (app.Context.Items[SharedItems.HttpContextPropagatedResourceNameKey] is string resourceName
501+
&& !string.IsNullOrEmpty(resourceName))
502+
{
503+
currentSpan.ResourceName = resourceName;
504+
}
505+
else
506+
{
507+
currentSpan.ResourceName = BuildResourceName(tracer, app.Request);
508+
}
438509
}
439510
}
440511
finally
441512
{
442-
try
513+
if (container.OwnsScope)
443514
{
444-
if (scope.Span.ResourceName is null)
515+
try
445516
{
446-
scope.Span.ResourceName = BuildResourceName(tracer, app.Request);
517+
if (scope.Span.ResourceName is null)
518+
{
519+
scope.Span.ResourceName = BuildResourceName(tracer, app.Request);
520+
}
447521
}
448-
}
449-
catch (Exception ex)
450-
{
451-
Log.Debug(ex, "Unable to set fallback resource name.");
522+
catch (Exception ex)
523+
{
524+
Log.Debug(ex, "Unable to set fallback resource name.");
525+
}
526+
527+
scope.Dispose();
452528
}
453529

454-
scope.Dispose();
455530
proxyScope?.Dispose();
456531
// Clear the context to make sure another TracingHttpModule doesn't try to close the same scope
457532
TryClearContext(app.Context);
@@ -536,14 +611,23 @@ private void TryClearContext(HttpContext context)
536611
/// </summary>
537612
internal sealed class ScopeContainer
538613
{
539-
public ScopeContainer(Scope scope, Scope proxyScope = null)
614+
public ScopeContainer(Scope scope, Scope proxyScope = null, bool ownsScope = true)
540615
{
541616
Scope = scope;
542617
ProxyScope = proxyScope;
618+
OwnsScope = ownsScope;
543619
}
544620

545621
public Scope Scope { get; }
546622

623+
/// <summary>
624+
/// Gets a value indicating whether this request started the scope, and is therefore the
625+
/// one that names and finishes it. <c>false</c> for a request produced by
626+
/// <see cref="HttpServerUtility.TransferRequest(string)"/> under OpenTelemetry semantics,
627+
/// where the span belongs to the request that transferred to this one.
628+
/// </summary>
629+
public bool OwnsScope { get; }
630+
547631
/// <summary>
548632
/// Gets the inferred proxy scope. Only present when inferred proxy spans are enabled
549633
/// AND necessary proxy headers were present.

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/ApiController_ExecuteAsync_Integration.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ internal static CallTargetState OnMethodBegin<TTarget, TController>(TTarget inst
5858
{
5959
// Make sure to box the controllerContext proxy only once
6060
var boxedControllerContext = (IHttpControllerContext)controllerContext;
61+
var tracer = Tracer.Instance;
62+
63+
if (AspNetWebApi2Integration.UsesExistingServerSpan(tracer))
64+
{
65+
// With OpenTelemetry semantics a request has a single HTTP server span, so enrich the
66+
// ASP.NET one instead of nesting an aspnet-webapi.request span inside it. The controller
67+
// context is carried through so the route can be refreshed once the action has run.
68+
AspNetWebApi2Integration.UpdateExistingServerSpan(tracer, boxedControllerContext);
69+
return new CallTargetState(scope: null, state: boxedControllerContext);
70+
}
6171

6272
var scope = AspNetWebApi2Integration.CreateScope(boxedControllerContext, out _);
6373

@@ -90,6 +100,13 @@ internal static TResponse OnAsyncMethodEnd<TTarget, TResponse>(TTarget instance,
90100

91101
if (scope is null)
92102
{
103+
if (state.State is IHttpControllerContext existingSpanContext)
104+
{
105+
// No span of our own: the route information belongs on the ASP.NET server span, and
106+
// this is the first point at which the executed route is guaranteed to be resolved.
107+
AspNetWebApi2Integration.UpdateExistingServerSpan(Tracer.Instance, existingSpanContext);
108+
}
109+
93110
return responseMessage;
94111
}
95112

0 commit comments

Comments
 (0)