ignore entire `baggage` header if malformed #6743

lucaspimentel · 2025-03-04T21:16:11Z

Summary of changes

When parsing a baggage header, if it is malformed in any way, ignore the entire header instead of trying to extract the valid key/value pairs.

Reason for change

This is the defined behavior in the RFC and how we implement baggage in other tracing libraries.

Implementation details

Bail our as soon as we find any invalid format in the baggage header.

Test coverage

Fixed existing tests and added more test cases.

Other details

n/a

rachelyangdog

LGTM!

andrewlock · 2025-03-04T22:04:21Z

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 (6743) - mean (70ms)  : 68, 71
     .   : milestone, 70,
    master - mean (70ms)  : 65, 74
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6743) - mean (1,010ms)  : 980, 1039
     .   : milestone, 1010,
    master - mean (1,003ms)  : 977, 1029
     .   : milestone, 1003,

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

    section CallTarget+Inlining+NGEN
    This PR (6743) - mean (690ms)  : 672, 707
     .   : milestone, 690,
    master - mean (688ms)  : 664, 711
     .   : milestone, 688,

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

    section CallTarget+Inlining+NGEN
    This PR (6743) - mean (651ms)  : 623, 678
     .   : milestone, 651,
    master - mean (640ms)  : 626, 654
     .   : milestone, 640,

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

    section CallTarget+Inlining+NGEN
    This PR (6743) - mean (1,113ms)  : 1080, 1145
     .   : milestone, 1113,
    master - mean (1,107ms)  : 1081, 1133
     .   : milestone, 1107,

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

    section CallTarget+Inlining+NGEN
    This PR (6743) - mean (875ms)  : 842, 908
     .   : milestone, 875,
    master - mean (877ms)  : 842, 913
     .   : milestone, 877,

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

    section CallTarget+Inlining+NGEN
    This PR (6743) - mean (855ms)  : 827, 884
     .   : milestone, 855,
    master - mean (855ms)  : 817, 892
     .   : milestone, 855,

datadog-datadog-prod-us1 · 2025-03-04T22:18:15Z

Datadog Report

Branch report: lpimentel/ignore-invalid-baggage-header
Commit report: cc89c3c
Test service: dd-trace-dotnet

✅ 0 Failed, 248549 Passed, 2389 Skipped, 19h 58m 30.48s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests.UnixDomainSocketLargePayloadTests - Last Failure

Expand for error

 Expected collection to contain at least 10000 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.

andrewlock · 2025-03-04T22:40:55Z

Benchmarks Report for tracer 🐌

Benchmarks for #6743 compared to master:

