Create an inferred span for AWS API Gateway #6624

bouwkast · 2025-02-04T00:04:36Z

Summary of changes

Creates an inferred span for AWS API Gateway from HTTP headers.

Reason for change

TBD

Implementation details

TBD

Test coverage

Some, yes.

Other details

datadog-ddstaging · 2025-02-04T00:26:35Z

Datadog Report

Branch report: steven/inferred-api-gateway
Commit report: a134b49
Test service: dd-trace-dotnet

❌ 66 Failed (0 Known Flaky), 489234 Passed, 4360 Skipped, 32h 38m 47.74s Total Time

❌ Failed Tests (66)

This report shows up to 5 failed tests.

CheckProperties - Datadog.Trace.Tests.Tagging.TagsListTests - Details

Expand for error

 Expected collection to contain only items matching Not(IsNullOrEmpty(x.Item2)), but 
 {
     {
         Item1 = SpanKind, 
         Item2 = <null>
     }
 } do(es) not match.

CheckProperties - Datadog.Trace.Tests.Tagging.TagsListTests - Details

Expand for error

 Expected propertyAndTagName to contain only items matching Not(IsNullOrEmpty(x.Item2)), but 
 {
     {
         Item1 = SpanKind, 
         Item2 = <null>
     }
 } do(es) not match.

CheckProperties - Datadog.Trace.Tests.Tagging.TagsListTests - Details

Expand for error

 Expected propertyAndTagName to contain only items matching Not(IsNullOrEmpty(x.Item2)), but 
 {
     {
         Item1 = SpanKind, 
         Item2 = <null>
     }
 } do(es) not match.

CheckProperties - Datadog.Trace.Tests.Tagging.TagsListTests - Details

Expand for error

 Expected collection to contain only items matching Not(IsNullOrEmpty(x.Item2)), but 
 {
     {
         Item1 = SpanKind, 
         Item2 = <null>
     }
 } do(es) not match.

CheckProperties - Datadog.Trace.Tests.Tagging.TagsListTests - Details

Expand for error

 Expected collection to contain only items matching Not(IsNullOrEmpty(x.Item2)), but 
 {
     {
         Item1 = SpanKind, 
         Item2 = <null>
     }
 } do(es) not match.

andrewlock · 2025-02-04T00:39:08Z

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.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6624) - mean (69ms)  : 65, 73
     .   : milestone, 69,
    master - mean (78ms)  : 73, 83
     .   : milestone, 78,

    section CallTarget+Inlining+NGEN
    This PR (6624) - mean (1,002ms)  : 973, 1032
     .   : milestone, 1002,
    master - mean (1,105ms)  : 1047, 1164
     .   : milestone, 1105,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6624) - mean (102ms)  : 100, 104
     .   : milestone, 102,
    master - mean (114ms)  : 107, 121
     .   : milestone, 114,

    section CallTarget+Inlining+NGEN
    This PR (6624) - mean (671ms)  : 648, 694
     .   : milestone, 671,
    master - mean (715ms)  : 678, 751
     .   : milestone, 715,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6624) - mean (89ms)  : 86, 91
     .   : milestone, 89,
    master - mean (92ms)  : 90, 94
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (6624) - mean (626ms)  : 611, 641
     .   : milestone, 626,
    master - mean (635ms)  : 617, 653
     .   : milestone, 635,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6624) - mean (191ms)  : 187, 194
     .   : milestone, 191,
    master - mean (191ms)  : 187, 196
     .   : milestone, 191,

    section CallTarget+Inlining+NGEN
    This PR (6624) - mean (1,110ms)  : 1081, 1139
     .   : milestone, 1110,
    master - mean (1,108ms)  : 1080, 1137
     .   : milestone, 1108,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6624) - mean (272ms)  : 266, 277
     .   : milestone, 272,
    master - mean (271ms)  : 267, 275
     .   : milestone, 271,

    section CallTarget+Inlining+NGEN
    This PR (6624) - mean (864ms)  : 834, 894
     .   : milestone, 864,
    master - mean (866ms)  : 835, 898
     .   : milestone, 866,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6624) - mean (263ms)  : 257, 268
     .   : milestone, 263,
    master - mean (262ms)  : 259, 266
     .   : milestone, 262,

    section CallTarget+Inlining+NGEN
    This PR (6624) - mean (841ms)  : 804, 879
     .   : milestone, 841,
    master - mean (847ms)  : 815, 879
     .   : milestone, 847,

