Skip to content

refactor(profiling): split stack sampler helpers - #18975

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
taegyunkim/prof-14213-stack-sampler-refactor
Jul 13, 2026
Merged

refactor(profiling): split stack sampler helpers#18975
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
taegyunkim/prof-14213-stack-sampler-refactor

Conversation

@taegyunkim

@taegyunkim taegyunkim commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Description

Pure refactor of existing stack sampler internals, with no intended behavior or internal contract changes, in preparation for timer_create based cpu time profiling in #18724

Specifically:

  • Extracts the existing task/greenlet/thread-stack rendering branch in ThreadInfo::sample() into a local render_unwound_stacks() helper.
  • Extracts duplicate one-time thread registration failure logging in Sampler::register_thread() into a local helper.

This does not introduce CPU timer profiling, new configuration, new native bindings, or changes to thread registration semantics.

Testing

  • scripts/lint cformat

Attempted a targeted profiling test run from the worktree, but the test runner failed during riot venv setup before executing tests due the worktree using /home/bits/project/.riot, which was not writable/available in that context.

Risks

Low. This is intended to be behavior-preserving refactoring only.

Additional Notes

No changelog needed, internal refactor only.

PROF-14213

@taegyunkim taegyunkim added the changelog/no-changelog A changelog entry is not required for this PR. label Jul 9, 2026
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 584be28 | Docs | Datadog PR Page | Give us feedback!

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codeowners resolved as

ddtrace/internal/datadog/profiling/stack/echion/echion/threads.h        @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/src/echion/threads.cc          @DataDog/profiling-python

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Jul 9, 2026

Copy link
Copy Markdown

Circular import analysis

⚠️ Existing circular imports

There are 4974 circular imports that already exist on the base branch and have not been changed by this PR.

Show existing cycles (showing 5 of 4974 shortest)
ddtrace.internal.core -> ddtrace._trace.span -> ddtrace.internal.core
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.google_cloud_pubsub -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.kafka -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.kombu -> ddtrace.internal.datastreams
ddtrace.internal.datastreams -> ddtrace.internal.datastreams.aiokafka -> ddtrace.internal.datastreams

To see all cycles, download the cycles-base.json and cycles-pr.json artifacts from this CI job and run:

uv run --script scripts/import-analysis/cycles.py compare cycles-base.json cycles-pr.json

Extract the task/greenlet/normal-stack rendering block out of
ThreadInfo::sample into a render_unwound_stacks() free function so the
rendering path can be reused by additional callers. Pure refactor: same
render order, renderer, and container clears; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@taegyunkim
taegyunkim force-pushed the taegyunkim/prof-14213-stack-sampler-refactor branch from 9866b28 to 8ac5dd7 Compare July 9, 2026 22:12
@taegyunkim taegyunkim added the Profiling Continous Profling label Jul 10, 2026
@taegyunkim
taegyunkim marked this pull request as ready for review July 10, 2026 15:54
@taegyunkim
taegyunkim requested a review from a team as a code owner July 10, 2026 15:54
@taegyunkim
taegyunkim requested a review from gyuheon0h July 10, 2026 15:54
Comment thread ddtrace/internal/datadog/profiling/stack/src/echion/threads.cc Outdated
taegyunkim and others added 3 commits July 10, 2026 16:37
Extract the task/greenlet/normal-stack rendering block out of
ThreadInfo::sample into a private ThreadInfo::render_unwound_stacks()
method so the rendering path can be reused by additional callers,
consistent with the existing unwind_tasks/unwind_greenlets helpers. Pure
refactor: same render order, renderer, and container clears; no behavior
change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…//github.com/DataDog/dd-trace-py into taegyunkim/prof-14213-stack-sampler-refactor

# Conflicts:
#	ddtrace/internal/datadog/profiling/stack/src/echion/threads.cc

@KowalskiThomas KowalskiThomas left a comment

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.

Code changes LGTM

@taegyunkim

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Jul 13, 2026

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-07-13 16:58:00 UTC ℹ️ Start processing command /merge


2026-07-13 16:58:06 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 1h (p90).


2026-07-13 17:45:38 UTC ℹ️ MergeQueue: This merge request was merged

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit 9344867 into main Jul 13, 2026
469 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the taegyunkim/prof-14213-stack-sampler-refactor branch July 13, 2026 17:45
vlad-scherbich pushed a commit that referenced this pull request Jul 15, 2026
## Description

Pure refactor of existing stack sampler internals, with no intended behavior or internal contract changes, in preparation for timer_create based cpu time profiling in #18724 

Specifically:

- Extracts the existing task/greenlet/thread-stack rendering branch in `ThreadInfo::sample()` into a local `render_unwound_stacks()` helper.
- Extracts duplicate one-time thread registration failure logging in `Sampler::register_thread()` into a local helper.

This does not introduce CPU timer profiling, new configuration, new native bindings, or changes to thread registration semantics.

## Testing

- `scripts/lint cformat`

Attempted a targeted profiling test run from the worktree, but the test runner failed during riot venv setup before executing tests due the worktree using `/home/bits/project/.riot`, which was not writable/available in that context.

## Risks

Low. This is intended to be behavior-preserving refactoring only.

## Additional Notes

No changelog needed, internal refactor only.


Co-authored-by: taegyun.kim <taegyun.kim@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR. Profiling Continous Profling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants