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

Conversation

bouwkast
Copy link
Collaborator

@bouwkast bouwkast commented May 7, 2025

Summary of changes

Set's a preliminary Span.ResourceName for ASP.NET spans in OnBeginRequest as well as in OnEndRequest to ensure that resource-based sampling will work.

Reason for change

Within TracingHttpModule we do not set a ResourceName for the span in OnBeginRequest, this is to try and ensure that the resource names from the parent ASP.NET span and it's child span will be equivalent. We instead set it in the OnEndRequest.

This poses an issue though in IIS as we then (commonly) call Inject which ends up making a sampling decision without a Span.ResourceName. What this ends up causing is that resource-based sampling decisions / rules will never be applied.
Once a sampling decision has been made it is not changed at a later point.

The following code path gets hit for ASP.NET in IIS which causes the behavior.

// Decorate the incoming HTTP Request with distributed tracing headers
// in case the next processor cannot access the stored Scope
// (e.g. WCF being hosted in IIS)
if (HttpRuntime.UsingIntegratedPipeline)
{
    var injectedContext = new PropagationContext(scope.Span.Context, Baggage.Current);
    tracer.TracerManager.SpanContextPropagator.Inject(injectedContext, requestHeaders.Wrap());
}

Implementation details

I've extracted a part of the logic to build the resource name in the OnEndRequest and am using it in the OnBeginRequest phase. This will allow us to have some semblance of a relatively accurate resource name at the time that we are making the sampling decision.

The part that is different is that we do not in the OnBeginRequest have access to the child span resource name, which is the path ->

if (app.Context.Items[SharedItems.HttpContextPropagatedResourceNameKey] is string resourceName
 && !string.IsNullOrEmpty(resourceName))
{
    currentSpan.ResourceName = resourceName;
}

So while I think that the resource name computed now will be relatively accurate there may be instances where the child HTTP span from it has a different resource name that overrides it.

Test coverage

Updated MVC4 ASP net tests to have a resource that should be sampled out (see the snapshot sampling value) based on its aspnet span (not the MVC) span resource name.

Other details

It may be beneficial to tag the original resource name as it may look a bit different to help determine what resource was used in the sampling decision (if any), but we can come back to that later.

@github-actions github-actions bot added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label May 7, 2025
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented May 7, 2025

Datadog Report

All test runs 0c230b8 🔗

2 Total Test Services: 0 Failed, 2 Passed
❄️ 1 with New Flaky

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Test Service View
dd-trace-dotnet 0 0 1 247488 2257 18h 18m 41.81s Link
exploration_tests 0 0 0 22085 3 2m 12.47s Link

@andrewlock
Copy link
Member

Benchmarks Report for tracer 🐌

Benchmarks for #6936 compared to master:

  • 4 benchmarks are faster, with geometric mean 1.422
  • 1 benchmarks are slower, with geometric mean 1.179
  • All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 7.95μs 141ns 1.41μs 0 0 0 5.55 KB
