Skip to content

refactor(profiling): separate task address discovery - #19709

Draft
taegyunkim wants to merge 1 commit into
mainfrom
taegyunkim/profiling-task-address-discovery
Draft

refactor(profiling): separate task address discovery#19709
taegyunkim wants to merge 1 commit into
mainfrom
taegyunkim/profiling-task-address-discovery

Conversation

@taegyunkim

@taegyunkim taegyunkim commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Description

Motivation and scope

This refactor was identified while implementing the CPU timer profiler in #18724. CPU timer signals capture task identity evidence, and the later drain phase needs to scan lightweight task identities before materializing only the matching task and its parents. The existing task traversal could only return fully constructed TaskInfo snapshots.

This PR remains independently mergeable because it contains no CPU timer behavior, configuration, or public API. It only extracts the existing CPython task-source traversal behind a private visitor while preserving the current wall-sampling behavior. PR #18724 is the first planned consumer of the new visitor, but this feature-neutral extraction can be reviewed and merged independently before it.

Before this PR

Asyncio task-source traversal is coupled directly to wall-sampling snapshot construction:

task source -> TaskInfo::create() -> event-loop filter -> wall task list

Each Python-version-specific task source contains its own TaskInfo construction and filtering logic. Another sampling path cannot reuse task discovery without either constructing every full task snapshot or duplicating the CPython task-source traversal.

After this PR

Task-source traversal reports each address to a shared visitor:

                                      ┌-> wall visitor: TaskInfo::create() -> filter -> wall task list
task source -> visit task address ----┤
                                      └-> follow-up visitor: lightweight task identity

Wall sampling still constructs and filters each TaskInfo immediately when its address is visited. It does not collect task addresses for later materialization, so this PR does not add a temporal gap or change the task-snapshot order.

The following behavior is unchanged:

  • Python task sources and their traversal order.
  • Python 3.14 queries its per-thread and per-interpreter native task lists only when both tstate and tstate_addr are available.
  • Immediate wall-sampling TaskInfo construction and event-loop filtering.
  • Best-effort handling of unreadable or concurrently removed tasks.

The CPU timer follow-up can use the same visitor to copy only each task's address, coroutine address, and waiter address. After identifying the task captured by the signal handler, it constructs TaskInfo only for that task and the parents needed for logical ancestry.

Review guide

This is intended to be a mechanical extraction. The existing comments, helper names, local variables, source conditions, and traversal control flow are retained.

  1. The linked-list helpers now invoke callback(task_address) where they previously constructed TaskInfo.
  2. The Python 3.14 and pre-3.14 task-source loops make the same substitution without changing their surrounding logic.
  3. get_all_tasks() supplies a callback containing the previous TaskInfo::create(), event-loop filter, and append operation.

There is no intermediate task-address vector and no change to the wall sampler's point of materialization.

Testing

  • Built the native profiling stack in release mode with warnings as errors against CPython 3.10.12 and CPython 3.14.5.
  • Ran scripts/lint cformat.
  • Ran scripts/lint profiling-native-check.
  • Ran scripts/lint checks.
  • Ran a CPython 3.10 wall-profiler workload with 500 suspended asyncio tasks and fixed 1 ms sampling. Repeated 3-second measurements showed no measurable regression, with baseline and candidate medians overlapping around 0.44 CPU seconds.

Risks

Low. Task-source traversal now invokes a private callback for each address. Wall sampling performs the same guarded snapshot construction and filtering inside that callback, at the same point in traversal as before.

Additional Notes

No release note is needed because this is an internal refactor with no customer-visible behavior change.

@taegyunkim taegyunkim added the changelog/no-changelog A changelog entry is not required for this PR. label Aug 14, 2026
@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Codeowners resolved as

Resolved from the full PR diff against main using the target branch CODEOWNERS file.
CODEOWNERS team requests not listed below are not required by the current file set.

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 Aug 14, 2026

Copy link
Copy Markdown

Circular import analysis

⚠️ Existing circular imports

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

ddtrace.contrib.internal.pytorch._distributed -> ddtrace.contrib.internal.pytorch._rank_root -> ddtrace.contrib.internal.pytorch._distributed
ddtrace.contrib.internal.django.patch -> ddtrace.contrib.internal.django.response -> ddtrace.contrib.internal.django.patch
ddtrace.llmobs -> ddtrace.llmobs._evaluators -> ddtrace.llmobs._evaluators.format -> ddtrace.llmobs._experiment -> ddtrace.llmobs
ddtrace.errortracking._handled_exceptions.bytecode_injector -> ddtrace.errortracking._handled_exceptions.callbacks -> ddtrace.errortracking._handled_exceptions.collector -> ddtrace.errortracking._handled_exceptions.bytecode_reporting -> ddtrace.errortracking._handled_exceptions.bytecode_injector
ddtrace.appsec._asm_request_context -> ddtrace.appsec._iast._iast_request_context_base -> ddtrace.appsec._iast._iast_env -> ddtrace.appsec._iast.reporter -> ddtrace.appsec._exploit_prevention.stack_traces -> ddtrace.appsec._asm_request_context

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 14, 2026

Copy link
Copy Markdown

Dependency direction analysis

⚠️ Existing dependency direction violations

There are 255 dependency direction violations that already exist on the base branch and have not been changed by this PR.

Show existing violations (showing 5 of 255 highest severity)
ddtrace.internal.tracemethods -×-> ddtrace.trace  (internal-core -> product:tracing, score=134)
ddtrace.internal.test_visibility.api -×-> ddtrace.trace  (product:ci_visibility -> product:tracing, score=132)
ddtrace.llmobs._integrations.anthropic -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=132)
ddtrace.internal.ci_visibility.recorder -×-> ddtrace.trace  (product:ci_visibility -> product:tracing, score=132)
ddtrace.llmobs._integrations.vllm -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=132)

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

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

@datadog-prod-us1-4

datadog-prod-us1-4 Bot commented Aug 14, 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: 9bda1d5 | Docs | Datadog PR Page | Give us feedback!

@taegyunkim
taegyunkim force-pushed the taegyunkim/profiling-task-address-discovery branch from 1fdd48c to 9f3c682 Compare August 14, 2026 15:53
@taegyunkim
taegyunkim force-pushed the taegyunkim/profiling-task-address-discovery branch from 9f3c682 to 9bda1d5 Compare August 14, 2026 16:54
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant