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.

Sign up for GitHub

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

Jump to bottom

Fix: Rabbitmq7 header injection overwrites user supplied basic properties [from external PR #6730] #6753

Open

lucaspimentel wants to merge 17 commits into master from fix-rabbitmq-7-header-injection

Member

lucaspimentel commented Mar 10, 2025 •

edited

Loading

Based on #6730 by @johang88.

Summary of changes

Fixes the header injection so that basic properties are preserved from the supplied argument list in case PopulateBasicPropertiesHeaders returns null which happens if the basic properties argument is writable or if no changes had to be made to it.

Reason for change

Fixes a bug that would remove all user supplied basic properties.

Implementation details

Stores the basicproperties argument in the active scope and retrieves it a null return value is encountered. Changed CachedBasicPropertiesHelper to call the copy constructor that takes a IReadOnlyBasicProperties as argument.

Test coverage

Added basic properties to one of the rabbitmq7 test methods as well as assertions.

Other details

Fixes #6723

lucaspimentel added type:bug area:integrations labels

lucaspimentel mentioned this pull request

Fix: Rabbitmq7 header injection overwrites user supplied basic properties #6730

Open

lucaspimentel marked this pull request as ready for review

March 10, 2025 17:25

lucaspimentel requested review from a team as code owners

March 10, 2025 17:25

andrewlock reviewed

View reviewed changes

...er/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/RabbitMQ/CachedBasicPropertiesHelper.cs Outdated Show resolved Hide resolved

....Trace/ClrProfiler/AutoInstrumentation/RabbitMQ/PopulateBasicPropertiesHeadersIntegration.cs Outdated Show resolved Hide resolved

....Trace/ClrProfiler/AutoInstrumentation/RabbitMQ/PopulateBasicPropertiesHeadersIntegration.cs Outdated Show resolved Hide resolved

....Trace/ClrProfiler/AutoInstrumentation/RabbitMQ/PopulateBasicPropertiesHeadersIntegration.cs Outdated Show resolved Hide resolved

.../test/Datadog.Trace.ClrProfiler.Managed.Tests/Datadog.Trace.ClrProfiler.Managed.Tests.csproj Show resolved Hide resolved

bouwkast reviewed

View reviewed changes

.../test/Datadog.Trace.ClrProfiler.Managed.Tests/Datadog.Trace.ClrProfiler.Managed.Tests.csproj Show resolved Hide resolved

...e.ClrProfiler.Managed.Tests/AutoInstrumentation/RabbitMQ/CachedBasicPropertiesHelperTests.cs Show resolved Hide resolved

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

edited

Loading

Datadog Report

All test runs 8e13f07 🔗

❌ 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`	49	0	0	565449	4459	34h 2m 18.33s	Link
`exploration_tests`	0	0	0	22085	3	2m 8.61s	Link

❌ Failed Tests (49)

This report shows up to 5 failed tests.

HandleProduceAndConsume - Datadog.Trace.ClrProfiler.IntegrationTests.DataStreamsMonitoringRabbitMQTests - Details

Expand for error

 Results do not match.
 Differences:
 Received: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.received.txt
 Verified: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.verified.txt
 Received Content:
 [
   {
     EdgeTags: [
       direction:out,
       exchange:FanoutExchange,
 ...

HandleProduceAndConsume - Datadog.Trace.ClrProfiler.IntegrationTests.DataStreamsMonitoringRabbitMQTests - Details

Expand for error

 Results do not match.
 Differences:
 Received: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.received.txt
 Verified: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.verified.txt
 Received Content:
 [
   {
     EdgeTags: [
       direction:out,
       exchange:FanoutExchange,
 ...

HandleProduceAndConsume - Datadog.Trace.ClrProfiler.IntegrationTests.DataStreamsMonitoringRabbitMQTests - Details

Expand for error

 Results do not match.
 Differences:
 Received: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.received.txt
 Verified: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.verified.txt
 Received Content:
 [
   {
     EdgeTags: [
       direction:out,
       exchange:FanoutExchange,
 ...

HandleProduceAndConsume - Datadog.Trace.ClrProfiler.IntegrationTests.DataStreamsMonitoringRabbitMQTests - Details

Expand for error

 Results do not match.
 Differences:
 Received: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.received.txt
 Verified: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.verified.txt
 Received Content:
 [
   {
     EdgeTags: [
       direction:out,
       exchange:FanoutExchange,
 ...

HandleProduceAndConsume - Datadog.Trace.ClrProfiler.IntegrationTests.DataStreamsMonitoringRabbitMQTests - Details

Expand for error

 Results do not match.
 Differences:
 Received: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.received.txt
 Verified: DataStreamsMonitoringRabbitMQTests.HandleProduceAndConsume.verified.txt
 Received Content:
 [
   {
     EdgeTags: [
       direction:out,
       exchange:FanoutExchange,
 ...

Member

andrewlock commented Mar 10, 2025 •

edited

Loading

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 (6753) - mean (69ms)  : 66, 71
     .   : milestone, 69,
    master - mean (70ms)  : 67, 74
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6753) - mean (1,000ms)  : 985, 1015
     .   : milestone, 1000,
    master - mean (1,009ms)  : 989, 1028
     .   : milestone, 1009,

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

    section CallTarget+Inlining+NGEN
    This PR (6753) - mean (679ms)  : 661, 696
     .   : milestone, 679,
    master - mean (689ms)  : 671, 707
     .   : milestone, 689,

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

    section CallTarget+Inlining+NGEN
    This PR (6753) - mean (637ms)  : 622, 653
     .   : milestone, 637,
    master - mean (645ms)  : 632, 659
     .   : milestone, 645,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6753) - mean (190ms)  : 185, 195
     .   : milestone, 190,
    master - mean (189ms)  : 186, 192
     .   : milestone, 189,

    section CallTarget+Inlining+NGEN
    This PR (6753) - mean (1,102ms)  : 1072, 1133
     .   : milestone, 1102,
    master - mean (1,103ms)  : 1077, 1128
     .   : milestone, 1103,

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

    section CallTarget+Inlining+NGEN
    This PR (6753) - mean (871ms)  : 830, 912
     .   : milestone, 871,
    master - mean (872ms)  : 844, 900
     .   : milestone, 872,

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

    section CallTarget+Inlining+NGEN
    This PR (6753) - mean (854ms)  : 815, 893
     .   : milestone, 854,
    master - mean (862ms)  : 827, 897
     .   : milestone, 862,

lucaspimentel force-pushed the fix-rabbitmq-7-header-injection branch from 41124da to 74669a8 Compare

March 11, 2025 15:57

Member

andrewlock commented Mar 11, 2025 •

edited

Loading

Benchmarks Report for tracer 🐌

Benchmarks for #6753 compared to master:

1 benchmarks are faster, with geometric mean 1.115
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.33μs	47.7ns	360ns	0.0198	0.00793	0	5.61 KB
master	`StartStopWithChild`	netcoreapp3.1	10.5μs	57.9ns	375ns	0.0208	0.0104	0	5.81 KB
master	`StartStopWithChild`	net472	16.4μs	57ns	221ns	1.04	0.304	0.0961	6.21 KB
#6753	`StartStopWithChild`	net6.0	8.5μs	49.4ns	425ns	0.0215	0.0129	0.0043	5.61 KB
#6753	`StartStopWithChild`	netcoreapp3.1	10.4μs	57ns	407ns	0.0217	0.0109	0	5.8 KB
#6753	`StartStopWithChild`	net472	16.5μs	51ns	198ns	1.05	0.312	0.096	6.2 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	490μs	412ns	1.59μs	0	0	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	655μs	599ns	2.32μs	0	0	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	863μs	675ns	2.52μs	0.428	0	0	3.3 KB
#6753	`WriteAndFlushEnrichedTraces`	net6.0	502μs	453ns	1.75μs	0	0	0	2.7 KB
#6753	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	673μs	746ns	2.89μs	0	0	0	2.7 KB
#6753	`WriteAndFlushEnrichedTraces`	net472	869μs	569ns	2.2μs	0.431	0	0	3.3 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	132μs	544ns	2.11μs	0.189	0	0	14.47 KB
master	`SendRequest`	netcoreapp3.1	150μs	245ns	917ns	0.15	0	0	17.27 KB
master	`SendRequest`	net472	0ns	0ns	0ns	0	0	0	0 b
#6753	`SendRequest`	net6.0	132μs	472ns	1.83μs	0.131	0	0	14.47 KB
#6753	`SendRequest`	netcoreapp3.1	151μs	205ns	794ns	0.225	0	0	17.27 KB
#6753	`SendRequest`	net472	0.00129ns	0.000241ns	0.000868ns	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	580μs	3.27μs	22.4μs	0.563	0	0	41.68 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	692μs	3.94μs	27.3μs	0.338	0	0	41.74 KB
master	`WriteAndFlushEnrichedTraces`	net472	848μs	3.71μs	14.4μs	8.45	2.53	0.422	53.3 KB
#6753	`WriteAndFlushEnrichedTraces`	net6.0	595μs	3.37μs	24μs	0.641	0	0	41.52 KB
#6753	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	726μs	4.18μs	34.5μs	0.357	0	0	41.76 KB
#6753	`WriteAndFlushEnrichedTraces`	net472	882μs	4.46μs	20.4μs	8.25	2.6	0.434	53.27 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`ExecuteNonQuery`	net6.0	1.44μs	1.62ns	6.27ns	0.0143	0	0	1.02 KB
master	`ExecuteNonQuery`	netcoreapp3.1	1.81μs	3.72ns	14.4ns	0.0135	0	0	1.02 KB
master	`ExecuteNonQuery`	net472	2.02μs	2.09ns	8.09ns	0.156	0.001	0	987 B
#6753	`ExecuteNonQuery`	net6.0	1.41μs	2.28ns	8.84ns	0.0141	0	0	1.02 KB
#6753	`ExecuteNonQuery`	netcoreapp3.1	1.75μs	1.31ns	4.92ns	0.0142	0	0	1.02 KB
#6753	`ExecuteNonQuery`	net472	2.21μs	2.81ns	10.9ns	0.156	0.00111	0	987 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`CallElasticsearch`	net6.0	1.22μs	0.685ns	2.56ns	0.0134	0	0	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.57μs	0.578ns	2.16ns	0.0133	0	0	976 B
master	`CallElasticsearch`	net472	2.45μs	1.67ns	6.46ns	0.157	0	0	995 B
master	`CallElasticsearchAsync`	net6.0	1.31μs	0.423ns	1.58ns	0.0131	0	0	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.73μs	0.38ns	1.32ns	0.0138	0	0	1.02 KB
master	`CallElasticsearchAsync`	net472	2.63μs	1.84ns	7.13ns	0.166	0	0	1.05 KB
#6753	`CallElasticsearch`	net6.0	1.21μs	0.613ns	2.29ns	0.0139	0	0	976 B
#6753	`CallElasticsearch`	netcoreapp3.1	1.6μs	1.13ns	4.22ns	0.0127	0	0	976 B
#6753	`CallElasticsearch`	net472	2.63μs	2.36ns	9.16ns	0.157	0	0	995 B
#6753	`CallElasticsearchAsync`	net6.0	1.44μs	0.696ns	2.61ns	0.013	0	0	952 B
#6753	`CallElasticsearchAsync`	netcoreapp3.1	1.71μs	0.963ns	3.47ns	0.0136	0	0	1.02 KB
#6753	`CallElasticsearchAsync`	net472	2.8μs	1.43ns	5.36ns	0.166	0	0	1.05 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.34μs	0.731ns	2.73ns	0.0133	0	0	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.54μs	0.991ns	3.71ns	0.0126	0	0	952 B
master	`ExecuteAsync`	net472	1.86μs	0.496ns	1.92ns	0.145	0	0	915 B
#6753	`ExecuteAsync`	net6.0	1.28μs	0.422ns	1.58ns	0.013	0	0	952 B
#6753	`ExecuteAsync`	netcoreapp3.1	1.69μs	0.796ns	3.08ns	0.0127	0	0	952 B
#6753	`ExecuteAsync`	net472	1.87μs	0.243ns	0.842ns	0.145	0	0	915 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.37μs	6.82ns	26.4ns	0.0326	0	0	2.31 KB
master	`SendAsync`	netcoreapp3.1	5.44μs	2.54ns	9.84ns	0.0374	0	0	2.85 KB
master	`SendAsync`	net472	7.35μs	1.4ns	5.24ns	0.496	0	0	3.12 KB
#6753	`SendAsync`	net6.0	4.5μs	2.06ns	7.69ns	0.0314	0	0	2.31 KB
#6753	`SendAsync`	netcoreapp3.1	5.35μs	1.88ns	6.79ns	0.0391	0	0	2.85 KB
#6753	`SendAsync`	net472	7.49μs	2.38ns	9.21ns	0.494	0	0	3.12 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.57μs	1.16ns	4.5ns	0.0235	0	0	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.32μs	1.24ns	4.63ns	0.022	0	0	1.64 KB
master	`EnrichedLog`	net472	2.66μs	0.637ns	2.47ns	0.249	0	0	1.57 KB
#6753	`EnrichedLog`	net6.0	1.47μs	0.904ns	3.38ns	0.0229	0	0	1.64 KB
#6753	`EnrichedLog`	netcoreapp3.1	2.29μs	0.94ns	3.64ns	0.0219	0	0	1.64 KB
#6753	`EnrichedLog`	net472	2.49μs	0.832ns	3.22ns	0.25	0	0	1.57 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	112μs	179ns	694ns	0.0561	0	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	116μs	133ns	516ns	0.0576	0	0	4.28 KB
master	`EnrichedLog`	net472	150μs	202ns	782ns	0.674	0.225	0	4.46 KB
#6753	`EnrichedLog`	net6.0	112μs	94.4ns	366ns	0.0555	0	0	4.28 KB
#6753	`EnrichedLog`	netcoreapp3.1	115μs	127ns	476ns	0.0575	0	0	4.28 KB
#6753	`EnrichedLog`	net472	149μs	152ns	588ns	0.674	0.225	0	4.46 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.01μs	0.939ns	3.51ns	0.0302	0	0	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.33μs	1.52ns	5.87ns	0.0303	0	0	2.2 KB
master	`EnrichedLog`	net472	4.89μs	1.16ns	4.49ns	0.321	0	0	2.02 KB
#6753	`EnrichedLog`	net6.0	3.08μs	0.735ns	2.75ns	0.0309	0	0	2.2 KB
#6753	`EnrichedLog`	netcoreapp3.1	4.15μs	1.43ns	5.53ns	0.029	0	0	2.2 KB
#6753	`EnrichedLog`	net472	4.79μs	1.14ns	4.41ns	0.321	0	0	2.02 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.3μs	0.482ns	1.81ns	0.0163	0	0	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.8μs	0.804ns	3.11ns	0.0154	0	0	1.14 KB
master	`SendReceive`	net472	2.14μs	1.55ns	6.02ns	0.183	0	0	1.16 KB
#6753	`SendReceive`	net6.0	1.34μs	0.489ns	1.89ns	0.0162	0	0	1.14 KB
#6753	`SendReceive`	netcoreapp3.1	1.87μs	1.32ns	5.11ns	0.0148	0	0	1.14 KB
#6753	`SendReceive`	net472	2.06μs	0.48ns	1.86ns	0.183	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.69μs	0.868ns	3.36ns	0.0215	0	0	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	4.09μs	1.23ns	4.61ns	0.0228	0	0	1.65 KB
master	`EnrichedLog`	net472	4.29μs	2.21ns	7.98ns	0.323	0	0	2.04 KB
#6753	`EnrichedLog`	net6.0	2.82μs	0.918ns	3.43ns	0.0225	0	0	1.6 KB
#6753	`EnrichedLog`	netcoreapp3.1	3.9μs	1.65ns	6.39ns	0.0214	0	0	1.65 KB
#6753	`EnrichedLog`	net472	4.53μs	2.98ns	11.5ns	0.323	0	0	2.04 KB

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

Faster 🎉 in #6753

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0	1.115	546.48	490.13

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`StartFinishSpan`	net6.0	435ns	1.04ns	4.04ns	0.00803	0	0	576 B
master	`StartFinishSpan`	netcoreapp3.1	562ns	0.624ns	2.42ns	0.00782	0	0	576 B
master	`StartFinishSpan`	net472	651ns	0.34ns	1.32ns	0.0916	0	0	578 B
master	`StartFinishScope`	net6.0	546ns	0.0758ns	0.284ns	0.00983	0	0	696 B
master	`StartFinishScope`	netcoreapp3.1	697ns	0.279ns	1.04ns	0.00941	0	0	696 B
master	`StartFinishScope`	net472	798ns	0.32ns	1.24ns	0.105	0	0	658 B
#6753	`StartFinishSpan`	net6.0	397ns	0.193ns	0.746ns	0.00815	0	0	576 B
#6753	`StartFinishSpan`	netcoreapp3.1	577ns	0.351ns	1.36ns	0.00754	0	0	576 B
#6753	`StartFinishSpan`	net472	592ns	0.227ns	0.818ns	0.0916	0	0	578 B
#6753	`StartFinishScope`	net6.0	490ns	0.122ns	0.455ns	0.00985	0	0	696 B
#6753	`StartFinishScope`	netcoreapp3.1	675ns	0.332ns	1.28ns	0.00953	0	0	696 B
#6753	`StartFinishScope`	net472	782ns	0.211ns	0.79ns	0.104	0	0	658 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	602ns	0.161ns	0.623ns	0.00966	0	0	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.04μs	1.13ns	4.37ns	0.00933	0	0	696 B
master	`RunOnMethodBegin`	net472	1.04μs	0.68ns	2.64ns	0.104	0	0	658 B
#6753	`RunOnMethodBegin`	net6.0	666ns	0.165ns	0.638ns	0.00966	0	0	696 B
#6753	`RunOnMethodBegin`	netcoreapp3.1	964ns	0.46ns	1.72ns	0.00918	0	0	696 B
#6753	`RunOnMethodBegin`	net472	1.12μs	5.3ns	21.2ns	0.104	0	0	658 B

andrewlock reviewed

View reviewed changes

Member

andrewlock left a comment

A couple of suggestions, and once the tests pass so we know it works I think we'll be good! 😄

....Trace/ClrProfiler/AutoInstrumentation/RabbitMQ/PopulateBasicPropertiesHeadersIntegration.cs Outdated Show resolved Hide resolved

....Trace/ClrProfiler/AutoInstrumentation/RabbitMQ/PopulateBasicPropertiesHeadersIntegration.cs Outdated Show resolved Hide resolved

Johan Gustafsson and others added 14 commits

March 12, 2025 11:05


          Fix: Rabbitmq7 header injection overwrites user supplied basic proper…

7fdf8e0

…ties


          Fixed CachedBasicPropertiesHelper Activator

5ac24f0


          small code clean up and reformatting

1f13a63


          create a static dynamic method, use return type in Func<,>

fb973ca


          code comments

4b95ddc


          add tests for CachedBasicPropertiesHelper<T>.CreateHeaders()

f5ca602


          remove generic constraint to avoid the extra boxing

f3d2769


          wrap static initialization in try/catch

ec9af0f


          add log for invalid operation

3a7e270


          check for the actual types

92cf9e9


          use TReturn instead of GetType()

cfa81de

Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>


          check for null and avoid GetType()

0d09351

Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>


          remove unused variable

0580e60


          add more code comments

2be7c3f

lucaspimentel force-pushed the fix-rabbitmq-7-header-injection branch from 1f0708e to 2be7c3f Compare

March 12, 2025 15:28

lucaspimentel added 2 commits

March 12, 2025 11:38


          add null check, reformat code

617dcc8


          convert UTF-8 bytes to string before comparing

76e7f8b

lucaspimentel changed the title ~~Fix RabbitMQ 7 header injection~~ Fix: Rabbitmq7 header injection overwrites user supplied basic properties [from external PR #6730]


          add new spapshots for RabbitMQ.Client 7.x

8e13f07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:integrations type:bug