master StartStopWithChild netcoreapp3.1 8.98μs 32.1ns 116ns 0 0 0 5.74 KB
master StartStopWithChild net472 15.1μs 78.4ns 400ns 1.08 0.432 0.144 6.11 KB
#6936 StartStopWithChild net6.0 7.66μs 109ns 1.09μs 0 0 0 5.57 KB
#6936 StartStopWithChild netcoreapp3.1 10.3μs 148ns 1.48μs 0 0 0 5.75 KB
#6936 StartStopWithChild net472 15.5μs 85.3ns 526ns 0.998 0.285 0.0713 6.11 KB
Benchmarks.Trace.AgentWriterBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6936

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 1.971 1,049,792.19 532,753.65

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 1.04ms 4.78μs 18.5μs 0 0 0 2.71 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 658μs 561ns 2.17μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 858μs 650ns 2.52μs 0 0 0 3.31 KB
#6936 WriteAndFlushEnrichedTraces net6.0 533μs 1.16μs 4.33μs 0 0 0 2.7 KB
#6936 WriteAndFlushEnrichedTraces netcoreapp3.1 659μs 1.9μs 7.09μs 0 0 0 2.7 KB
#6936 WriteAndFlushEnrichedTraces net472 859μs 542ns 1.95μs 0 0 0 3.31 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 135μs 784ns 6.74μs 0 0 0 14.48 KB
master SendRequest netcoreapp3.1 150μs 720ns 2.79μs 0 0 0 17.28 KB
master SendRequest net472 0.00117ns 0.000629ns 0.00244ns 0 0 0 0 b
#6936 SendRequest net6.0 139μs 790ns 5.7μs 0 0 0 14.48 KB
#6936 SendRequest netcoreapp3.1 150μs 857ns 6.24μs 0 0 0 17.28 KB
#6936 SendRequest net472 0.0025ns 0.000712ns 0.00276ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 548μs 2.52μs 9.75μs 0 0 0 41.57 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 648μs 2.11μs 7.59μs 0 0 0 41.77 KB
master WriteAndFlushEnrichedTraces net472 879μs 6.29μs 62.9μs 7.81 0 0 53.44 KB
#6936 WriteAndFlushEnrichedTraces net6.0 565μs 3.12μs 19.3μs 0 0 0 41.65 KB
#6936 WriteAndFlushEnrichedTraces netcoreapp3.1 667μs 3.82μs 27.8μs 0 0 0 41.8 KB
#6936 WriteAndFlushEnrichedTraces net472 896μs 5.23μs 48.2μs 8.33 4.17 0 53.39 KB
Benchmarks.Trace.DbCommandBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6936

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑netcoreapp3.1 1.624 2,907.66 1,790.51

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.41μs 7.58ns 46.1ns 0.00923 0 0 1.03 KB
master ExecuteNonQuery netcoreapp3.1 2.91μs 3.89ns 13.5ns 0 0 0 1.02 KB
master ExecuteNonQuery net472 2.1μs 3.9ns 15.1ns 0.148 0.0106 0 995 B
#6936 ExecuteNonQuery net6.0 1.33μs 2.48ns 9.62ns 0.0133 0 0 1.03 KB
#6936 ExecuteNonQuery netcoreapp3.1 1.79μs 2.7ns 10.1ns 0.009 0 0 1.02 KB
#6936 ExecuteNonQuery net472 2.08μs 4.14ns 16ns 0.155 0.0103 0 995 B
Benchmarks.Trace.ElasticsearchBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6936

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync‑net6.0 1.120 1,387.37 1,239.05

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.23μs 0.849ns 3.29ns 0.0122 0 0 984 B
master CallElasticsearch netcoreapp3.1 1.56μs 0.675ns 2.43ns 0.00793 0 0 984 B
master CallElasticsearch net472 2.41μs 1.16ns 4.48ns 0.159 0 0 1 KB
master CallElasticsearchAsync net6.0 1.39μs 0.847ns 3.28ns 0.0137 0 0 960 B
master CallElasticsearchAsync netcoreapp3.1 1.61μs 2.3ns 8.61ns 0.00806 0 0 1.03 KB
master CallElasticsearchAsync net472 2.67μs 1.09ns 4.08ns 0.161 0 0 1.06 KB
#6936 CallElasticsearch net6.0 1.2μs 6.41ns 35.7ns 0.0112 0 0 984 B
#6936 CallElasticsearch netcoreapp3.1 1.56μs 1.21ns 4.54ns 0.00796 0 0 984 B
#6936 CallElasticsearch net472 2.52μs 2.2ns 8.52ns 0.151 0 0 1 KB
#6936 CallElasticsearchAsync net6.0 1.24μs 0.961ns 3.72ns 0.0122 0 0 960 B
#6936 CallElasticsearchAsync netcoreapp3.1 1.61μs 2.69ns 9.69ns 0.00797 0 0 1.03 KB
#6936 CallElasticsearchAsync net472 2.54μs 0.882ns 3.3ns 0.165 0 0 1.06 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.35μs 1.41ns 5.46ns 0.0134 0 0 960 B
master ExecuteAsync netcoreapp3.1 1.64μs 8.7ns 45.2ns 0.00813 0 0 960 B
master ExecuteAsync net472 1.77μs 1.02ns 3.83ns 0.142 0 0 923 B
#6936 ExecuteAsync net6.0 1.41μs 1.01ns 3.93ns 0.0071 0 0 960 B
#6936 ExecuteAsync netcoreapp3.1 1.6μs 1.97ns 7.37ns 0.00795 0 0 960 B
#6936 ExecuteAsync net472 1.83μs 1.2ns 4.63ns 0.146 0 0 923 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 4.39μs 3.18ns 12.3ns 0.022 0 0 2.32 KB
master SendAsync netcoreapp3.1 5.2μs 5.02ns 18.1ns 0.0259 0 0 2.86 KB
master SendAsync net472 7.42μs 2.74ns 10.3ns 0.482 0 0 3.13 KB
#6936 SendAsync net6.0 4.59μs 18.4ns 78ns 0.0227 0 0 2.32 KB
#6936 SendAsync netcoreapp3.1 5.34μs 2.89ns 11.2ns 0.0267 0 0 2.86 KB
#6936 SendAsync net472 7.43μs 6.4ns 24ns 0.485 0 0 3.13 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 1.56μs 1.45ns 5.01ns 0.0236 0 0 1.71 KB
master EnrichedLog netcoreapp3.1 2.41μs 4.06ns 14ns 0.012 0 0 1.71 KB
master EnrichedLog net472 2.66μs 2.48ns 9.26ns 0.251 0 0 1.64 KB
#6936 EnrichedLog net6.0 1.55μs 1.27ns 4.76ns 0.0232 0 0 1.71 KB
#6936 EnrichedLog netcoreapp3.1 2.22μs 2.57ns 9.97ns 0.0224 0 0 1.71 KB
#6936 EnrichedLog net472 2.59μs 1.84ns 7.13ns 0.258 0 0 1.64 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 113μs 575ns 2.64μs 0 0 0 4.32 KB
master EnrichedLog netcoreapp3.1 119μs 364ns 1.41μs 0 0 0 4.32 KB
master EnrichedLog net472 153μs 226ns 875ns 0 0 0 4.51 KB
#6936 EnrichedLog net6.0 111μs 277ns 1.07μs 0 0 0 4.32 KB
#6936 EnrichedLog netcoreapp3.1 117μs 414ns 1.61μs 0 0 0 4.32 KB
#6936 EnrichedLog net472 151μs 346ns 1.34μs 0 0 0 4.51 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 3.02μs 2.92ns 11.3ns 0.0303 0 0 2.26 KB
master EnrichedLog netcoreapp3.1 4.35μs 3.58ns 13.4ns 0 0 0 2.26 KB
master EnrichedLog net472 4.82μs 2.86ns 10.7ns 0.312 0 0 2.09 KB
#6936 EnrichedLog net6.0 3.17μs 2ns 7.75ns 0.0318 0 0 2.26 KB
#6936 EnrichedLog netcoreapp3.1 4.22μs 2.65ns 9.9ns 0.021 0 0 2.26 KB
#6936 EnrichedLog net472 5.12μs 3.41ns 13.2ns 0.308 0 0 2.09 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.41μs 1.13ns 4.24ns 0.0143 0 0 1.15 KB
master SendReceive netcoreapp3.1 1.82μs 3.01ns 11.7ns 0.00885 0 0 1.15 KB
master SendReceive net472 2.1μs 1.9ns 7.35ns 0.176 0 0 1.16 KB
#6936 SendReceive net6.0 1.37μs 0.79ns 3.06ns 0.0138 0 0 1.15 KB
#6936 SendReceive netcoreapp3.1 1.85μs 4.64ns 17.3ns 0.00909 0 0 1.15 KB
#6936 SendReceive net472 2.13μs 1.06ns 4.09ns 0.181 0 0 1.16 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.79μs 1.95ns 7.55ns 0.0139 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 3.91μs 4.34ns 16.8ns 0.0197 0 0 1.69 KB
master EnrichedLog net472 4.45μs 4.87ns 18.2ns 0.31 0 0 2.08 KB
#6936 EnrichedLog net6.0 2.73μs 1.24ns 4.49ns 0.0136 0 0 1.64 KB
#6936 EnrichedLog netcoreapp3.1 3.99μs 3.11ns 11.2ns 0.0199 0 0 1.69 KB
#6936 EnrichedLog net472 4.46μs 2.59ns 9.7ns 0.314 0 0 2.08 KB
Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6936

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net472 1.179 778.05 917.42

