Ensure that we never run any call target instrumentations in partial trust #6290

andrewlock · 2024-11-14T17:52:53Z

Summary of changes

Update the CallTargetInvoker to explicitly bail-out if partial-trust is detected.

Reason for change

We don't support partial trust. In the native profiler, we have code that explicitly bails out if we're in partial trust. Unfortunately... IIS.

In IIS, we could have two apps running in the same app pool—one in full trust, one in partial trust. The full trust app will be instrumented as normal. Unfortunately, because they're running in the same app pool, the partial trust app is effectively also instrumented.

In this scenario, we'll likely create errors in the partial trust app (we have seen MemberAccessExceptions for example), and we still can't really work there.

Implementation details

The problem is essentially the same as a recent fixed issue (#6147) - multiple apps in the same pool causing oddities with instrumentation. The fix in this PR piggy-backs in the work there, essentially

We check whether the app domain is in partial trust.
If it is, we bail out of running any CallTarget instrumentation.

This should solve the issue for calltarget and for manual instrumentation (which is just calltarget in v3).

The issue remains for callsite instrumentation unfortunately, and likely cannot easily be mitigated @daniel-romano-DD

Test coverage

sigh I spent all day trying to create an aspnet app that I could run in partial trust. All day. I failed. If anyone can produce one that will run in IIS under partial trust, then I have the setup prepared.

Unfortunately, as I can't even pass that low bar, I can't even manually test this 🙁 All I can confirm is that it doesn't affect "full trust" applications that we use in CI 😅

Other details

Seriously, can anyone get a partial trust app running? 🤷‍♂️

…in partial trust

tracer/src/Datadog.Trace/ClrProfiler/CallTarget/CallTargetInvoker.cs

datadog-ddstaging · 2024-11-14T18:19:22Z

Datadog Report

Branch report: andrew/bail-out-in-partial-trust
Commit report: 1059800
Test service: dd-trace-dotnet

✅ 0 Failed, 453466 Passed, 2728 Skipped, 19h 15m 39.33s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

WorkingWithContinuousProfiler - Datadog.Trace.Tools.dd_dotnet.ArtifactTests.Checks.ProcessBasicChecksTests - Last Failure

Expand for error

 
 
 With StandardOutput:
 Running checks on process 3520 Process name: Samples.Console  ---- STARTING TRACER SETUP CHECKS ----- Target process is running with .NET Framework 1. Checking Modules Needed so the Tracer Loads:  [SUCCESS]: The native library version 3.6.0.0 is loaded into the process.  [WARNING]: Found multiple instances of Datadog.Trace.dll in the target process. Detected versions: 3.6.0.0  2. Checking DD_DOTNET_TRACER_HOME and related configuration value:  [SUCCESS]: DD_DOTNET_TRACER_HOME is set to 'D:\a\1\s\shared\bin\monitoring-home' and the directory was found correctly. 3. Checking COR_PROFILER_PATH and related configuration value:  [SUCCESS]: The environment variable COR_PROFILER_PATH is set to the correct value of D:\a\1\s\shared\bin\monitoring-home\win-x64\Datadog.Trace.ClrProfiler.Native.dll . 4. Checking COR_PROFILER and related configuration value:  [SUCCESS]: The environment variable COR_PROFILER is set to the correct value of {846F5F1C-F9AE-4B07-969E-05C26BC060D8}. 5. Checking COR_ENABLE_PROFILING and related configuration value:  [SUCCESS]: The environment variable COR_ENABLE_PROFILING is set to the correct value of 1.  ---- CONFIGURATION CHECKS ----- 1. Checking if tracing is disabled using DD_TRACE_ENABLED.  [INFO]: DD_TRACE_ENABLED is not set, the default value is true. 2. Checking if profiling is enabled using DD_PROFILING_ENABLED.  [SUCCESS]: DD_PROFILING_ENABLED is set.  ---- DATADOG AGENT CHECKS ----- Detected agent url: http://127.0.0.1:52779/. Note: this url may be incorrect if you configured the application through a configuration file. Connecting to Agent at endpoint http://127.0.0.1:52779/ using HTTP  [WARNING]: Could not detect the agent version. It may be running with a version older than 7.27.0.  [SUCCESS]: No issue found with the target process. 
 With ErrorOutput:
 
 With ExitCode:
 0acer version 3.6.0.0 is loaded into the process."}.

andrewlock · 2024-11-14T18:22:09Z

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 (6290) - mean (73ms)  : 63, 82
     .   : milestone, 73,
    master - mean (71ms)  : 65, 77
     .   : milestone, 71,

    section CallTarget+Inlining+NGEN
    This PR (6290) - mean (1,112ms)  : 1093, 1131
     .   : milestone, 1112,
    master - mean (1,106ms)  : 1082, 1131
     .   : milestone, 1106,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6290) - mean (109ms)  : 106, 112
     .   : milestone, 109,
    master - mean (108ms)  : 106, 110
     .   : milestone, 108,

    section CallTarget+Inlining+NGEN
    This PR (6290) - mean (770ms)  : 755, 786
     .   : milestone, 770,
    master - mean (773ms)  : 759, 788
     .   : milestone, 773,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6290) - mean (93ms)  : 91, 94
     .   : milestone, 93,
    master - mean (93ms)  : 89, 97
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (6290) - mean (729ms)  : 711, 747
     .   : milestone, 729,
    master - mean (728ms)  : 710, 746
     .   : milestone, 728,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6290) - mean (191ms)  : 186, 196
     .   : milestone, 191,
    master - mean (190ms)  : 187, 194
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6290) - mean (1,211ms)  : 1189, 1233
     .   : milestone, 1211,
    master - mean (1,227ms)  : 1201, 1254
     .   : milestone, 1227,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6290) - mean (276ms)  : 271, 280
     .   : milestone, 276,
    master - mean (276ms)  : 270, 282
     .   : milestone, 276,

    section CallTarget+Inlining+NGEN
    This PR (6290) - mean (948ms)  : 933, 963
     .   : milestone, 948,
    master - mean (945ms)  : 919, 971
     .   : milestone, 945,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6290) - mean (265ms)  : 262, 269
     .   : milestone, 265,
    master - mean (265ms)  : 260, 271
     .   : milestone, 265,

    section CallTarget+Inlining+NGEN
    This PR (6290) - mean (926ms)  : 910, 941
     .   : milestone, 926,
    master - mean (929ms)  : 914, 945
     .   : milestone, 929,

