Skip to content

Update the ProfilerSettings enablement to ignore SSI#7557

Merged
andrewlock merged 2 commits into
masterfrom
andrew/fix-profiler-enablement
Sep 25, 2025
Merged

Update the ProfilerSettings enablement to ignore SSI#7557
andrewlock merged 2 commits into
masterfrom
andrew/fix-profiler-enablement

Conversation

@andrewlock

@andrewlock andrewlock commented Sep 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Summary of changes

Updates the rules around ProfilerSettings enablement

Reason for change

After #7287, we decide whether the profiler is enabled in managed code, instead of on the native side. However, we got some of the rules wrong. Specifically, we believed that the profiler should behave different when DD_INJECTION_ENABLED was set, but that's not the case. The profiler should behave the same in all cases.

Implementation details

Fixed the implementation to ignore the SSI variable, and updated the corresponding unit tests

Test coverage

Updated the tests that were affected

Other details

The incorrect behaviour was actually introduced in #7303, but it didn't have a visible impact until #7287

szegedi
szegedi approved these changes Sep 24, 2025
View reviewed changes
Comment thread tracer/test/Datadog.Trace.Tests/ContinuousProfiler/ProfilerSettingsTests.cs
Comment on lines +20 to +21
// historically, we set a different value for profiling when we are in SSI. That is no longer
// the case, but we keep the test here to ensure that remains the case!

@szegedi szegedi Sep 24, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the way :-)

@datadog-datadog-prod-us1

This comment has been minimized.

Sign in to view
@dd-trace-dotnet-ci-bot

dd-trace-dotnet-ci-bot Bot commented Sep 24, 2025

Copy link
Copy Markdown

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.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7557) - mean (72ms)  : 71, 73
     .   : milestone, 72,
    master - mean (72ms)  : 71, 73
     .   : milestone, 72,

    section Baseline
    This PR (7557) - mean (68ms)  : 66, 70
     .   : milestone, 68,
    master - mean (68ms)  : 66, 70
     .   : milestone, 68,

    section CallTarget+Inlining+NGEN
    This PR (7557) - mean (1,048ms)  : 992, 1104
     .   : milestone, 1048,
    master - mean (1,049ms)  : 988, 1110
     .   : milestone, 1049,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7557) - mean (106ms)  : 105, 108
     .   : milestone, 106,
    master - mean (106ms)  : 105, 108
     .   : milestone, 106,

    section Baseline
    This PR (7557) - mean (106ms)  : 104, 107
     .   : milestone, 106,
    master - mean (105ms)  : 103, 108
     .   : milestone, 105,

    section CallTarget+Inlining+NGEN
    This PR (7557) - mean (738ms)  : 714, 761
     .   : milestone, 738,
    master - mean (745ms)  : 716, 774
     .   : milestone, 745,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7557) - mean (94ms)  : 93, 95
     .   : milestone, 94,
    master - mean (94ms)  : 93, 95
     .   : milestone, 94,

    section Baseline
    This PR (7557) - mean (94ms)  : 91, 96
     .   : milestone, 94,
    master - mean (93ms)  : 91, 95
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (7557) - mean (702ms)  : 683, 722
     .   : milestone, 702,
    master - mean (706ms)  : 677, 735
     .   : milestone, 706,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7557) - mean (93ms)  : 91, 94
     .   : milestone, 93,
    master - mean (92ms)  : 91, 93
     .   : milestone, 92,

    section Baseline
    This PR (7557) - mean (92ms)  : 90, 94
     .   : milestone, 92,
    master - mean (92ms)  : 90, 94
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (7557) - mean (661ms)  : 643, 679
     .   : milestone, 661,
    master - mean (664ms)  : 647, 680
     .   : milestone, 664,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7557) - mean (208ms)  : 202, 215
     .   : milestone, 208,
    master - mean (199ms)  : 197, 202
     .   : milestone, 199,

    section Baseline
    This PR (7557) - mean (202ms)  : 189, 216
     .   : milestone, 202,
    master - mean (196ms)  : 193, 199
     .   : milestone, 196,

    section CallTarget+Inlining+NGEN
    This PR (7557) - mean (1,221ms)  : 1147, 1295
     .   : milestone, 1221,
    master - mean (1,170ms)  : 1101, 1240
     .   : milestone, 1170,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7557) - mean (301ms)  : crit, 292, 310
     .   : crit, milestone, 301,
    master - mean (282ms)  : 275, 289
     .   : milestone, 282,

    section Baseline
    This PR (7557) - mean (296ms)  : 285, 307
     .   : milestone, 296,
    master - mean (281ms)  : 276, 286
     .   : milestone, 281,

    section CallTarget+Inlining+NGEN
    This PR (7557) - mean (973ms)  : 926, 1020
     .   : milestone, 973,
    master - mean (945ms)  : 900, 989
     .   : milestone, 945,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7557) - mean (281ms)  : 274, 288
     .   : milestone, 281,
    master - mean (275ms)  : 269, 281
     .   : milestone, 275,

    section Baseline
    This PR (7557) - mean (281ms)  : 271, 291
     .   : milestone, 281,
    master - mean (275ms)  : 270, 279
     .   : milestone, 275,

    section CallTarget+Inlining+NGEN
    This PR (7557) - mean (953ms)  : 914, 991
     .   : milestone, 953,
    master - mean (930ms)  : 895, 965
     .   : milestone, 930,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7557) - mean (280ms)  : 271, 289
     .   : milestone, 280,
    master - mean (275ms)  : 265, 286
     .   : milestone, 275,

    section Baseline
    This PR (7557) - mean (279ms)  : 268, 290
     .   : milestone, 279,
    master - mean (273ms)  : 266, 281
     .   : milestone, 273,

    section CallTarget+Inlining+NGEN
    This PR (7557) - mean (873ms)  : 837, 909
     .   : milestone, 873,
    master - mean (861ms)  : 840, 883
     .   : milestone, 861,
Loading

@andrewlock andrewlock added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) area:profiler Issues related to the continous-profiler labels Sep 24, 2025
@andrewlock
andrewlock marked this pull request as ready for review September 24, 2025 16:21
@andrewlock
andrewlock requested review from a team as code owners September 24, 2025 16:21
chrisnas
chrisnas approved these changes Sep 25, 2025
View reviewed changes

@chrisnas chrisnas 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.

LGTM

@andrewlock
andrewlock merged commit 1428251 into master Sep 25, 2025
161 checks passed
@andrewlock
andrewlock deleted the andrew/fix-profiler-enablement branch September 25, 2025 11:48
@github-actions github-actions Bot added this to the vNext-v3 milestone Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szegedi szegedi szegedi approved these changes
@anna-git anna-git anna-git approved these changes
@chrisnas chrisnas chrisnas approved these changes

Assignees

No one assigned

Labels

area:profiler Issues related to the continous-profiler area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations)

Projects

None yet

Milestone

3.28.0

Development

Successfully merging this pull request may close these issues.

4 participants

@andrewlock @szegedi @anna-git @chrisnas