1 benchmarks are slower, with geometric mean 1.119
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	8.19μs	46.8ns	338ns	0.0119	0.00398	0	5.6 KB
master	`StartStopWithChild`	netcoreapp3.1	10.7μs	60.3ns	435ns	0.015	0.00499	0	5.81 KB
master	`StartStopWithChild`	net472	16.2μs	47.8ns	185ns	1.04	0.299	0.0969	6.2 KB
#6743	`StartStopWithChild`	net6.0	8.34μs	47.6ns	340ns	0.0124	0.00414	0	5.61 KB
#6743	`StartStopWithChild`	netcoreapp3.1	10.6μs	57ns	323ns	0.0218	0.0109	0	5.8 KB
#6743	`StartStopWithChild`	net472	16μs	57.3ns	222ns	1.04	0.319	0.0957	6.21 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	483μs	358ns	1.39μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	644μs	635ns	2.46μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	849μs	397ns	1.54μs	0.422	3.3 KB
#6743	`WriteAndFlushEnrichedTraces`	net6.0	508μs	266ns	961ns	0	2.7 KB
#6743	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	665μs	147ns	548ns	0	2.7 KB
#6743	`WriteAndFlushEnrichedTraces`	net472	852μs	471ns	1.82μs	0.425	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	133μs	411ns	1.59μs	0.199	14.47 KB
master	`SendRequest`	netcoreapp3.1	151μs	212ns	794ns	0.227	17.27 KB
master	`SendRequest`	net472	0.000146ns	9.84E‑05ns	0.000368ns	0	0 b
#6743	`SendRequest`	net6.0	135μs	322ns	1.25μs	0.199	14.47 KB
#6743	`SendRequest`	netcoreapp3.1	151μs	138ns	536ns	0.151	17.27 KB
#6743	`SendRequest`	net472	0.000505ns	0.000171ns	0.000664ns	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	549μs	2.62μs	10.5μs	0.543	0	0	41.47 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	683μs	3.34μs	13.8μs	0.324	0	0	41.84 KB
master	`WriteAndFlushEnrichedTraces`	net472	878μs	3.73μs	13.9μs	8.19	2.59	0.431	53.32 KB
#6743	`WriteAndFlushEnrichedTraces`	net6.0	548μs	2.53μs	10.1μs	0.541	0	0	41.63 KB
#6743	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	664μs	3.47μs	16.6μs	0.334	0	0	41.8 KB
#6743	`WriteAndFlushEnrichedTraces`	net472	884μs	2.96μs	11.5μs	8.28	2.48	0.414	53.32 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.33μs	1.28ns	4.97ns	0.014	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.82μs	1.88ns	7.28ns	0.0133	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.12μs	1.29ns	4.81ns	0.156	0.00106	987 B
#6743	`ExecuteNonQuery`	net6.0	1.28μs	0.628ns	2.26ns	0.0141	0	1.02 KB
#6743	`ExecuteNonQuery`	netcoreapp3.1	1.83μs	1.46ns	5.47ns	0.0136	0	1.02 KB
#6743	`ExecuteNonQuery`	net472	2.02μs	1.6ns	6.18ns	0.156	0.00101	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.26μs	0.7ns	2.62ns	0.0139	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.55μs	3.3ns	12.3ns	0.0131	976 B
master	`CallElasticsearch`	net472	2.47μs	0.758ns	2.73ns	0.158	995 B
master	`CallElasticsearchAsync`	net6.0	1.24μs	1.08ns	4.18ns	0.0131	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.64μs	1.06ns	3.95ns	0.0131	1.02 KB
master	`CallElasticsearchAsync`	net472	2.64μs	1ns	3.62ns	0.166	1.05 KB
#6743	`CallElasticsearch`	net6.0	1.17μs	0.487ns	1.82ns	0.0135	976 B
#6743	`CallElasticsearch`	netcoreapp3.1	1.54μs	0.662ns	2.48ns	0.0131	976 B
#6743	`CallElasticsearch`	net472	2.48μs	1.62ns	6.05ns	0.158	995 B
#6743	`CallElasticsearchAsync`	net6.0	1.36μs	1.09ns	4.23ns	0.0129	952 B
#6743	`CallElasticsearchAsync`	netcoreapp3.1	1.61μs	0.865ns	3.12ns	0.0137	1.02 KB
#6743	`CallElasticsearchAsync`	net472	2.57μs	1.3ns	4.87ns	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.34μs	1.58ns	5.9ns	0.0133	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.69μs	1.57ns	6.08ns	0.0128	952 B
master	`ExecuteAsync`	net472	1.85μs	0.672ns	2.6ns	0.145	915 B
#6743	`ExecuteAsync`	net6.0	1.33μs	0.43ns	1.61ns	0.0133	952 B
#6743	`ExecuteAsync`	netcoreapp3.1	1.69μs	0.798ns	3.09ns	0.0126	952 B
#6743	`ExecuteAsync`	net472	1.81μs	0.304ns	1.14ns	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.36μs	1.04ns	3.75ns	0.0325	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.22μs	2.2ns	7.94ns	0.0393	2.85 KB
master	`SendAsync`	net472	7.45μs	2.36ns	9.15ns	0.492	3.12 KB
#6743	`SendAsync`	net6.0	4.36μs	1.42ns	5.49ns	0.0327	2.31 KB
#6743	`SendAsync`	netcoreapp3.1	5.3μs	2.06ns	7.72ns	0.0375	2.85 KB
#6743	`SendAsync`	net472	7.44μs	1.72ns	6.64ns	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.47μs	0.781ns	3.03ns	0.0234	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.14μs	5.9ns	22.9ns	0.0223	1.64 KB
master	`EnrichedLog`	net472	2.63μs	2.01ns	7.8ns	0.25	1.57 KB
#6743	`EnrichedLog`	net6.0	1.54μs	0.865ns	3.35ns	0.0227	1.64 KB
#6743	`EnrichedLog`	netcoreapp3.1	2.11μs	0.976ns	3.65ns	0.0224	1.64 KB
#6743	`EnrichedLog`	net472	2.6μs	1.03ns	3.85ns	0.25	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	114μs	158ns	612ns	0.0563	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	118μs	179ns	692ns	0	0	4.28 KB
master	`EnrichedLog`	net472	149μs	211ns	817ns	0.668	0.223	4.46 KB
#6743	`EnrichedLog`	net6.0	112μs	95.7ns	358ns	0.0554	0	4.28 KB
#6743	`EnrichedLog`	netcoreapp3.1	116μs	154ns	598ns	0.0589	0	4.28 KB
#6743	`EnrichedLog`	net472	150μs	199ns	770ns	0.673	0.224	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.9μs	0.691ns	2.59ns	0.0318	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.16μs	3.18ns	11.5ns	0.029	2.2 KB
master	`EnrichedLog`	net472	5.01μs	1.69ns	6.54ns	0.32	2.02 KB
#6743	`EnrichedLog`	net6.0	2.98μs	0.781ns	2.92ns	0.0297	2.2 KB
#6743	`EnrichedLog`	netcoreapp3.1	4.14μs	0.625ns	2.42ns	0.029	2.2 KB
#6743	`EnrichedLog`	net472	4.72μs	0.936ns	3.5ns	0.32	2.02 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.36μs	0.605ns	2.34ns	0.0156	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.74μs	0.624ns	2.41ns	0.0155	1.14 KB
master	`SendReceive`	net472	2.11μs	1.34ns	5.2ns	0.183	1.16 KB
#6743	`SendReceive`	net6.0	1.44μs	0.471ns	1.7ns	0.016	1.14 KB
#6743	`SendReceive`	netcoreapp3.1	1.85μs	1.07ns	4.15ns	0.0157	1.14 KB
#6743	`SendReceive`	net472	2.07μs	0.999ns	3.87ns	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.74μs	0.565ns	2.19ns	0.022	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.9μs	2.99ns	11.6ns	0.0216	1.65 KB
master	`EnrichedLog`	net472	4.46μs	2.17ns	8.11ns	0.324	2.04 KB
#6743	`EnrichedLog`	net6.0	2.82μs	0.723ns	2.71ns	0.0226	1.6 KB
#6743	`EnrichedLog`	netcoreapp3.1	3.83μs	1.44ns	5.58ns	0.023	1.65 KB
#6743	`EnrichedLog`	net472	4.25μs	3.45ns	13.4ns	0.322	2.04 KB

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

