Skip to content

Adds a workaround for the dotnet runtime issue 77973#3506

Merged
tonyredondo merged 9 commits into
masterfrom
tony/dotnet-runtime-77973-workaround
Dec 13, 2022
Merged

Adds a workaround for the dotnet runtime issue 77973#3506
tonyredondo merged 9 commits into
masterfrom
tony/dotnet-runtime-77973-workaround

Conversation

@tonyredondo
Copy link
Copy Markdown
Member

@tonyredondo tonyredondo commented Nov 24, 2022 •
edited
Loading

Summary of changes

This PR adds a workaround for dotnet/runtime#77973

Implementation details

The PR disables by default Tiered Compilation for the following cases:

  • Runtime version is .NET 5
  • Runtime version is >= 6.0.0 && <= 6.0.12 (current version)
  • Runtime version is <= 7.0.1

Microsoft confirmed the inclussion of a fix for the next service release for .NET 6 and .NET 7

Note: The workaround can be disabled by setting the environment variable: DD_INTERNAL_WORKAROUND_77973_ENABLED=false

@tonyredondo tonyredondo self-assigned this Nov 24, 2022
@tonyredondo tonyredondo added the area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native) label Nov 24, 2022
andrewlock
andrewlock approved these changes Nov 24, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

LGTM, thanks 👍

Comment thread tracer/src/Datadog.Tracer.Native/cor_profiler.cpp Outdated
@tonyredondo tonyredondo marked this pull request as ready for review November 24, 2022 13:11
@tonyredondo tonyredondo requested a review from a team as a code owner November 24, 2022 13:11
OmerRaviv
OmerRaviv approved these changes Nov 24, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@OmerRaviv OmerRaviv left a comment

Choose a reason for hiding this comment

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

LGTM, but shouldn't we also revert #3479 in the same PR?

@tonyredondo tonyredondo requested review from a team as code owners November 24, 2022 13:17
@tonyredondo
Copy link
Copy Markdown
Member Author

tonyredondo commented Nov 24, 2022

LGTM, but shouldn't we also revert #3479 in the same PR?

Is included in the PR.

@datadog-ddstaging
Copy link
Copy Markdown

datadog-ddstaging Bot commented Nov 24, 2022 •
edited
Loading

Datadog Report

Branch report: tony/dotnet-runtime-77973-workaround
Commit report: 31716fe

❌ dd-trace-dotnet 2 Failed (2 Known Flaky), 0 New Flaky, 274884 Passed, 1524 Skipped, 23m 17.04s Wall Time