andrewlock · 2025-02-04T01:22:54Z

Benchmarks Report for tracer 🐌

Benchmarks for #6624 compared to master:

1 benchmarks are faster, with geometric mean 1.163
4 benchmarks are slower, with geometric mean 1.236
1 benchmarks have fewer 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	8.15μs	48.2ns	480ns	0.0198	0.00793	0	5.61 KB
master	`StartStopWithChild`	netcoreapp3.1	10.3μs	58.1ns	406ns	0.0154	0.00515	0	5.8 KB
master	`StartStopWithChild`	net472	16μs	45.4ns	164ns	1.03	0.289	0.0882	6.22 KB
#6624	`StartStopWithChild`	net6.0	8.3μs	48.3ns	456ns	0.0166	0.00416	0	5.61 KB
#6624	`StartStopWithChild`	netcoreapp3.1	10.6μs	60.8ns	467ns	0.0201	0.00502	0	5.8 KB
#6624	`StartStopWithChild`	net472	16.2μs	45.5ns	176ns	1.06	0.327	0.0956	6.22 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	466μs	211ns	789ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	645μs	311ns	1.21μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	853μs	424ns	1.59μs	0.422	3.3 KB
#6624	`WriteAndFlushEnrichedTraces`	net6.0	479μs	350ns	1.31μs	0	2.7 KB
#6624	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	656μs	773ns	2.99μs	0	2.7 KB
#6624	`WriteAndFlushEnrichedTraces`	net472	861μs	622ns	2.41μs	0.422	3.3 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	130μs	485ns	1.88μs	0.198	14.47 KB
master	`SendRequest`	netcoreapp3.1	146μs	181ns	677ns	0.219	17.27 KB
master	`SendRequest`	net472	0.00237ns	0.00115ns	0.00445ns	0	0 b
#6624	`SendRequest`	net6.0	130μs	258ns	999ns	0.193	14.47 KB
#6624	`SendRequest`	netcoreapp3.1	149μs	336ns	1.3μs	0.219	17.27 KB
#6624	`SendRequest`	net472	0.00286ns	0.00125ns	0.00483ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Slower ⚠️ Fewer allocations 🎉

Slower ⚠️ in #6624

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0	1.322	557,834.57	737,660.03	bimodal
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1	1.246	723,049.20	900,793.84	bimodal

