Skip to content

refactor(profiling): clarify stack discovery limits - #20352

Draft
taegyunkim wants to merge 1 commit into
mainfrom
taegyunkim/prof-14213-clarify-stack-discovery-limit
Draft

taegyunkim wants to merge 1 commit into
mainfrom
taegyunkim/prof-14213-clarify-stack-discovery-limit

Conversation

@taegyunkim

@taegyunkim taegyunkim commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Description

PROF-14213

Clarifies the existing native stack safety limit by renaming MAX_TASK_FRAMES to MAX_STACK_DISCOVERY_DEPTH everywhere it is used, while retaining the value 2048 and the current behavior.

Extracts ThreadInfo::is_asyncio_boundary_frame from find_upper_python_stack_size. The latter still scans the full discovered Python stack, preserves its return semantics, and retains the existing asyncio/uvloop boundary-frame caches.

This deliberately does not introduce the bounded task frame budget from #19085.

Testing

  • scripts/lint cformat: passed.
  • scripts/lint checks: passed (existing repository-wide AST-grep warnings were reported).
  • scripts/run-tests with the Python 3.13 profiling environment: 408 passed, 35 skipped, 2 xfailed.
  • scripts/run-tests with the Python 3.13 uvloop profiling environment: 411 passed, 32 skipped, 2 xfailed.
  • Native stack standalone test build on Python 3.13: all 23 C++ tests passed, including StackUnwind.ProbeBudgetExhaustionRemainsUnknown.
  • scripts/run-benchmarks --list <changed files>: no matching benchmark suites.

Risks

Low. This is an internal naming and extraction refactor. The safety-limit value, all call sites, full-stack boundary scan, boundary caching, and stitching behavior are unchanged.

Additional Notes

This is prerequisite 1 for #19085.

No release note is needed because the change is internal and behavior-preserving; the PR uses the changelog/no-changelog label.

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

datadog-prod-us1-5 Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Pipelines  Tests

⚠️ Warnings

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 1 Pipeline job failed

Changelog | Validate changelog

View more details · View in GitHub Actions

ℹ️ Info

No other issues found (see more)

🧪 All tests passed
❄️ No new flaky tests detected

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: caccca3 | Docs | View more details | Give us feedback!

@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/config.h         @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/echion/echion/threads.h        @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/fuzz/fuzz_echion_stacks.cpp    @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/src/echion/greenlets.cc        @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/src/echion/stacks.cc           @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/src/echion/threads.cc          @DataDog/profiling-python
ddtrace/internal/datadog/profiling/stack/test/test_sampling_cycle_state.cpp  @DataDog/profiling-python

@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Circular import analysis

⚠️ Existing circular imports

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

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

@cit-pr-commenter-54b7da

Copy link
Copy Markdown

Dependency direction analysis

⚠️ Existing dependency direction violations

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

Show existing violations (showing 5 of 218 highest severity)
ddtrace.internal.tracemethods -×-> ddtrace.trace  (internal-core -> product:tracing, score=133)
ddtrace.profiling.collector.stack -×-> ddtrace.trace  (product:profiling -> product:tracing, score=131)
ddtrace.llmobs._integrations.crewai -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=131)
ddtrace.llmobs._integrations.mcp -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=131)
ddtrace.internal.ci_visibility.filters -×-> ddtrace.trace  (product:ci_visibility -> product:tracing, score=131)

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

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