Faster 🎉 in #6936

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0 1.142 462.98 405.25

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 463ns 0.23ns 0.83ns 0.00687 0 0 584 B
master StartFinishSpan netcoreapp3.1 569ns 0.309ns 1.15ns 0.00569 0 0 584 B
master StartFinishSpan net472 656ns 0.434ns 1.68ns 0.0922 0 0 586 B
master StartFinishScope net6.0 508ns 0.461ns 1.79ns 0.0101 0 0 704 B
master StartFinishScope netcoreapp3.1 681ns 0.641ns 2.4ns 0.00682 0 0 704 B
master StartFinishScope net472 779ns 0.282ns 1.09ns 0.105 0 0 666 B
#6936 StartFinishSpan net6.0 405ns 0.153ns 0.594ns 0.00831 0 0 584 B
#6936 StartFinishSpan netcoreapp3.1 554ns 0.624ns 2.34ns 0.00623 0 0 584 B
#6936 StartFinishSpan net472 646ns 0.281ns 1.09ns 0.0905 0 0 586 B
#6936 StartFinishScope net6.0 479ns 0.364ns 1.36ns 0.00951 0 0 704 B
#6936 StartFinishScope netcoreapp3.1 684ns 0.25ns 0.901ns 0.0069 0 0 704 B
#6936 StartFinishScope net472 917ns 0.459ns 1.78ns 0.106 0 0 666 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 674ns 0.586ns 2.27ns 0.0101 0 0 704 B
master RunOnMethodBegin netcoreapp3.1 951ns 0.877ns 3.28ns 0.00948 0 0 704 B
master RunOnMethodBegin net472 1.04μs 0.588ns 2.28ns 0.104 0 0 666 B
#6936 RunOnMethodBegin net6.0 691ns 0.423ns 1.64ns 0.00691 0 0 704 B
#6936 RunOnMethodBegin netcoreapp3.1 902ns 1.18ns 4.57ns 0.00903 0 0 704 B
#6936 RunOnMethodBegin net472 1.02μs 0.39ns 1.46ns 0.102 0 0 666 B