Slower ⚠️ in #6743

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net472	1.119	588.48	658.36

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	392ns	0.434ns	1.68ns	0.00798	576 B
master	`StartFinishSpan`	netcoreapp3.1	606ns	1.38ns	5.36ns	0.0079	576 B
master	`StartFinishSpan`	net472	589ns	0.892ns	3.46ns	0.0916	578 B
master	`StartFinishScope`	net6.0	481ns	0.798ns	3.09ns	0.00986	696 B
master	`StartFinishScope`	netcoreapp3.1	725ns	1.74ns	6.72ns	0.00921	696 B
master	`StartFinishScope`	net472	842ns	1.98ns	7.67ns	0.104	658 B
#6743	`StartFinishSpan`	net6.0	403ns	0.625ns	2.42ns	0.00813	576 B
#6743	`StartFinishSpan`	netcoreapp3.1	557ns	1.04ns	4.04ns	0.0078	576 B
#6743	`StartFinishSpan`	net472	657ns	1.12ns	4.05ns	0.0915	578 B
#6743	`StartFinishScope`	net6.0	525ns	0.71ns	2.75ns	0.0097	696 B
#6743	`StartFinishScope`	netcoreapp3.1	750ns	0.995ns	3.72ns	0.00936	696 B
#6743	`StartFinishScope`	net472	804ns	1.14ns	4.41ns	0.104	658 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	615ns	1.18ns	4.58ns	0.00981	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	922ns	1.49ns	5.78ns	0.00937	696 B
master	`RunOnMethodBegin`	net472	1.06μs	2.1ns	8.12ns	0.104	658 B
#6743	`RunOnMethodBegin`	net6.0	577ns	0.876ns	3.39ns	0.00981	696 B
#6743	`RunOnMethodBegin`	netcoreapp3.1	918ns	2.02ns	7.82ns	0.00935	696 B
#6743	`RunOnMethodBegin`	net472	1.07μs	1.99ns	7.73ns	0.104	658 B

