Simplify profiler sampling overhead attribution with a placeholder stack by eregon · Pull Request #5870 · DataDog/dd-trace-rb

eregon · 2026-06-08T16:52:05Z

Replace the confusing profiler_overhead_stack_thread parameter (which borrowed a real stack from the worker's owner thread) with a synthetic "Datadog::Profiling::Collectors::Stack" synthesized frame, matching the pattern used by GC and other synthesized stacks. This removes the stack_from_thread parameter and passes only one thread through the entire sampling call chain.

What does this PR do?
^

Motivation:
Simplicity

Change log entry

None

Additional Notes:

How to test the change?

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c453716ae9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

datadog-prod-us1-3 · 2026-06-08T16:54:34Z

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 90.08% (+0.02%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 41bcd1f | Docs | Datadog PR Page | Give us feedback!}

pr-commenter · 2026-06-08T17:20:29Z

Benchmarks

Benchmark execution time: 2026-06-09 13:49:59

Comparing candidate commit 41bcd1f in PR branch simplify-profiler_overhead_stack_thread with baseline commit 5c9136e in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 44 metrics, 1 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

🟩 = significantly better candidate vs. baseline
🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:profiling - sample and serialize

🟩 throughput [+0.372op/s; +0.377op/s] or [+8.923%; +9.054%]

ivoanjo

This is looking really good! Left a few notes :)

ivoanjo

👍 LGTM

Replace the confusing profiler_overhead_stack_thread parameter (which borrowed a real stack from the worker's owner thread) with a synthetic "Datadog::Profiling::Collectors::Stack" placeholder frame, matching the pattern used by GC and other placeholder stacks. This removes the stack_from_thread parameter and passes only one thread through the entire sampling call chain. Co-Authored-By: Claude Opus 4.6 <[email protected]>

dd-octo-sts · 2026-06-09T13:23:55Z

Typing analysis

Note: Ignored files are excluded from the next sections.

Untyped methods

This PR introduces 2 partially typed methods, and clears 2 partially typed methods.

Partially typed methods (+2-2)

❌ Introduced:

sig/datadog/profiling/collectors/thread_context.rbs:27
└── def self.for_testing: (
          recorder: Datadog::Profiling::StackRecorder,
          ?max_frames: ::Integer,
          ?tracer: Datadog::Tracing::Tracer?,
          ?endpoint_collection_enabled: bool,
          ?waiting_for_gvl_threshold_ns: ::Integer,
          ?otel_context_enabled: (::Symbol? | bool),
          ?native_filenames_enabled: bool,
          **untyped
        ) -> Datadog::Profiling::Collectors::ThreadContext
sig/datadog/profiling/collectors/thread_context.rbs:48
└── def safely_extract_context_key_from: (untyped tracer) -> ::Symbol?

✅ Cleared:

sig/datadog/profiling/collectors/thread_context.rbs:26
└── def self.for_testing: (
          recorder: Datadog::Profiling::StackRecorder,
          ?max_frames: ::Integer,
          ?tracer: Datadog::Tracing::Tracer?,
          ?endpoint_collection_enabled: bool,
          ?waiting_for_gvl_threshold_ns: ::Integer,
          ?otel_context_enabled: (::Symbol? | bool),
          ?native_filenames_enabled: bool,
          **untyped
        ) -> Datadog::Profiling::Collectors::ThreadContext
sig/datadog/profiling/collectors/thread_context.rbs:47
└── def safely_extract_context_key_from: (untyped tracer) -> ::Symbol?

If you believe a method or an attribute is rightfully untyped or partially typed, you can add # untyped:accept on the line before the definition to remove it from the stats.

Since #5870 / 41bcd1f overhead samples no longer show up on the thread that does the sampling so we can simplify this.

eregon requested review from a team as code owners June 8, 2026 16:52

dd-octo-sts Bot added the profiling Involves Datadog profiling label Jun 8, 2026

eregon changed the title ~~Simplify profiler sampling overhead attribution with a synthesized stack~~ Simplify profiler sampling overhead attribution with a placeholder stack Jun 8, 2026

chatgpt-codex-connector Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread ext/datadog_profiling_native_extension/collectors_thread_context.c Outdated

eregon force-pushed the simplify-profiler_overhead_stack_thread branch from c453716 to 0c44197 Compare June 8, 2026 16:54

eregon force-pushed the simplify-profiler_overhead_stack_thread branch 4 times, most recently from 34c258e to 27d77fa Compare June 8, 2026 18:21

eregon commented Jun 8, 2026

View reviewed changes

Comment thread ext/datadog_profiling_native_extension/collectors_thread_context.c Outdated

eregon commented Jun 8, 2026

View reviewed changes

Comment thread spec/datadog/profiling/collectors/thread_context_spec.rb Outdated

ivoanjo reviewed Jun 9, 2026

View reviewed changes

eregon force-pushed the simplify-profiler_overhead_stack_thread branch from 27d77fa to c189b9e Compare June 9, 2026 12:31

ivoanjo approved these changes Jun 9, 2026

View reviewed changes

eregon enabled auto-merge June 9, 2026 13:22

eregon force-pushed the simplify-profiler_overhead_stack_thread branch from c189b9e to 41bcd1f Compare June 9, 2026 13:23

eregon merged commit e014f83 into master Jun 9, 2026
592 checks passed

eregon deleted the simplify-profiler_overhead_stack_thread branch June 9, 2026 13:51

dd-octo-sts Bot added this to the 2.36.0 milestone Jun 9, 2026

eregon mentioned this pull request Jun 9, 2026

Skip sampling threads which did not have the GVL since the last sample #5777

Merged

ivoanjo mentioned this pull request Jun 30, 2026

[NO-TICKET] Profiling: Cleanup unneeded sampling_buffer argument #5966

Merged

ivoanjo added a commit that referenced this pull request Jul 7, 2026

Remove filtering of overhead samples on Thread.current

96253d2

Since #5870 / 41bcd1f overhead samples no longer show up on the thread that does the sampling so we can simplify this.

ivoanjo mentioned this pull request Jul 7, 2026

[NO-TICKET] Profiling: Reset per-thread state when starting profiler #5960

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Simplify profiler sampling overhead attribution with a placeholder stack#5870

Simplify profiler sampling overhead attribution with a placeholder stack#5870
eregon merged 1 commit into
masterfrom
simplify-profiler_overhead_stack_thread

eregon commented Jun 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

datadog-prod-us1-3 Bot commented Jun 8, 2026 •

edited by datadog-datadog-prod-us1 Bot

Loading

Uh oh!

pr-commenter Bot commented Jun 8, 2026 •

edited

Loading

Explanation

More details about the CI and significant changes

Uh oh!

Uh oh!

Uh oh!

ivoanjo left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivoanjo left a comment

Uh oh!

dd-octo-sts Bot commented Jun 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

eregon commented Jun 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

datadog-prod-us1-3 Bot commented Jun 8, 2026 • edited by datadog-datadog-prod-us1 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Explanation

More details about the CI and significant changes

scenario:profiling - sample and serialize

Uh oh!

Uh oh!

Uh oh!

ivoanjo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivoanjo left a comment

Choose a reason for hiding this comment

Uh oh!

dd-octo-sts Bot commented Jun 9, 2026

Typing analysis

Untyped methods

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

datadog-prod-us1-3 Bot commented Jun 8, 2026 •

edited by datadog-datadog-prod-us1 Bot

Loading

pr-commenter Bot commented Jun 8, 2026 •

edited

Loading