@andrewlock
Copy link
Member

andrewlock commented May 7, 2025

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6936) - mean (68ms)  : 66, 70
     .   : milestone, 68,
    master - mean (68ms)  : 65, 71
     .   : milestone, 68,

    section CallTarget+Inlining+NGEN
    This PR (6936) - mean (1,013ms)  : 989, 1036
     .   : milestone, 1013,
    master - mean (1,011ms)  : 986, 1035
     .   : milestone, 1011,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6936) - mean (103ms)  : 100, 105
     .   : milestone, 103,
    master - mean (102ms)  : 100, 105
     .   : milestone, 102,

    section CallTarget+Inlining+NGEN
    This PR (6936) - mean (690ms)  : 665, 714
     .   : milestone, 690,
    master - mean (698ms)  : 674, 721
     .   : milestone, 698,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6936) - mean (90ms)  : 88, 91
     .   : milestone, 90,
    master - mean (89ms)  : 87, 91
     .   : milestone, 89,

    section CallTarget+Inlining+NGEN
    This PR (6936) - mean (649ms)  : 624, 674
     .   : milestone, 649,
    master - mean (658ms)  : 634, 682
     .   : milestone, 658,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6936) - mean (189ms)  : 186, 193
     .   : milestone, 189,
    master - mean (190ms)  : 186, 193
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6936) - mean (1,112ms)  : 1087, 1137
     .   : milestone, 1112,
    master - mean (1,117ms)  : 1092, 1142
     .   : milestone, 1117,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6936) - mean (268ms)  : 264, 272
     .   : milestone, 268,
    master - mean (269ms)  : 266, 272
     .   : milestone, 269,

    section CallTarget+Inlining+NGEN
    This PR (6936) - mean (878ms)  : 842, 913
     .   : milestone, 878,
    master - mean (879ms)  : 846, 912
     .   : milestone, 879,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6936) - mean (262ms)  : 258, 265
     .   : milestone, 262,
    master - mean (262ms)  : 258, 266
     .   : milestone, 262,

    section CallTarget+Inlining+NGEN
    This PR (6936) - mean (863ms)  : 832, 894
     .   : milestone, 863,
    master - mean (869ms)  : 832, 907
     .   : milestone, 869,

