Skip to content

Fix build when using C#11#3102

Merged
tonyredondo merged 2 commits into
masterfrom
tony/fix-build-in-csharp-11
Aug 23, 2022
Merged

Fix build when using C#11#3102
tonyredondo merged 2 commits into
masterfrom
tony/fix-build-in-csharp-11

Conversation

@tonyredondo

@tonyredondo tonyredondo commented Aug 19, 2022

Copy link
Copy Markdown
Member

Summary of changes

This PR contains workarounds to build the solution when using C#11

Reason for change

C#11 introduces a couple of breaking changes that are affecting us in the methods we kept for backward compatibility in the CallTargetInvoker class, especifically the CS8166 and CS8347

More info can be found in: https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/breaking-changes/compiler%20breaking%20changes%20-%20dotnet%207

image

@tonyredondo tonyredondo added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Aug 19, 2022
@tonyredondo tonyredondo self-assigned this Aug 19, 2022
@tonyredondo
tonyredondo requested a review from a team as a code owner August 19, 2022 09:40
@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

andrewlock commented Aug 19, 2022

Copy link
Copy Markdown
Member

Benchmarks Report 🐌

Benchmarks for #3102 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.292
  • 1 benchmarks are slower, with geometric mean 1.195
  • 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.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net472 717μs 1μs 3.89μs 0.359 0 0 3.18 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 455μs 582ns 2.26μs 0 0 0 2.58 KB