Fewer allocations 🎉 in #6624

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0	41.5 KB	41.23 KB	-271 B	-0.65%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	559μs	2.35μs	8.79μs	0.571	0	0	41.5 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	727μs	4.25μs	38.5μs	0.342	0	0	41.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	872μs	4.06μs	17.2μs	8.19	2.59	0.431	53.27 KB
#6624	`WriteAndFlushEnrichedTraces`	net6.0	730μs	9.4μs	94μs	0.584	0	0	41.23 KB
#6624	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	800μs	13.1μs	131μs	0.446	0	0	41.72 KB
#6624	`WriteAndFlushEnrichedTraces`	net472	875μs	4.04μs	23.2μs	7.95	2.27	0.568	53.26 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net6.0	1.3μs	0.825ns	3.2ns	0.0143	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.72μs	2.47ns	9.58ns	0.0139	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.05μs	2.53ns	9.8ns	0.156	0.00102	987 B
#6624	`ExecuteNonQuery`	net6.0	1.41μs	1.48ns	5.74ns	0.0141	0	1.02 KB
#6624	`ExecuteNonQuery`	netcoreapp3.1	1.85μs	0.751ns	2.91ns	0.0132	0	1.02 KB
#6624	`ExecuteNonQuery`	net472	2.09μs	1.63ns	5.86ns	0.157	0.00105	987 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net6.0	1.25μs	0.615ns	2.3ns	0.0137	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.53μs	0.438ns	1.58ns	0.0135	976 B
master	`CallElasticsearch`	net472	2.5μs	2.35ns	9.11ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.3μs	0.331ns	1.28ns	0.0136	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.69μs	1.07ns	4.02ns	0.0135	1.02 KB
master	`CallElasticsearchAsync`	net472	2.48μs	1.68ns	6.5ns	0.166	1.05 KB
#6624	`CallElasticsearch`	net6.0	1.28μs	0.505ns	1.95ns	0.0134	976 B
#6624	`CallElasticsearch`	netcoreapp3.1	1.54μs	0.826ns	2.98ns	0.0131	976 B
#6624	`CallElasticsearch`	net472	2.56μs	1.85ns	7.18ns	0.158	995 B
#6624	`CallElasticsearchAsync`	net6.0	1.31μs	1.51ns	5.66ns	0.0132	952 B
#6624	`CallElasticsearchAsync`	netcoreapp3.1	1.65μs	0.539ns	2.02ns	0.0139	1.02 KB
#6624	`CallElasticsearchAsync`	net472	2.58μs	1.1ns	4.1ns	0.167	1.05 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net6.0	1.25μs	0.418ns	1.51ns	0.0124	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.68μs	0.758ns	2.84ns	0.0131	952 B
master	`ExecuteAsync`	net472	1.92μs	0.92ns	3.56ns	0.145	915 B
#6624	`ExecuteAsync`	net6.0	1.34μs	1.01ns	3.79ns	0.0133	952 B
#6624	`ExecuteAsync`	netcoreapp3.1	1.58μs	0.813ns	3.04ns	0.0128	952 B
#6624	`ExecuteAsync`	net472	1.91μs	0.359ns	1.34ns	0.145	915 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.58μs	2.9ns	11.2ns	0.0321	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.22μs	1.99ns	7.7ns	0.0366	2.85 KB
master	`SendAsync`	net472	7.39μs	2.29ns	8.86ns	0.494	3.12 KB
#6624	`SendAsync`	net6.0	4.37μs	1.45ns	5.6ns	0.033	2.31 KB
#6624	`SendAsync`	netcoreapp3.1	5.19μs	2.6ns	9.37ns	0.0388	2.85 KB
#6624	`SendAsync`	net472	7.43μs	1.38ns	4.79ns	0.494	3.12 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.66μs	1.25ns	4.67ns	0.0229	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.18μs	0.612ns	2.37ns	0.0218	1.64 KB
master	`EnrichedLog`	net472	2.5μs	1.22ns	4.57ns	0.249	1.57 KB
#6624	`EnrichedLog`	net6.0	1.56μs	0.837ns	3.13ns	0.0226	1.64 KB
#6624	`EnrichedLog`	netcoreapp3.1	2.14μs	1.5ns	5.81ns	0.0216	1.64 KB
#6624	`EnrichedLog`	net472	2.58μs	1.48ns	5.75ns	0.249	1.57 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	113μs	98.3ns	368ns	0.0566	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	116μs	152ns	589ns	0.0579	0	4.28 KB
master	`EnrichedLog`	net472	151μs	192ns	742ns	0.676	0.225	4.46 KB
#6624	`EnrichedLog`	net6.0	111μs	175ns	679ns	0.0558	0	4.28 KB
#6624	`EnrichedLog`	netcoreapp3.1	118μs	314ns	1.18μs	0.0596	0	4.28 KB
#6624	`EnrichedLog`	net472	150μs	201ns	777ns	0.668	0.223	4.46 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.98μs	1.13ns	4.37ns	0.0299	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.06μs	0.794ns	3.08ns	0.0285	2.2 KB
master	`EnrichedLog`	net472	4.79μs	1.34ns	5.18ns	0.32	2.02 KB
#6624	`EnrichedLog`	net6.0	3.21μs	1.49ns	5.37ns	0.0303	2.2 KB
#6624	`EnrichedLog`	netcoreapp3.1	4.06μs	3.05ns	11.8ns	0.0304	2.2 KB
#6624	`EnrichedLog`	net472	4.81μs	2.14ns	8.29ns	0.321	2.02 KB