❌ Failed Tests (2)

  • TestSecurityInitialization - Datadog.Trace.Security.IntegrationTests.AspNetCore5AsmInitialization - ❄️ Known flaky - Details

    Expand for error 
     Results do not match.
 Differences:
 Received: Security.AspNetCore5AsmInitialization.__enableSecurity=True_expectedStatusCode=200_ruleset=wrong-tags-name-rule-set.json.received.txt
 Verified: Security.AspNetCore5AsmInitialization.__enableSecurity=True_expectedStatusCode=200_ruleset=wrong-tags-name-rule-set.json.verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

  • TestSecurity - Datadog.Trace.Security.IntegrationTests.AspNetCoreBare - ❄️ Known flaky - Details

    Expand for error 
     Results do not match.
 Differences:
 Received: Security.AspNetCoreBare.__expectedStatusCode=200_url=_void-param=[$slice].received.txt
 Verified: Security.AspNetCoreBare.__expectedStatusCode=200_url=_void-param=[$slice].verified.txt
 Received Content:
 [
   {
     TraceId: Id_1,
     SpanId: Id_2,
     Name: aspnet_core.request,
 ...

@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

This comment has been minimized.

Sign in to view
@datadog-ddstaging
Copy link
Copy Markdown

datadog-ddstaging Bot commented Dec 12, 2022 •
edited
Loading

Datadog Report

Branch report: tony/dotnet-runtime-77973-workaround
Commit report: 874d40b

✅ dd-trace-dotnet 0 Failed, 0 New Flaky, 225672 Passed, 862 Skipped, 19m 38.67s Wall Time

@tonyredondo
Copy link
Copy Markdown
Member Author

tonyredondo commented Dec 12, 2022

The PR was update with the versions confirmed by the dotnet team.

.NET 6: dotnet/runtime#78670 (comment)
.NET 7: dotnet/runtime#78669 (comment)

@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock
Copy link
Copy Markdown
Member

andrewlock commented Dec 12, 2022

Benchmarks Report 🐌

Benchmarks for #3506 compared to master:

  • All benchmarks have the same speed
  • 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 765μs 804ns 3.12μs 0.377 0 0 3.22 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 551μs 150ns 561ns 0 0 0 2.63 KB
#3506 WriteAndFlushEnrichedTraces net472 763μs 446ns 1.67μs 0.377 0 0 3.22 KB
#3506 WriteAndFlushEnrichedTraces netcoreapp3.1 545μs 170ns 614ns 0 0 0 2.63 KB
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net472 25.2μs 20.2ns 75.6ns 0.273 0 0 1.77 KB
master AllCycleSimpleBody netcoreapp3.1 23.7μs 136ns 1.04μs 0.0216 0 0 1.64 KB
master AllCycleMoreComplexBody net472 194μs 96.6ns 361ns 2.04 0 0 13.02 KB
master AllCycleMoreComplexBody netcoreapp3.1 186μs 123ns 459ns 0.0929 0 0 12.1 KB
master BodyExtractorSimpleBody net472 292ns 0.285ns 1.1ns 0.0573 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 240ns 0.0861ns 0.31ns 0.00375 0 0 272 B
master BodyExtractorMoreComplexBody net472 16.3μs 18.9ns 73.3ns 1.21 0.0163 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 12.9μs 6.25ns 23.4ns 0.0901 0 0 6.75 KB
#3506 AllCycleSimpleBody net472 25.2μs 138ns 874ns 0.271 0 0 1.77 KB
#3506 AllCycleSimpleBody netcoreapp3.1 23.5μs 131ns 840ns 0.012 0 0 1.64 KB
#3506 AllCycleMoreComplexBody net472 193μs 263ns 985ns 2.07 0 0 13.02 KB
#3506 AllCycleMoreComplexBody netcoreapp3.1 183μs 813ns 3.15μs 0.0923 0 0 12.1 KB
#3506 BodyExtractorSimpleBody net472 280ns 0.145ns 0.543ns 0.0573 0 0 361 B
#3506 BodyExtractorSimpleBody netcoreapp3.1 238ns 0.153ns 0.572ns 0.00371 0 0 272 B
#3506 BodyExtractorMoreComplexBody net472 15.9μs 14ns 54.3ns 1.2 0.0159 0 7.62 KB
#3506 BodyExtractorMoreComplexBody netcoreapp3.1 13.1μs 45.3ns 170ns 0.0912 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 0ns 0ns 0ns 0 0 0 0 b
master SendRequest netcoreapp3.1 177μs 193ns 749ns 0.267 0 0 20.39 KB
#3506 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3506 SendRequest netcoreapp3.1 176μs 96.1ns 372ns 0.264 0 0 20.39 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.78μs 1.38ns 5.18ns 0.161 0.000887 0 1.01 KB
master ExecuteNonQuery netcoreapp3.1 1.44μs 0.521ns 2.02ns 0.0138 0 0 1 KB
#3506 ExecuteNonQuery net472 1.84μs 2.59ns 10ns 0.16 0.000921 0 1.01 KB
#3506 ExecuteNonQuery netcoreapp3.1 1.43μs 0.437ns 1.69ns 0.0135 0 0 1 KB
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.57μs 1.08ns 4.05ns 0.194 0 0 1.22 KB
master CallElasticsearch netcoreapp3.1 1.51μs 0.496ns 1.92ns 0.0151 0 0 1.16 KB
master CallElasticsearchAsync net472 2.67μs 0.71ns 2.75ns 0.214 0 0 1.36 KB
master CallElasticsearchAsync netcoreapp3.1 1.68μs 0.508ns 1.83ns 0.0175 0 0 1.28 KB
#3506 CallElasticsearch net472 2.42μs 1.14ns 4.42ns 0.193 0 0 1.22 KB
#3506 CallElasticsearch netcoreapp3.1 1.48μs 0.395ns 1.48ns 0.0155 0 0 1.16 KB
#3506 CallElasticsearchAsync net472 2.67μs 0.751ns 2.91ns 0.215 0 0 1.36 KB
#3506 CallElasticsearchAsync netcoreapp3.1 1.59μs 2.11ns 7.9ns 0.0174 0 0 1.28 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.74μs 0.974ns 3.64ns 0.235 0 0 1.49 KB
master ExecuteAsync netcoreapp3.1 1.72μs 0.634ns 2.29ns 0.0189 0 0 1.41 KB
#3506 ExecuteAsync net472 2.69μs 3.44ns 13.3ns 0.235 0 0 1.49 KB
#3506 ExecuteAsync netcoreapp3.1 1.74μs 0.631ns 2.44ns 0.0193 0 0 1.41 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 5.74μs 1.12ns 4.2ns 0.449 0 0 2.83 KB
master SendAsync netcoreapp3.1 3.68μs 15.8ns 61.2ns 0.036 0 0 2.66 KB
#3506 SendAsync net472 5.72μs 2.14ns 8.01ns 0.45 0 0 2.83 KB
#3506 SendAsync netcoreapp3.1 3.67μs 6.1ns 23.6ns 0.0349 0 0 2.66 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.72μs 1.54ns 5.98ns 0.298 0 0 1.88 KB
master EnrichedLog netcoreapp3.1 2.31μs 0.869ns 3.25ns 0.0253 0 0 1.91 KB
#3506 EnrichedLog net472 2.92μs 4.12ns 15.4ns 0.297 0 0 1.88 KB
#3506 EnrichedLog netcoreapp3.1 2.41μs 1.46ns 5.64ns 0.0254 0 0 1.91 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 86.5ns 335ns 0.749 0.225 0 4.72 KB
master EnrichedLog netcoreapp3.1 121μs 302ns 1.09μs 0.0599 0 0 4.55 KB
#3506 EnrichedLog net472 151μs 188ns 729ns 0.677 0.226 0 4.72 KB
#3506 EnrichedLog netcoreapp3.1 121μs 289ns 1.12μs 0.0603 0 0 4.55 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.28μs 0.906ns 3.39ns 0.58 0.00265 0 3.65 KB
master EnrichedLog netcoreapp3.1 4.25μs 2.01ns 7.53ns 0.053 0 0 3.98 KB
#3506 EnrichedLog net472 5.42μs 1.73ns 6.48ns 0.578 0.00271 0 3.65 KB
#3506 EnrichedLog netcoreapp3.1 4.37μs 3.04ns 11.8ns 0.0547 0 0 3.98 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.14μs 1.26ns 4.71ns 0.228 0 0 1.44 KB
master SendReceive netcoreapp3.1 1.79μs 0.647ns 2.42ns 0.0187 0 0 1.38 KB
#3506 SendReceive net472 2.17μs 2.44ns 9.45ns 0.228 0 0 1.44 KB
#3506 SendReceive netcoreapp3.1 1.81μs 0.667ns 2.5ns 0.019 0 0 1.38 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.82μs 1.45ns 5.42ns 0.364 0 0 2.3 KB
master EnrichedLog netcoreapp3.1 3.9μs 4.15ns 15.5ns 0.0253 0 0 1.86 KB
#3506 EnrichedLog net472 4.84μs 1.58ns 6.11ns 0.363 0 0 2.3 KB
#3506 EnrichedLog netcoreapp3.1 4μs 1.08ns 4.03ns 0.0241 0 0 1.86 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net472 1.11μs 0.297ns 1.15ns 0.139 0 0 875 B
master StartFinishSpan netcoreapp3.1 964ns 0.381ns 1.42ns 0.0111 0 0 824 B
master StartFinishScope net472 1.39μs 0.903ns 3.5ns 0.151 0 0 955 B
master StartFinishScope netcoreapp3.1 1.06μs 0.544ns 2.04ns 0.0127 0 0 944 B
#3506 StartFinishSpan net472 1.13μs 0.412ns 1.59ns 0.139 0 0 875 B
#3506 StartFinishSpan netcoreapp3.1 902ns 0.795ns 3.08ns 0.0112 0 0 824 B
#3506 StartFinishScope net472 1.39μs 1.11ns 4.29ns 0.151 0 0 955 B
#3506 StartFinishScope netcoreapp3.1 1.01μs 0.956ns 3.7ns 0.0126 0 0 944 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.39μs 0.358ns 1.34ns 0.151 0 0 955 B
master RunOnMethodBegin netcoreapp3.1 1.19μs 0.651ns 2.35ns 0.0128 0 0 944 B
#3506 RunOnMethodBegin net472 1.46μs 0.567ns 2.19ns 0.151 0 0 955 B
#3506 RunOnMethodBegin netcoreapp3.1 1.17μs 0.373ns 1.45ns 0.0128 0 0 944 B

@andrewlock
Copy link
Copy Markdown
Member

andrewlock commented Dec 12, 2022

Code Coverage Report 📊

✔️ Merging #3506 into master will not change line coverage
✔️ Merging #3506 into master will not change branch coverage
✔️ Merging #3506 into master will not change complexity

master #3506 Change
Lines 20114 / 28304 20221 / 28304
Lines % 71% 71% 0% ✔️
Branches 12029 / 18069 12088 / 18069
Branches % 67% 67% 0% ✔️
Complexity 19496 19496 0 ✔️

View the full report for further details:

Datadog.Trace Breakdown ✔️

master #3506 Change
Lines % 71% 71% 0% ✔️
Branches % 67% 67% 0% ✔️
Complexity 19496 19496 0 ✔️

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.Debugger.Configurations.Models.SnapshotProbe 8% ✔️ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Configurations.Models.Where 8% ✔️ 0% ✔️ 0 ✔️
Datadog.Trace.Ci.GitInfo 11% ✔️ 8% ✔️ 0 ✔️
Datadog.Trace.Pdb.DatadogPdbReader 20% ✔️ 38% ✔️ 0 ✔️
Datadog.Trace.Debugger.LineProbeResolver 81% ✔️ 82% ✔️ 0 ✔️
BoundLineProbeLocation 100% ✔️ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Models.LineProbeResolveResult 100% ✔️ 0% ✔️ 0 ✔️

View the full reports for further details:

@tonyredondo tonyredondo merged commit 8491d25 into master Dec 13, 2022
@tonyredondo tonyredondo deleted the tony/dotnet-runtime-77973-workaround branch December 13, 2022 06:13
@github-actions github-actions Bot added this to the vNext milestone Dec 13, 2022
@andrewlock andrewlock mentioned this pull request Dec 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@OmerRaviv OmerRaviv OmerRaviv approved these changes

@lucaspimentel lucaspimentel lucaspimentel approved these changes

@zacharycmontoya zacharycmontoya zacharycmontoya approved these changes

@andrewlock andrewlock andrewlock approved these changes

Assignees

@tonyredondo tonyredondo

Labels

area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native)

Projects

None yet

Milestone

2.21.0

Development

Successfully merging this pull request may close these issues.

5 participants

@tonyredondo @andrewlock @OmerRaviv @lucaspimentel @zacharycmontoya