#3102 WriteAndFlushEnrichedTraces net472 732μs 425ns 1.65μs 0.363 0 0 3.18 KB
#3102 WriteAndFlushEnrichedTraces netcoreapp3.1 468μs 131ns 491ns 0 0 0 2.58 KB
Benchmarks.Trace.AppSecBodyBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #3102

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.AppSecBodyBenchmark.AllCycleSimpleBody‑net472 1.292 232.42 179.94

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net472 232ns 0.242ns 0.906ns 0.0676 0 0 425 B
master AllCycleSimpleBody netcoreapp3.1 235ns 0.269ns 1.04ns 0.00581 0 0 424 B
master AllCycleMoreComplexBody net472 184ns 0.252ns 0.975ns 0.0637 0 0 401 B
master AllCycleMoreComplexBody netcoreapp3.1 235ns 0.195ns 0.754ns 0.0054 0 0 400 B
master BodyExtractorSimpleBody net472 261ns 0.358ns 1.39ns 0.0574 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 221ns 0.272ns 1.02ns 0.00379 0 0 272 B
master BodyExtractorMoreComplexBody net472 14.8μs 12.7ns 47.4ns 1.21 0.0147 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 12.4μs 10.3ns 37.3ns 0.0923 0 0 6.75 KB
#3102 AllCycleSimpleBody net472 180ns 0.134ns 0.502ns 0.0676 0 0 425 B
#3102 AllCycleSimpleBody netcoreapp3.1 236ns 0.225ns 0.873ns 0.00579 0 0 424 B
#3102 AllCycleMoreComplexBody net472 182ns 0.172ns 0.664ns 0.0637 0 0 401 B
#3102 AllCycleMoreComplexBody netcoreapp3.1 237ns 0.271ns 1.05ns 0.0055 0 0 400 B
#3102 BodyExtractorSimpleBody net472 259ns 0.271ns 1.05ns 0.0573 0 0 361 B
#3102 BodyExtractorSimpleBody netcoreapp3.1 230ns 0.214ns 0.8ns 0.0038 0 0 272 B
#3102 BodyExtractorMoreComplexBody net472 14.5μs 59.2ns 222ns 1.21 0.0215 0 7.62 KB
#3102 BodyExtractorMoreComplexBody netcoreapp3.1 12.1μs 23.9ns 89.5ns 0.0908 0 0 6.75 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 net472 0.00178ns 0.00132ns 0.00513ns 0 0 0 0 b
master SendRequest netcoreapp3.1 179μs 255ns 987ns 0.267 0 0 20.33 KB
#3102 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3102 SendRequest netcoreapp3.1 179μs 205ns 792ns 0.18 0 0 20.33 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 net472 1.53μs 2.25ns 8.71ns 0.126 0.00076 0 794 B
master ExecuteNonQuery netcoreapp3.1 1.22μs 1.22ns 4.55ns 0.011 0 0 824 B
#3102 ExecuteNonQuery net472 1.58μs 1.56ns 6.06ns 0.126 0.000791 0 794 B
#3102 ExecuteNonQuery netcoreapp3.1 1.25μs 1.11ns 3.99ns 0.0108 0 0 824 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 net472 2.15μs 1.39ns 5ns 0.159 0 0 1 KB
master CallElasticsearch netcoreapp3.1 1.46μs 0.953ns 3.56ns 0.0132 0 0 984 B
master CallElasticsearchAsync net472 2.4μs 0.525ns 1.96ns 0.181 0 0 1.14 KB
master CallElasticsearchAsync netcoreapp3.1 1.45μs 0.719ns 2.69ns 0.0152 0 0 1.1 KB
#3102 CallElasticsearch net472 2.13μs 0.322ns 1.2ns 0.159 0 0 1 KB
#3102 CallElasticsearch netcoreapp3.1 1.36μs 1.2ns 4.49ns 0.0135 0 0 984 B
#3102 CallElasticsearchAsync net472 2.4μs 0.93ns 3.22ns 0.181 0 0 1.14 KB
#3102 CallElasticsearchAsync netcoreapp3.1 1.46μs 0.835ns 3.13ns 0.0148 0 0 1.1 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 net472 2.33μs 5.78ns 22.4ns 0.199 0 0 1.26 KB
master ExecuteAsync netcoreapp3.1 1.57μs 2.91ns 11.3ns 0.0169 0 0 1.22 KB
#3102 ExecuteAsync net472 2.41μs 2.95ns 11.4ns 0.2 0 0 1.26 KB
#3102 ExecuteAsync netcoreapp3.1 1.62μs 2.01ns 7.8ns 0.0168 0 0 1.22 KB
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net472 4.94μs 11.6ns 45ns 0.393 0 0 2.48 KB
master SendAsync netcoreapp3.1 3.21μs 6.92ns 26.8ns 0.0319 0 0 2.36 KB
#3102 SendAsync net472 4.97μs 7.7ns 29.8ns 0.394 0 0 2.48 KB
#3102 SendAsync netcoreapp3.1 3.29μs 4.59ns 17.8ns 0.0325 0 0 2.36 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 net472 2.81μs 0.637ns 2.38ns 0.264 0 0 1.66 KB
master EnrichedLog netcoreapp3.1 2.42μs 0.95ns 3.55ns 0.0229 0 0 1.73 KB
#3102 EnrichedLog net472 2.9μs 1.03ns 3.97ns 0.263 0 0 1.66 KB
#3102 EnrichedLog netcoreapp3.1 2.3μs 0.741ns 2.57ns 0.0231 0 0 1.73 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 net472 150μs 121ns 436ns 0.681 0.227 0 4.5 KB
master EnrichedLog netcoreapp3.1 114μs 252ns 974ns 0.0567 0 0 4.38 KB
#3102 EnrichedLog net472 149μs 65ns 243ns 0.666 0.222 0 4.5 KB
#3102 EnrichedLog netcoreapp3.1 113μs 93.8ns 338ns 0 0 0 4.38 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 net472 5.15μs 13.4ns 51.8ns 0.545 0.00256 0 3.43 KB
master EnrichedLog netcoreapp3.1 4.11μs 4.98ns 18.7ns 0.0515 0 0 3.8 KB
#3102 EnrichedLog net472 5.36μs 9.58ns 37.1ns 0.544 0.00269 0 3.43 KB
#3102 EnrichedLog netcoreapp3.1 4.13μs 10.2ns 39.7ns 0.0515 0 0 3.8 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 net472 2.08μs 0.618ns 2.39ns 0.194 0 0 1.22 KB
master SendReceive netcoreapp3.1 1.69μs 0.602ns 2.17ns 0.0161 0 0 1.21 KB
#3102 SendReceive net472 2.04μs 0.716ns 2.77ns 0.193 0 0 1.22 KB
#3102 SendReceive netcoreapp3.1 1.71μs 0.595ns 2.23ns 0.0163 0 0 1.21 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 net472 4.69μs 2.09ns 8.08ns 0.328 0 0 2.08 KB
master EnrichedLog netcoreapp3.1 4.15μs 4.5ns 16.8ns 0.0226 0 0 1.69 KB
#3102 EnrichedLog net472 4.66μs 0.866ns 3.24ns 0.329 0 0 2.08 KB
#3102 EnrichedLog netcoreapp3.1 4.24μs 2.46ns 9.53ns 0.0211 0 0 1.69 KB
Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #3102

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net472 1.195 829.23 990.64

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net472 830ns 1.5ns 5.8ns 0.104 0 0 658 B
master StartFinishSpan netcoreapp3.1 739ns 0.577ns 2.16ns 0.00846 0 0 648 B
master StartFinishScope net472 1.05μs 1.07ns 4.14ns 0.117 0 0 738 B
master StartFinishScope netcoreapp3.1 938ns 0.544ns 2.11ns 0.0103 0 0 768 B
#3102 StartFinishSpan net472 991ns 1.07ns 4.15ns 0.105 0 0 658 B
#3102 StartFinishSpan netcoreapp3.1 755ns 0.76ns 2.94ns 0.00868 0 0 648 B
#3102 StartFinishScope net472 1.04μs 0.674ns 2.61ns 0.117 0 0 738 B
#3102 StartFinishScope netcoreapp3.1 868ns 0.476ns 1.84ns 0.0105 0 0 768 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 net472 1.21μs 1.26ns 4.7ns 0.117 0 0 738 B
master RunOnMethodBegin netcoreapp3.1 1.01μs 0.959ns 3.59ns 0.0106 0 0 768 B
#3102 RunOnMethodBegin net472 1.22μs 0.889ns 3.44ns 0.117 0 0 738 B
#3102 RunOnMethodBegin netcoreapp3.1 992ns 0.765ns 2.96ns 0.0104 0 0 768 B