Benchmarks.Trace.RedisBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6624

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.RedisBenchmark.SendReceive‑net6.0	1.163	1,522.98	1,309.66

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.52μs	0.758ns	2.94ns	0.016	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.74μs	0.539ns	1.87ns	0.0157	1.14 KB
master	`SendReceive`	net472	2.05μs	0.814ns	3.15ns	0.183	1.16 KB
#6624	`SendReceive`	net6.0	1.31μs	0.361ns	1.35ns	0.0158	1.14 KB
#6624	`SendReceive`	netcoreapp3.1	1.78μs	0.766ns	2.97ns	0.0151	1.14 KB
#6624	`SendReceive`	net472	2.06μs	1.55ns	6.01ns	0.183	1.16 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.82μs	1.6ns	6.18ns	0.0226	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.85μs	1.41ns	5.47ns	0.0212	1.65 KB
master	`EnrichedLog`	net472	4.27μs	4.02ns	15.6ns	0.323	2.04 KB
#6624	`EnrichedLog`	net6.0	2.87μs	1.31ns	5.07ns	0.0215	1.6 KB
#6624	`EnrichedLog`	netcoreapp3.1	3.93μs	3.42ns	13.2ns	0.0216	1.65 KB
#6624	`EnrichedLog`	net472	4.15μs	2.54ns	9.49ns	0.323	2.04 KB

Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6624

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0	1.180	388.99	458.86

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	389ns	0.594ns	2.3ns	0.00805	576 B
master	`StartFinishSpan`	netcoreapp3.1	584ns	1.31ns	5.07ns	0.00778	576 B
master	`StartFinishSpan`	net472	615ns	1.6ns	6.21ns	0.0918	578 B
master	`StartFinishScope`	net6.0	486ns	0.582ns	2.18ns	0.00967	696 B
master	`StartFinishScope`	netcoreapp3.1	791ns	1.76ns	6.83ns	0.00941	696 B
master	`StartFinishScope`	net472	793ns	2.47ns	9.55ns	0.104	658 B
#6624	`StartFinishSpan`	net6.0	459ns	0.305ns	1.14ns	0.00806	576 B
#6624	`StartFinishSpan`	netcoreapp3.1	578ns	0.734ns	2.84ns	0.0079	576 B
#6624	`StartFinishSpan`	net472	599ns	1.35ns	5.24ns	0.0918	578 B
#6624	`StartFinishScope`	net6.0	531ns	1.23ns	4.77ns	0.00981	696 B
#6624	`StartFinishScope`	netcoreapp3.1	713ns	1.24ns	4.79ns	0.0094	696 B
#6624	`StartFinishScope`	net472	849ns	1.55ns	6ns	0.104	658 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6624

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑netcoreapp3.1	1.202	833.37	1,002.02

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	621ns	0.952ns	3.69ns	0.00983	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	835ns	1.49ns	5.77ns	0.00922	696 B
master	`RunOnMethodBegin`	net472	996ns	2.28ns	8.82ns	0.104	658 B
#6624	`RunOnMethodBegin`	net6.0	660ns	0.504ns	1.95ns	0.00959	696 B
#6624	`RunOnMethodBegin`	netcoreapp3.1	1μs	1.82ns	7.05ns	0.00944	696 B
#6624	`RunOnMethodBegin`	net472	1.11μs	1.08ns	4.2ns	0.104	658 B

...filer/AutoInstrumentation/AspNet/AsyncControllerActionInvoker_EndInvokeAction_Integration.cs

tracer/src/Datadog.Trace/PlatformHelpers/AspNetCoreHttpRequestHandler.cs

...filer/AutoInstrumentation/AspNet/AsyncControllerActionInvoker_EndInvokeAction_Integration.cs

andrewlock · 2025-02-10T13:41:50Z

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Proxy/InferredProxyData.cs

+    // x-dd-proxy
+    public string ProxyName { get; init; } = proxyName;
+
+    // x-dd-proxy-request-time-ms
+    public DateTimeOffset StartTime { get; init; } = startTime;
+
+    // x-dd-proxy-domain-name
+    public string DomainName { get; init; } = domainName;
+
+    // x-dd-proxy-httpmethod
+    public string HttpMethod { get; init; } = httpMethod;
+
+    // x-dd-proxy-path
+    public string Path { get; init; } = path;
+
+    // x-dd-proxy-stage
+    public string Stage { get; init; } = stage;


I assume this is all already defined (and implemented?) in an RFC, but that's a lot of extra headers, I'm surprised if they can't be at least partially combined?

It wasn't clear to me from the dev docs how to achieve this: https://docs.aws.amazon.com/apigateway/latest/developerguide/request-response-data-mappings.html . I did look into it by testing API Gateway directly but wasn't able to prove that it works.

For now, I think we should stick with these separate headers and in the future, if someone has time to research the exact configuration needed to collapse the headers into a single value, we can refactor/improve this with another header configuration. It is possible other gateway systems handle headers differently.

Another pro of having them split out for now is that there's no confusion to what the header needs to be.

that's a lot of extra headers

I commented the same thing in the RFC, but several languages were already implementing the feature at that point 😅

