Skip to content
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

Revert "Load the tracer/profiler after guardrails checks" #6200

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Revert "Load the tracer/profiler after guardrails checks" #6200

merged 1 commit into from
Oct 25, 2024

Conversation

kevingosse
Copy link
Collaborator

@kevingosse kevingosse commented Oct 25, 2024 •
edited
Loading

Reverts #5968

It seems it causes a crash when .NET Framework and .NET Core are loaded in the same process.

@kevingosse kevingosse requested a review from a team as a code owner October 25, 2024 08:31
@kevingosse kevingosse added area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native) type:bug labels Oct 25, 2024
@andrewlock
Copy link
Member

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 (6200) - mean (70ms)  : 68, 72
     .   : milestone, 70,
    master - mean (70ms)  : 68, 73
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6200) - mean (1,114ms)  : 1093, 1136
     .   : milestone, 1114,
    master - mean (1,114ms)  : 1092, 1135
     .   : milestone, 1114,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6200) - mean (110ms)  : 106, 113
     .   : milestone, 110,
    master - mean (109ms)  : 106, 113
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (6200) - mean (776ms)  : 754, 798
     .   : milestone, 776,
    master - mean (772ms)  : 755, 788
     .   : milestone, 772,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6200) - mean (93ms)  : 90, 96
     .   : milestone, 93,
    master - mean (93ms)  : 90, 96
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (6200) - mean (732ms)  : 713, 750
     .   : milestone, 732,
    master - mean (726ms)  : 709, 744
     .   : milestone, 726,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6200) - mean (191ms)  : 187, 196
     .   : milestone, 191,
    master - mean (190ms)  : 186, 193
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6200) - mean (1,204ms)  : 1179, 1229
     .   : milestone, 1204,
    master - mean (1,200ms)  : 1169, 1231
     .   : milestone, 1200,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6200) - mean (276ms)  : 272, 280
     .   : milestone, 276,
    master - mean (277ms)  : 272, 283
     .   : milestone, 277,

    section CallTarget+Inlining+NGEN
    This PR (6200) - mean (946ms)  : 929, 964
     .   : milestone, 946,
    master - mean (942ms)  : 922, 962
     .   : milestone, 942,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6200) - mean (264ms)  : 261, 267
     .   : milestone, 264,
    master - mean (265ms)  : 261, 268
     .   : milestone, 265,

    section CallTarget+Inlining+NGEN
    This PR (6200) - mean (929ms)  : 908, 951
     .   : milestone, 929,
    master - mean (927ms)  : 907, 947
     .   : milestone, 927,
Loading

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Oct 25, 2024 •
edited
Loading

Datadog Report

Branch report: revert-5968-kevin/better_loader
Commit report: 7264f85
Test service: dd-trace-dotnet

❌ 3 Failed (0 Known Flaky), 364415 Passed, 2391 Skipped, 17h 26m 28.91s Total Time
❄️ 1 New Flaky

❌ Failed Tests (3)

  • AzureFunctionsTests+InProcessRuntimeV4.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error 
     The sample did not exit in 600000ms. Memory dump taken: True. Killing process.

  • AzureFunctionsTests+IsolatedRuntimeV4.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error 
     Expected exit code: -1, actual exit code: 1.

  • AzureFunctionsTests+IsolatedRuntimeV4.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error 
     The sample did not exit in 600000ms. Memory dump taken: True. Killing process.

New Flaky Tests (1)

  • HttpClient_SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests.HttpMessageHandlerTests - Last Failure

    Expand for error 
     
 Expected var integration = latestIntegrations {empty} to contain key "CurlHandler".), but no such item was found.t found False.

@andrewlock
Copy link
Member

Throughput/Crank Report ⚡

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6200) (11.144M)   : 0, 11143816
    master (11.222M)   : 0, 11221830
    benchmarks/2.9.0 (11.081M)   : 0, 11080577

    section Automatic
    This PR (6200) (7.286M)   : 0, 7285673
    master (7.223M)   : 0, 7222557
    benchmarks/2.9.0 (7.732M)   : 0, 7732233

    section Trace stats
    master (7.605M)   : 0, 7605147

    section Manual
    master (11.118M)   : 0, 11118437

    section Manual + Automatic
    This PR (6200) (6.778M)   : 0, 6778433
    master (6.667M)   : 0, 6667462

    section DD_TRACE_ENABLED=0
    master (10.270M)   : 0, 10270323
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6200) (9.645M)   : 0, 9645449
    master (9.681M)   : 0, 9680676
    benchmarks/2.9.0 (9.798M)   : 0, 9798067

    section Automatic
    This PR (6200) (6.548M)   : 0, 6547977
    master (6.497M)   : 0, 6497461

    section Trace stats
    master (6.874M)   : 0, 6874362

    section Manual
    master (9.547M)   : 0, 9547427

    section Manual + Automatic
    This PR (6200) (6.069M)   : 0, 6068865
    master (6.064M)   : 0, 6063734

    section DD_TRACE_ENABLED=0
    master (8.989M)   : 0, 8989239
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6200) (9.886M)   : 0, 9886451
    master (10.046M)   : 0, 10046157
    benchmarks/2.9.0 (10.067M)   : 0, 10067315

    section Automatic
    This PR (6200) (6.562M)   : 0, 6561513
    master (6.439M)   : 0, 6439338
    benchmarks/2.9.0 (7.552M)   : 0, 7552193

    section Trace stats
    master (7.286M)   : 0, 7285853

    section Manual
    master (9.860M)   : 0, 9859672

    section Manual + Automatic
    This PR (6200) (6.204M)   : 0, 6204139
    master (5.916M)   : 0, 5915686

    section DD_TRACE_ENABLED=0
    master (9.362M)   : 0, 9361540
Loading

@kevingosse kevingosse merged commit 4d9df8b into master Oct 25, 2024
80 of 83 checks passed
@kevingosse kevingosse deleted the revert-5968-kevin/better_loader branch October 25, 2024 12:38
@github-actions github-actions bot added this to the vNext-v3 milestone Oct 25, 2024
andrewlock pushed a commit that referenced this pull request Oct 25, 2024
@kevingosse @andrewlock
Revert "Load the tracer/profiler after guardrails checks" (#6200)
f6a1f76 
Reverts #5968

It seems it causes a crash when .NET Framework and .NET Core are loaded
in the same process.
@andrewlock andrewlock mentioned this pull request Oct 25, 2024
Merged
andrewlock added a commit that referenced this pull request Oct 25, 2024
@andrewlock @kevingosse
Revert "Load the tracer/profiler after guardrails checks" (#6200) (#6205
d1cda49 
)

Reverts #5968

It seems it causes a crash when .NET Framework and .NET Core are loaded
in the same process.

Cherry pick of #6200

Co-authored-by: Kevin Gosse <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewlock andrewlock andrewlock approved these changes

Assignees
No one assigned
Labels
area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native) type:bug
Projects
None yet
Milestone
3.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@kevingosse @andrewlock