andrewlock · 2024-11-14T22:38:51Z

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 (6290) (10.940M)   : 0, 10940226
    master (11.248M)   : 0, 11247687
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6290) (7.243M)   : 0, 7242726
    master (7.236M)   : 0, 7235877
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.529M)   : 0, 7529290

    section Manual
    master (11.094M)   : 0, 11093560

    section Manual + Automatic
    This PR (6290) (6.723M)   : 0, 6723439
    master (6.699M)   : 0, 6698725

    section DD_TRACE_ENABLED=0
    master (10.232M)   : 0, 10231991

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6290) (9.570M)   : 0, 9569733
    master (9.599M)   : 0, 9599416
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6290) (6.397M)   : 0, 6396518
    master (6.401M)   : 0, 6400631

    section Trace stats
    master (6.833M)   : 0, 6832952

    section Manual
    master (9.648M)   : 0, 9648079

    section Manual + Automatic
    This PR (6290) (5.972M)   : 0, 5972161
    master (6.042M)   : 0, 6041665

    section DD_TRACE_ENABLED=0
    master (8.938M)   : 0, 8938482

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6290) (9.518M)   : 0, 9517883
    master (9.703M)   : 0, 9703135
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6290) (6.366M)   : 0, 6365650
    master (6.287M)   : 0, 6286829
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (6.870M)   : 0, 6869636

    section Manual
    master (9.395M)   : 0, 9394829

    section Manual + Automatic
    This PR (6290) (5.764M)   : 0, 5763566
    master (5.848M)   : 0, 5848196

    section DD_TRACE_ENABLED=0
    master (8.825M)   : 0, 8824661

Ensure that we never run any call target instrumentations when we're …

1059800

…in partial trust

andrewlock added area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) identified-by:telemetry labels Nov 14, 2024

andrewlock requested a review from a team as a code owner November 14, 2024 17:52

tonyredondo reviewed Nov 14, 2024

View reviewed changes

tracer/src/Datadog.Trace/ClrProfiler/CallTarget/CallTargetInvoker.cs Show resolved Hide resolved

tonyredondo approved these changes Nov 15, 2024

View reviewed changes

kevingosse approved these changes Nov 15, 2024

View reviewed changes

andrewlock merged commit 95b2215 into master Nov 15, 2024
74 of 77 checks passed

andrewlock deleted the andrew/bail-out-in-partial-trust branch November 15, 2024 16:46

github-actions bot added this to the vNext-v3 milestone Nov 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure that we never run any call target instrumentations in partial trust #6290

Ensure that we never run any call target instrumentations in partial trust #6290

andrewlock commented Nov 14, 2024

datadog-ddstaging bot commented Nov 14, 2024 •

edited

Loading

andrewlock commented Nov 14, 2024

andrewlock commented Nov 14, 2024

Ensure that we never run any call target instrumentations in partial trust #6290

Ensure that we never run any call target instrumentations in partial trust #6290

Conversation

andrewlock commented Nov 14, 2024

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

datadog-ddstaging bot commented Nov 14, 2024 • edited Loading

Datadog Report

New Flaky Tests (1)

andrewlock commented Nov 14, 2024

Execution-Time Benchmarks Report ⏱️

andrewlock commented Nov 14, 2024

Throughput/Crank Report ⚡

datadog-ddstaging bot commented Nov 14, 2024 •

edited

Loading