lucaspimentel · 2025-03-05T19:04:24Z

Merge activity

Mar 5, 2:04 PM EST: A user merged this pull request with Graphite.

lucaspimentel added type:bug area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) labels Mar 4, 2025

lucaspimentel added 5 commits March 4, 2025 15:17

clarify comment

320a1d1

ignore nullability warning

85617f8

use HaveCount() in test

6252b57

ignore entire baggage header is any key/value pair is malformed

f72745d

fix tests and add test cases that combine valid/invalid pairs

4bf8896

lucaspimentel force-pushed the lpimentel/ignore-invalid-baggage-header branch from 0094e96 to 4bf8896 Compare March 4, 2025 21:17

lucaspimentel marked this pull request as ready for review March 4, 2025 21:17

lucaspimentel requested a review from a team as a code owner March 4, 2025 21:17

lucaspimentel added 3 commits March 4, 2025 15:26

check result from ParseHeader()

e5f64a9

clean up

b62405d

add code comments

cc89c3c

lucaspimentel requested review from a team and rachelyangdog March 4, 2025 21:32

rachelyangdog approved these changes Mar 4, 2025

View reviewed changes

zacharycmontoya approved these changes Mar 4, 2025

View reviewed changes

lucaspimentel merged commit 88ec1b1 into master Mar 5, 2025
143 of 146 checks passed

lucaspimentel deleted the lpimentel/ignore-invalid-baggage-header branch March 5, 2025 19:04

github-actions bot added this to the vNext-v3 milestone Mar 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ignore entire `baggage` header if malformed #6743

ignore entire `baggage` header if malformed #6743

lucaspimentel commented Mar 4, 2025

rachelyangdog left a comment

andrewlock commented Mar 4, 2025

datadog-datadog-prod-us1 bot commented Mar 4, 2025 •

edited

Loading

andrewlock commented Mar 4, 2025

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6743

Raw results

Raw results

lucaspimentel commented Mar 5, 2025

ignore entire baggage header if malformed #6743

ignore entire baggage header if malformed #6743

Conversation

lucaspimentel commented Mar 4, 2025

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

rachelyangdog left a comment

Choose a reason for hiding this comment

andrewlock commented Mar 4, 2025

Execution-Time Benchmarks Report ⏱️

datadog-datadog-prod-us1 bot commented Mar 4, 2025 • edited Loading

Datadog Report

New Flaky Tests (1)

andrewlock commented Mar 4, 2025

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #6743

Raw results

Raw results

lucaspimentel commented Mar 5, 2025

Merge activity

ignore entire `baggage` header if malformed #6743

ignore entire `baggage` header if malformed #6743

datadog-datadog-prod-us1 bot commented Mar 4, 2025 •

edited

Loading