Loading

// 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 😄

Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

LGTM if we can test and verify it 🙂

}
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();

// 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

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 😄

@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented May 12, 2025

Datadog Summary

✅ Dependencies    ❌ Pipelines

Next Steps

Test Optimization Report

All test runs ddbaf22 🔗

2 Total Test Services: 1 Failed, 1 Passed

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Test Service View
dd-trace-dotnet 659 0 0 249312 2277 17h 43m 22.32s Link
exploration_tests 0 0 0 22085 3 2m 16.08s Link

❌ Failed Tests (659)

This report shows up to 5 failed tests.

  • AspNetWebApi2TestsCallTargetIntegratedWithRouteTemplateExpansion+AspNetWebApi2TestsVirtualAppIntegratedWithFeatureFlag.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error
     
     Actual:   Falseave no remaining tags, but the following tags were found: ["dd.orignal_resource_name"="GET /api/delay-optional"]onds}, Service: sample/my-app}
    
  • AspNetWebApi2TestsCallTargetIntegratedWithRouteTemplateExpansion+AspNetWebApi2TestsVirtualAppIntegratedWithFeatureFlag.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error
     
     Actual:   Falseave no remaining tags, but the following tags were found: ["dd.orignal_resource_name"="GET /api/transient-failure/false"]ervice: sample/my-app}
    
  • AspNetWebApi2TestsCallTargetIntegratedWithRouteTemplateExpansion+AspNetWebApi2TestsVirtualAppIntegratedWithFeatureFlag.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error
     
     Actual:   Falseave no remaining tags, but the following tags were found: ["dd.orignal_resource_name"="GET /api2/optional/?"]}, Service: sample/my-app}
    
  • AspNetWebApi2TestsCallTargetIntegratedWithRouteTemplateExpansion+AspNetWebApi2TestsVirtualAppIntegratedWithFeatureFlag.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error
     
     Actual:   Falseave no remaining tags, but the following tags were found: ["dd.orignal_resource_name"="GET /api2/statuscode/?"]}, Service: sample/my-app}
    
  • AspNetWebApi2TestsCallTargetIntegratedWithRouteTemplateExpansion+AspNetWebApi2TestsVirtualAppIntegratedWithFeatureFlag.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error
     
     Actual:   Falseave no remaining tags, but the following tags were found: ["dd.orignal_resource_name"="GET /api2/transientfailure/true"]ervice: sample/my-app}
    

Was this helpful? Give us feedback!

Copy link
Contributor

@link04 link04 left a comment

Choose a reason for hiding this comment

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

LGTM only thing would be about supporting the edge cases... and if we can't with the PR then updating the docs so customers can know.

bouwkast added 3 commits May 22, 2025 11:53
This is necessary as we make a sampling decision in IIS
without a resource name, which then means that if it is the
root span for the trace context that we won't
take into account any rules based on resource name.
@bouwkast bouwkast force-pushed the steven/resource-based-sampling-aspnet-fix branch from bdaf097 to bbb1cfc Compare May 22, 2025 15:53
@bouwkast bouwkast marked this pull request as ready for review May 22, 2025 18:20
@bouwkast bouwkast requested review from a team as code owners May 22, 2025 18:20
@bouwkast bouwkast merged commit 8b1a9ff into master May 23, 2025
128 of 129 checks passed
@bouwkast bouwkast deleted the steven/resource-based-sampling-aspnet-fix branch May 23, 2025 17:55
@github-actions github-actions bot added this to the vNext-v3 milestone May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants