Skip to content

refactor(profiling): drop bytecode-lib dependency from _asyncio.py#17995

Open
vlad-scherbich wants to merge 16 commits into
mainfrom
vlad/prof-replace-bytecode-dependency
Open

refactor(profiling): drop bytecode-lib dependency from _asyncio.py#17995
vlad-scherbich wants to merge 16 commits into
mainfrom
vlad/prof-replace-bytecode-dependency

Conversation

@vlad-scherbich
Copy link
Copy Markdown
Contributor

@vlad-scherbich vlad-scherbich commented May 10, 2026

https://datadoghq.atlassian.net/browse/PROF-14616

Description

ddtrace/profiling/_asyncio.py previously hooked into asyncio internals via ddtrace.internal.wrapping.wrap, which rewrites the target's __code__ using the third-party bytecode library.

This PR replaces that with a small in-module _wrap() helper that clones a template trampoline's __code__ via CodeType.replace() and grafts it onto the original function. It is the same identity-preserving trick the previous bytecode.wrap() used, but without the dependency!

Why

  • We plan to remove the bytecode dependency during migration to v3.15 (@gab is working on the tracing-side implementation), because it blocks the entire minor version release until it become compatible
  • All 9 wrap sites in this file target plain non-async, non-generator callables. They do not need bytecode-level wrapping.
  • Eliminate a direct dependency on a fragile external lib that has to be upgraded to a new CPython version, making as dependent on its release schedule.

Scope

This PR affects profiling consumers only. ddtrace.internal.wrapping.wrap itself is unchanged — the tracing path still genuinely needs bytecode for async/generator function wrapping with stack-effect preservation. The other (legitimate) bytecode consumers (ddtrace/debugging/_expressions.py, ddtrace/internal/assembly.py, ddtrace/internal/bytecode_injection/__init__.py) are untouched.

Testing

  • New behavioral test suite at tests/profiling/collector/test_asyncio_wrapping.py (16 tests).
  • Passes on main and branch

Performance

TL;DR:

  • this branch adds very little overhead, and it is basically buried in the noise between bencmark runs

Microbenchmarked on Python 3.14.3 / macOS arm64.
Three-way:

  • unwrapped baseline
  • main (bytecode wrap)
  • branch

N = 12 trials per variant; each trial:

  • create_task: 200,000 iterations per timing
  • gather3 / shield: 2,000 iterations per timing
op baseline mean main mean branch mean branch − main 95 % CI verdict
create_task 673 ns 799 ns 724 ns −75 ns [−162, +13] ns indistinguishable
gather3 76.3 µs 78.9 µs 77.5 µs −1.4 µs [−5.3, +2.5] µs indistinguishable
shield 72.2 µs 78.0 µs 73.9 µs −4.2 µs [−9.3, +1.0] µs indistinguishable

Risks

  • Unwrapping not implemented: as before, the wrappers are installed for the lifetime of the process; nothing in the existing code unwraps them. This is the same as with current bytecode-dependency implementation.

@vlad-scherbich vlad-scherbich added changelog/no-changelog A changelog entry is not required for this PR. Profiling Continous Profling labels May 10, 2026
@datadog-prod-us1-3
Copy link
Copy Markdown

datadog-prod-us1-3 Bot commented May 10, 2026

Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

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

@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented May 10, 2026

Benchmarks

Benchmark execution time: 2026-05-10 20:21:06

Comparing candidate commit a982199 in PR branch vlad/prof-replace-bytecode-dependency with baseline commit d1a7113 in branch main.

Found 0 performance improvements and 5 performance regressions! Performance is the same for 592 metrics, 4 unstable metrics.

scenario:iast_aspects-re_expand_aspect

  • 🟥 execution_time [+257.868µs; +287.952µs] or [+7.645%; +8.537%]

scenario:iastaspects-lower_aspect

  • 🟥 execution_time [+57.183µs; +63.855µs] or [+19.453%; +21.723%]