For now, I think we should stick with these separate headers and in the future...

Ideally, we should figure these things out before releasing this feature as GA, because once we have tracer versions deployed in the wild, even if we change the headers, we'll need to support these ones for a long time to avoid breaking changes. Is this less of an issue in AWS Lambda since users just need to update the Lambda layer?

I may be mistaken but I think it has already been released.

tracer/missing-nullability-files.csv

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

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AspNetMvcIntegration.cs

lucaspimentel · 2025-02-11T20:55:43Z

...filer/AutoInstrumentation/AspNet/AsyncControllerActionInvoker_EndInvokeAction_Integration.cs

            var httpContext = HttpContext.Current;

            try
            {
                scope = SharedItems.TryPopScope(httpContext, AspNetMvcIntegration.HttpContextKey);
+                proxyScope = SharedItems.TryPopScope(httpContext, AspNetMvcIntegration.HttpContextKey + ".proxy");


You know the drill 😄

...filer/AutoInstrumentation/AspNet/AsyncControllerActionInvoker_EndInvokeAction_Integration.cs

lucaspimentel · 2025-02-11T21:05:33Z

tracer/src/Datadog.Trace/Propagators/ParseUtility.cs

@@ -153,6 +153,69 @@ static bool TryParse(IEnumerable<string?> headerValues, ref bool hasValue, out i
            }
        }

+        public static ulong? ParseUInt64<TCarrier>(TCarrier headers, string headerName)
+            where TCarrier : IHeadersCollection


We want to move away from using IHeadersCollection in favor of TCarrierGetter/ICarrierGetter<TCarrier>. We used to have all these ParseXYZ() methods for IHeadersCollection before and removed them 😅

For example:

dd-trace-dotnet/tracer/src/Datadog.Trace/SpanContextPropagator.cs

Lines 261 to 262 in d939f81

private static ulong ParseUInt64<T>(T headers, string headerName)

where T : IHeadersCollection