@andrewlock

andrewlock commented Aug 20, 2022

Copy link
Copy Markdown
Member

Code Coverage Report 📊

✔️ Merging #3102 into master will not change line coverage
✔️ Merging #3102 into master will not change branch coverage
⛔ Merging #3102 into master will will increase complexity by 4

master #3102 Change
Lines 16873 / 23241 16964 / 23248
Lines % 73% 73% 0% ✔️
Branches 10066 / 14356 10123 / 14360
Branches % 70% 70% 0% ✔️
Complexity 15452 15456 4 ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

master #3102 Change
Lines % 73% 73% 0% ✔️
Branches % 70% 70% 0% ✔️
Complexity 15452 15456 4 ⛔

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.Ci.CIVisibility 5% ✔️ 6% ✔️ 0 ✔️
Datadog.Trace.Ci.GitInfo 5% ✔️ 8% ✔️ 0 ✔️

View the full reports for further details:

zacharycmontoya
zacharycmontoya approved these changes Aug 22, 2022
View reviewed changes

@zacharycmontoya zacharycmontoya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a little surprised that the result.GetReturnValue() satsifies the ref escape analysis, but LGTM. Nice work avoiding new C# 11 keywords

lucaspimentel
lucaspimentel reviewed Aug 23, 2022
View reviewed changes
Comment thread tracer/src/Datadog.Trace/ClrProfiler/CallTarget/CallTargetInvoker.cs
@tonyredondo
tonyredondo merged commit 23bb305 into master Aug 23, 2022
@tonyredondo
tonyredondo deleted the tony/fix-build-in-csharp-11 branch August 23, 2022 17:09
@github-actions github-actions Bot added this to the vNext milestone Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucaspimentel lucaspimentel lucaspimentel approved these changes
@zacharycmontoya zacharycmontoya zacharycmontoya approved these changes

Assignees

@tonyredondo tonyredondo

Labels

area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations)

Projects

None yet

Milestone

2.15.0

Development

Successfully merging this pull request may close these issues.

4 participants

@tonyredondo @andrewlock @lucaspimentel @zacharycmontoya