-
Notifications
You must be signed in to change notification settings - Fork 143
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
Add support for RabbitMQ v7 #6479
base: andrew/value-task-support
Are you sure you want to change the base?
Conversation
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:
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 (6479) - mean (68ms) : 66, 70
. : milestone, 68,
master - mean (68ms) : 66, 71
. : milestone, 68,
section CallTarget+Inlining+NGEN
This PR (6479) - mean (979ms) : 948, 1010
. : milestone, 979,
master - mean (978ms) : 957, 1000
. : milestone, 978,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6479) - mean (107ms) : 104, 110
. : milestone, 107,
master - mean (107ms) : 104, 109
. : milestone, 107,
section CallTarget+Inlining+NGEN
This PR (6479) - mean (676ms) : 659, 693
. : milestone, 676,
master - mean (678ms) : 664, 693
. : milestone, 678,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6479) - mean (91ms) : 89, 93
. : milestone, 91,
master - mean (90ms) : 89, 92
. : milestone, 90,
section CallTarget+Inlining+NGEN
This PR (6479) - mean (631ms) : 615, 647
. : milestone, 631,
master - mean (632ms) : 616, 649
. : milestone, 632,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6479) - mean (193ms) : 189, 198
. : milestone, 193,
master - mean (193ms) : 189, 198
. : milestone, 193,
section CallTarget+Inlining+NGEN
This PR (6479) - mean (1,095ms) : 1070, 1120
. : milestone, 1095,
master - mean (1,095ms) : 1068, 1122
. : milestone, 1095,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6479) - mean (277ms) : 273, 282
. : milestone, 277,
master - mean (277ms) : 272, 282
. : milestone, 277,
section CallTarget+Inlining+NGEN
This PR (6479) - mean (868ms) : 844, 891
. : milestone, 868,
master - mean (870ms) : 840, 900
. : milestone, 870,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6479) - mean (267ms) : 262, 273
. : milestone, 267,
master - mean (267ms) : 262, 271
. : milestone, 267,
section CallTarget+Inlining+NGEN
This PR (6479) - mean (849ms) : 809, 889
. : milestone, 849,
master - mean (850ms) : 816, 883
. : milestone, 850,
|
Datadog ReportBranch report: ✅ 0 Failed, 472561 Passed, 3719 Skipped, 32h 58m 48.1s Total Time |
Benchmarks Report for tracer 🐌Benchmarks for #6479 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SpanBenchmark - Slower
|
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0 | 1.149 | 393.72 | 452.26 |
Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1 | 1.117 | 624.85 | 559.48 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StartFinishSpan |
net6.0 | 394ns | 0.22ns | 0.852ns | 0.00808 | 0 | 0 | 576 B |
master | StartFinishSpan |
netcoreapp3.1 | 625ns | 0.695ns | 2.6ns | 0.00764 | 0 | 0 | 576 B |
master | StartFinishSpan |
net472 | 658ns | 0.454ns | 1.76ns | 0.0917 | 0 | 0 | 578 B |
master | StartFinishScope |
net6.0 | 481ns | 0.199ns | 0.769ns | 0.00982 | 0 | 0 | 696 B |
master | StartFinishScope |
netcoreapp3.1 | 729ns | 1.06ns | 4.11ns | 0.00949 | 0 | 0 | 696 B |
master | StartFinishScope |
net472 | 846ns | 0.376ns | 1.36ns | 0.104 | 0 | 0 | 658 B |
#6479 | StartFinishSpan |
net6.0 | 452ns | 0.193ns | 0.722ns | 0.00819 | 0 | 0 | 576 B |
#6479 | StartFinishSpan |
netcoreapp3.1 | 560ns | 0.406ns | 1.52ns | 0.00785 | 0 | 0 | 576 B |
#6479 | StartFinishSpan |
net472 | 661ns | 0.269ns | 1.04ns | 0.0916 | 0 | 0 | 578 B |
#6479 | StartFinishScope |
net6.0 | 484ns | 0.267ns | 1.04ns | 0.00972 | 0 | 0 | 696 B |
#6479 | StartFinishScope |
netcoreapp3.1 | 803ns | 0.427ns | 1.65ns | 0.00951 | 0 | 0 | 696 B |
#6479 | StartFinishScope |
net472 | 928ns | 0.277ns | 1.07ns | 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 | 640ns | 1.74ns | 6.75ns | 0.00959 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
netcoreapp3.1 | 897ns | 1.04ns | 4.02ns | 0.00942 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
net472 | 1.05μs | 0.815ns | 3.16ns | 0.104 | 0 | 0 | 658 B |
#6479 | RunOnMethodBegin |
net6.0 | 584ns | 0.26ns | 0.937ns | 0.00969 | 0 | 0 | 696 B |
#6479 | RunOnMethodBegin |
netcoreapp3.1 | 916ns | 0.779ns | 3.02ns | 0.00936 | 0 | 0 | 696 B |
#6479 | RunOnMethodBegin |
net472 | 1.12μs | 0.768ns | 2.98ns | 0.104 | 0 | 0 | 658 B |
c47a319
to
c2b1675
Compare
ffc922e
to
457a93f
Compare
c2b1675
to
288fb51
Compare
457a93f
to
6c8dbe5
Compare
288fb51
to
882ea95
Compare
Summary of changes
Adds support for RabbitMQ v7
Reason for change
We want to support the latest versions of packages. 7.x.x was released ~6 weeks ago, with significant changes to both the public API and the internal API (which is probably why it hasn't been explicitly requested yet).
Implementation details
This one took a lot of work. In summary:
BasicProperties
because nowIReadOnlyBasicProperties
is null, which means we can't easily add the headers if they're not provided (to propagate context).#if RABBITMQ_7_0
my way in the existing methods, but was impossible to follow, so split into -pre and -post v7 methods instead.Test coverage
We get the same overall snapshots for v7 as we do for v6, so 🤞 Did an "all TFMs, all versions" run here (and a previous one for DSM)
Other details
Stacked on:
[DuckPropertyOrField]
#6463ValueTask
in .NET FX and < .NET Core 3.1 #6480Note
This PR actually highlighted a current limitation in our CallTarget instrumentation. If an API uses
ValueTask
, which comes from a library rather than being built-in (i.e. it's netfx or < .NET Core 3.1) then we don't instrument it correctly. Originally I worked around it, but there's a "proper" fix in #6480. Assuming that actually works and is merged, then we don't need the workaround, otherwise I can re-enable it.