Skip to content

[Tracer] Fix resource-based sampling for ASP.NET spans in IIS #6936

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.

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
merged 4 commits into from
May 23, 2025
Merged
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
36 changes: 23 additions & 13 deletions tracer/src/Datadog.Trace/AspNet/TracingHttpModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,22 @@ public void Dispose()
{
}

private static string BuildResourceName(Tracer tracer, HttpRequest httpRequest)
{
var url = tracer.Settings.BypassHttpRequestUrlCachingEnabled
? RequestDataHelper.BuildUrl(httpRequest)
: RequestDataHelper.GetUrl(httpRequest);
if (url is not null)
{
var path = UriHelpers.GetCleanUriPath(url, httpRequest.ApplicationPath);
return $"{httpRequest.HttpMethod.ToUpperInvariant()} {path.ToLowerInvariant()}";
}
else
{
return $"{httpRequest.HttpMethod.ToUpperInvariant()}";
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
return $"{httpRequest.HttpMethod.ToUpperInvariant()}";
return httpRequest.HttpMethod.ToUpperInvariant();

}
}

internal static void AddHeaderTagsFromHttpResponse(HttpContext httpContext, Scope scope)
{
if (!Tracer.Instance.Settings.HeaderTags.IsNullOrEmpty() &&
Expand Down Expand Up @@ -174,7 +190,12 @@ private void OnBeginRequest(object sender, EventArgs eventArgs)
var tags = new WebTags();
scope = tracer.StartActiveInternal(_requestOperationName, extractedContext.SpanContext, tags: tags);
// Leave resourceName blank for now - we'll update it in OnEndRequest
scope.Span.DecorateWebServerSpan(resourceName: null, httpMethod, host, url, userAgent, tags);

// Attempt to set Resource Name to something that will be close to what is expected
// Note: we will go and re-do it in OnEndRequest, but doing it here will allow for resource-based sampling
// this likely won't be perfect - but we need something to try and allow resource-based sampling to function
var resourceName = BuildResourceName(tracer, httpRequest);
Copy link
Member

Choose a reason for hiding this comment

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

This resource name won't include MVC route templates, right? Since we get those later when we instrument the MVC middleware. If the issue keep happening because the resource name doesn't match what the user's see in the UI, we may need to make a note that users should use what's in the http.url tag (or whatever it's called), instead of the resource name.

Copy link
Member

Choose a reason for hiding this comment

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

This resource name won't include MVC route templates, right

Yes, exactly, this is always going to be a problem 🙁

I think the proposed solution is a reasonable trade off though. We explicitly don't do this currently, to avoid extra allocation (as we're almost always going to create the resource name and then throw it away). But this is almost certainly preferable to the alternative.

Seems like another example where Doug's proposed "lazy evaluation of properties based on span facts" would be a net win, as we would only perform this calculation if we need it (e.g. if we need to make a sampling decision based on the resource name). But that's a long way off right now 😄

scope.Span.DecorateWebServerSpan(resourceName: resourceName, httpMethod, host, url, userAgent, tags);
tracer.TracerManager.SpanContextPropagator.AddHeadersToSpanAsTags(scope.Span, headers, tracer.Settings.HeaderTags, defaultTagPrefix: SpanContextPropagator.HttpRequestHeadersTagPrefix);

if (tracer.Settings.IpHeaderEnabled || Security.Instance.AppsecEnabled)
Expand Down Expand Up @@ -386,18 +407,7 @@ private void OnEndRequest(object sender, EventArgs eventArgs)
}
else
{
var url = tracer.Settings.BypassHttpRequestUrlCachingEnabled
? RequestDataHelper.BuildUrl(app.Request)
: RequestDataHelper.GetUrl(app.Request);
if (url is not null)
{
string path = UriHelpers.GetCleanUriPath(url, app.Request.ApplicationPath);
currentSpan.ResourceName = $"{app.Request.HttpMethod.ToUpperInvariant()} {path.ToLowerInvariant()}";
}
else
{
currentSpan.ResourceName = $"{app.Request.HttpMethod.ToUpperInvariant()}";
}
currentSpan.ResourceName = BuildResourceName(tracer, app.Request);
}
}
finally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System.Text;
using System.Threading.Tasks;
using Datadog.Trace.Configuration;
using Datadog.Trace.Sampling;
using Datadog.Trace.TestHelpers;
using FluentAssertions;
using VerifyXunit;
Expand Down Expand Up @@ -80,6 +81,11 @@ public AspNetMvc4Tests(IisFixture iisFixture, ITestOutputHelper output, bool cla
SetEnvironmentVariable(ConfigurationKeys.FeatureFlags.RouteTemplateResourceNamesEnabled, enableRouteTemplateResourceNames.ToString());
SetEnvironmentVariable(ConfigurationKeys.ExpandRouteTemplatesEnabled, enableRouteTemplateExpansion.ToString());

// these two are to test resource-based sampling on the parent ASP.NET span (non-MVC) one.
// it should not show up at all
SetEnvironmentVariable(ConfigurationKeys.CustomSamplingRules, """[{"sample_rate":0.0, "service":"*", "resource":"GET /home/pleasesamplemeout/?"}]""");
SetEnvironmentVariable(ConfigurationKeys.CustomSamplingRulesFormat, SamplingRulesFormat.Glob.ToString()); // for ease of use

_classicMode = classicMode;
_iisFixture = iisFixture;
_iisFixture.ShutdownPath = "/home/shutdown";
Expand Down Expand Up @@ -110,6 +116,7 @@ public AspNetMvc4Tests(IisFixture iisFixture, ITestOutputHelper output, bool cla
{ "/Home/BadRequestWithStatusCode?statuscode=401&TransferRequest=true", 401 },
{ "/Home/BadRequestWithStatusCode?statuscode=503&TransferRequest=true", 503 },
{ "/graphql/GetAllFoo", 200 }, // Slug in route template
{ "/Home/PleaseSampleMeOut/5555", 200 }, // test resource-based sampling for parent ASP.NET span (non-MVC) "GET /home/pleaseSampleMeOut/?" <- note the ? it isn't {id}
};

public override Result ValidateIntegrationSpan(MockSpan span, string metadataSchemaVersion) =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
[
{
TraceId: Id_1,
SpanId: Id_2,
Name: aspnet-mvc.request,
Resource: GET /home/pleasesamplemeout/?,
Service: sample,
Type: web,
ParentId: Id_3,
Tags: {
aspnet.action: pleasesamplemeout,
aspnet.controller: home,
aspnet.route: {controller}/{action}/{id},
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
span.kind: server,
version: 1.0.0
}
},
{
TraceId: Id_1,
SpanId: Id_3,
Name: aspnet.request,
Resource: GET /home/pleasesamplemeout/?,
Service: sample,
Type: web,
Tags: {
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.route: {controller}/{action}/{id},
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
runtime-id: Guid_1,
span.kind: server,
version: 1.0.0
},
Metrics: {
process_id: 0,
_dd.rule_psr: 0.0,
_dd.top_level: 1.0,
_dd.tracer_kr: 1.0,
_sampling_priority_v1: -1.0
}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
[
{
TraceId: Id_1,
SpanId: Id_2,
Name: aspnet-mvc.request,
Resource: GET /home/pleasesamplemeout/{id},
Service: sample,
Type: web,
ParentId: Id_3,
Tags: {
aspnet.action: pleasesamplemeout,
aspnet.controller: home,
aspnet.route: {controller}/{action}/{id},
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
span.kind: server,
version: 1.0.0
}
},
{
TraceId: Id_1,
SpanId: Id_3,
Name: aspnet.request,
Resource: GET /home/pleasesamplemeout/{id},
Service: sample,
Type: web,
Tags: {
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.route: {controller}/{action}/{id},
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
runtime-id: Guid_1,
span.kind: server,
version: 1.0.0
},
Metrics: {
process_id: 0,
_dd.top_level: 1.0,
_dd.tracer_kr: 1.0,
_sampling_priority_v1: 1.0
}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
[
{
TraceId: Id_1,
SpanId: Id_2,
Name: aspnet-mvc.request,
Resource: GET /home/pleasesamplemeout/?,
Service: sample,
Type: web,
ParentId: Id_3,
Tags: {
aspnet.action: pleasesamplemeout,
aspnet.controller: home,
aspnet.route: {controller}/{action}/{id},
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
span.kind: server,
version: 1.0.0
}
},
{
TraceId: Id_1,
SpanId: Id_3,
Name: aspnet.request,
Resource: GET /home/pleasesamplemeout/?,
Service: sample,
Type: web,
Tags: {
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.route: {controller}/{action}/{id},
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
runtime-id: Guid_1,
span.kind: server,
version: 1.0.0
},
Metrics: {
process_id: 0,
_dd.rule_psr: 0.0,
_dd.top_level: 1.0,
_dd.tracer_kr: 1.0,
_sampling_priority_v1: -1.0
}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
[
{
TraceId: Id_1,
SpanId: Id_2,
Name: aspnet-mvc.request,
Resource: GET /home/pleasesamplemeout/{id},
Service: sample,
Type: web,
ParentId: Id_3,
Tags: {
aspnet.action: pleasesamplemeout,
aspnet.controller: home,
aspnet.route: {controller}/{action}/{id},
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
span.kind: server,
version: 1.0.0
}
},
{
TraceId: Id_1,
SpanId: Id_3,
Name: aspnet.request,
Resource: GET /home/pleasesamplemeout/{id},
Service: sample,
Type: web,
Tags: {
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.route: {controller}/{action}/{id},
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
runtime-id: Guid_1,
span.kind: server,
version: 1.0.0
},
Metrics: {
process_id: 0,
_dd.rule_psr: 0.0,
_dd.top_level: 1.0,
_dd.tracer_kr: 1.0,
_sampling_priority_v1: -1.0
}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
[
{
TraceId: Id_1,
SpanId: Id_2,
Name: aspnet-mvc.request,
Resource: GET /home/pleasesamplemeout/{id},
Service: sample,
Type: web,
ParentId: Id_3,
Tags: {
aspnet.action: pleasesamplemeout,
aspnet.controller: home,
aspnet.route: {controller}/{action}/{id},
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
span.kind: server,
version: 1.0.0
}
},
{
TraceId: Id_1,
SpanId: Id_3,
Name: aspnet.request,
Resource: GET /home/pleasesamplemeout/{id},
Service: sample,
Type: web,
Tags: {
env: integration_tests,
http.method: GET,
http.request.headers.host: localhost:00000,
http.route: {controller}/{action}/{id},
http.status_code: 200,
http.url: http://localhost:00000/Home/PleaseSampleMeOut/5555,
http.useragent: testhelper,
language: dotnet,
runtime-id: Guid_1,
span.kind: server,
version: 1.0.0
},
Metrics: {
process_id: 0,
_dd.rule_psr: 0.0,
_dd.top_level: 1.0,
_dd.tracer_kr: 1.0,
_sampling_priority_v1: -1.0
}
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,14 @@ public ActionResult OptionalIdentifier(int? id)
ViewBag.Message = "Identifier set to " + id;
return View("About");
}

public ActionResult PleaseSampleMeOut(int? id)
{
// this should be sampled out - the resource name sampled is /home/pleaseSampleMeOut/?
// however the one that will be shown is /home/pleaseSampleMeOut/{id} after MVC routes are replaced
// but we make the sampling decision BEFORE the MVC routes are known
ViewBag.Message = "This should be sampled out based on the ASP.NET Request resource name " + id;
return View("About");
}
}
}
Loading