(I picked a random commit from 2021, there's nothing significant about this one in particular)

Anything that is still using IHeadersCollection is legacy code that hasn't been refactored.

😅 Okay good thing that wasn't used as I totally went with the IHeadersCollection at first but opted for the TCarrier as it worked better

lucaspimentel · 2025-02-11T21:13:36Z

tracer/src/Datadog.Trace/Tagging/InferredProxyTags.cs

+
+namespace Datadog.Trace.Tagging;
+
+internal partial class InferredProxyTags : InstrumentationTags


Suggested change

internal partial class InferredProxyTags : InstrumentationTags

internal partial class InferredProxyTags : InstrumentationTags, IHasStatusCode

We have some helper extension methods that rely on tag types implementing IHasStatusCode. Consider reusing those extensions for setting status codes instead of doing it directly. They do some validation and check settings to determine which ones should be considered errors.

dd-trace-dotnet/tracer/src/Datadog.Trace/ExtensionMethods/SpanExtensions.cs

Lines 91 to 133 in 4e98576

internal static bool HasHttpStatusCode(this Span span)

{

if (span.Tags is IHasStatusCode statusCodeTags)

{

return statusCodeTags.HttpStatusCode is not null;

}

else

{

return span.GetTag(Tags.HttpStatusCode) is not null;

}

}

internal static void SetHttpStatusCode(this Span span, int statusCode, bool isServer, TracerSettings tracerSettings)

{

if (statusCode < 100 || statusCode >= 600)

{

// not a valid status code. Likely the default integer value

return;

}

string statusCodeString = ConvertStatusCodeToString(statusCode);

if (span.Tags is IHasStatusCode statusCodeTags)

{

statusCodeTags.HttpStatusCode = statusCodeString;

}

else

{

span.SetTag(Tags.HttpStatusCode, statusCodeString);

}

// Check the customers http statuses that should be marked as errors

if (tracerSettings.IsErrorStatusCode(statusCode, isServer))

{

span.Error = true;

// if an error message already exists (e.g. from a previous exception), don't replace it

if (string.IsNullOrEmpty(span.GetTag(Tags.ErrorMsg)))

{

span.SetTag(Tags.ErrorMsg, $"The HTTP response has status code {statusCodeString}.");

}

}

}

IHasStatusCode is also used by the normalizer, if we ever actually get to use that code 😅

dd-trace-dotnet/tracer/src/Datadog.Trace/Processors/NormalizerTraceProcessor.cs

Line 131 in 4e98576

if (span.Tags is IHasStatusCode statusCodeTags)

Ahhhh thanks didn't realize that!

Necessary to avoid unnecessary heap allocations on every single request.

wantsui · 2025-02-19T15:57:24Z

...g.Trace.ClrProfiler.Managed.Tests/AutoInstrumentation/Proxy/AwsApiGatewaySpanFactoryTests.cs

+        scope.Should().NotBeNull();
+        var span = scope!.Span;
+        span.Should().NotBeNull();
+        span.OperationName.Should().Be("aws.api-gateway");


The span name in the Python, Go, and JS implementation is aws.apigateway. Any way to change this to be consistent?

The way we implemented it in other languages was to have a map w/the header value of aws-apigateway to map to aws.apigateway. We want the header value to remain unchanged and we want the operation name to be slightly different which can be represented by a key value map.

That way we can add more values in the future if we have more proxy headers that correspond to different operation names

Suggested change

span.OperationName.Should().Be("aws.api-gateway");

span.OperationName.Should().Be("aws.apigateway");

Left a comment where it comes from as well, as this is just a test. Unsure how I got that - in there 😅

Refer to this as an example

Note, they also include the component value in the map which would change for future proxies as well that may not necessarily be equal to the proxy header

I think that the .NET implementation should adhere to that with the above suggestion applied here and in the operation name as well

bouwkast · 2025-02-20T16:45:16Z

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Proxy/AwsApiGatewaySpanFactory.cs

+/// </summary>
+internal class AwsApiGatewaySpanFactory : IInferredSpanFactory
+{
+    private const string OperationName = "aws.api-gateway";


This is incorrect see: #6624 (comment)
Unsure how I got that - in there 😅

Suggested change

private const string OperationName = "aws.api-gateway";

private const string OperationName = "aws.apigateway";

bouwkast · 2025-02-20T16:45:40Z

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Proxy/AwsApiGatewaySpanFactory.cs

+            var tags = new InferredProxyTags
+            {
+                HttpMethod = data.HttpMethod,
+                InstrumentationName = data.ProxyName, // TODO: check to make sure this is really what we want for the component tag


Suggested change

InstrumentationName = data.ProxyName, // TODO: check to make sure this is really what we want for the component tag

InstrumentationName = data.ProxyName,

FYI this is correct per my understanding

bouwkast · 2025-02-20T16:46:03Z

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Proxy/AwsApiGatewaySpanFactory.cs

+            scope.Span.ResourceName = resourceName;
+            scope.Span.Type = SpanTypes.Web;
+
+            // TODO RFC said to copy over all Errors - do we do this here or outside of this function as we do with the current spans


Suggested change

// TODO RFC said to copy over all Errors - do we do this here or outside of this function as we do with the current spans

Done! Error should be copied over correctly

bouwkast · 2025-02-20T16:47:12Z

...g.Trace.ClrProfiler.Managed.Tests/AutoInstrumentation/Proxy/AwsApiGatewaySpanFactoryTests.cs

+        scope.Should().NotBeNull();
+        var span = scope!.Span;
+        span.Should().NotBeNull();
+        span.OperationName.Should().Be("aws.api-gateway");


Suggested change

span.OperationName.Should().Be("aws.api-gateway");

span.OperationName.Should().Be("aws.apigateway");

Left a comment where it comes from as well, as this is just a test. Unsure how I got that - in there 😅

bouwkast · 2025-02-20T16:48:18Z

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Proxy/AwsApiGatewayExtractor.cs

+                return false;
+            }
+
+            // the remaining headers aren't necessarily required


I think we should make tests for these to see what happens as we will create the span if the two required headers are present.

wantsui reviewed Feb 5, 2025

View reviewed changes

...filer/AutoInstrumentation/AspNet/AsyncControllerActionInvoker_EndInvokeAction_Integration.cs Show resolved Hide resolved

wantsui reviewed Feb 5, 2025

View reviewed changes

tracer/src/Datadog.Trace/PlatformHelpers/AspNetCoreHttpRequestHandler.cs Outdated Show resolved Hide resolved

andrewlock reviewed Feb 10, 2025

View reviewed changes

...filer/AutoInstrumentation/AspNet/AsyncControllerActionInvoker_EndInvokeAction_Integration.cs Outdated Show resolved Hide resolved

andrewlock reviewed Feb 10, 2025

View reviewed changes