scenario:iastaspectsospath-ospathbasename_aspect

  • 🟥 execution_time [+90.950µs; +99.087µs] or [+22.394%; +24.398%]

scenario:span-start

  • 🟥 execution_time [+1.141ms; +1.313ms] or [+7.307%; +8.412%]

scenario:telemetryaddmetric-1-count-metric-1-times

  • 🟥 execution_time [+199.065ns; +233.654ns] or [+9.545%; +11.204%]

@vlad-scherbich vlad-scherbich force-pushed the vlad/prof-replace-bytecode-dependency branch from a982199 to 87bd294 Compare May 10, 2026 20:27
@cit-pr-commenter-54b7da
Copy link
Copy Markdown

cit-pr-commenter-54b7da Bot commented May 10, 2026

Codeowners resolved as

tests/profiling/collector/test_asyncio_wrapping.py                      @DataDog/profiling-python

@vlad-scherbich
Copy link
Copy Markdown
Contributor Author

@hatgpt-codex-connector please review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the asyncio profiling integration to avoid ddtrace.internal.wrapping.wrap() (and its third-party bytecode dependency) by introducing a local _wrap() helper that uses functools.wraps + setattr, including explicit alias mirroring for asyncio.* / asyncio.tasks.* bindings.

Changes:

  • Replaced bytecode-based wrapping in ddtrace/profiling/_asyncio.py with an in-module _wrap() helper and added alias mirroring where needed.
  • Added a comprehensive new subprocess-based test suite covering alias identity, metadata preservation, callback firing, and argument/kwarg substitution behaviors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
ddtrace/profiling/_asyncio.py Introduces _wrap() and migrates asyncio/uvloop hook points off bytecode-based wrapping, with alias mirroring for selected APIs.
tests/profiling/collector/test_asyncio_wrapping.py Adds subprocess-isolated behavioral tests validating wrapping invariants and callback firing across asyncio APIs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ddtrace/profiling/_asyncio.py
Comment thread tests/profiling/collector/test_asyncio_wrapping.py Outdated
Comment thread tests/profiling/collector/test_asyncio_wrapping.py Outdated
@vlad-scherbich vlad-scherbich changed the title Vlad/prof replace bytecode dependency refactor(profiling): remove bytecode dependency by implementing a custom wrapper May 11, 2026
@vlad-scherbich vlad-scherbich force-pushed the vlad/prof-replace-bytecode-dependency branch 2 times, most recently from 02c593b to 9133537 Compare May 11, 2026 19:33
@vlad-scherbich vlad-scherbich requested a review from Copilot May 11, 2026 19:37
@vlad-scherbich
Copy link
Copy Markdown
Contributor Author

@chatgpt-codex-connector please review

@vlad-scherbich vlad-scherbich changed the title refactor(profiling): remove bytecode dependency by implementing a custom wrapper chore(profiling): remove bytecode dependency by implementing a custom wrapper May 11, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread ddtrace/profiling/_asyncio.py Outdated
Comment thread tests/profiling/collector/_asyncio_wrap_helpers.py
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9133537072

ℹ️ 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".

Comment thread ddtrace/profiling/_asyncio.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@vlad-scherbich vlad-scherbich force-pushed the vlad/prof-replace-bytecode-dependency branch from 2e404db to 348bb66 Compare May 11, 2026 20:48
@vlad-scherbich vlad-scherbich changed the title chore(profiling): remove bytecode dependency by implementing a custom wrapper refactor(profiling): drop bytecode-lib dependency from _asyncio.py May 11, 2026
@vlad-scherbich vlad-scherbich marked this pull request as ready for review May 11, 2026 21:29
@vlad-scherbich vlad-scherbich requested a review from a team as a code owner May 11, 2026 21:29
@vlad-scherbich vlad-scherbich requested a review from taegyunkim May 11, 2026 21:29
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