feat(profiling): native-heap live-heap cleanup (ABI, build env, auditwheel) (PROF-15423) - #19325
feat(profiling): native-heap live-heap cleanup (ABI, build env, auditwheel) (PROF-15423)#19325vlad-scherbich wants to merge 6 commits into
Conversation
Circular import analysis
|
Codeowners resolved asResolved from the full PR diff against |
|
f4f0ae3 to
3cf4d61
Compare
BenchmarksBenchmark execution time: 2026-08-16 13:20:26 Comparing candidate commit 7d2117f in PR branch Found 0 performance improvements and 8 performance regressions! Performance is the same for 614 metrics, 10 unstable metrics.
|
ddheap:free USDT in native heap gotter (PROF-15423)
ae8d175 to
e37b55f
Compare
f46c4e8 to
12d47f0
Compare
|
I think it would be good to also have a runtime knob to disable live heap - not do the GOT table override for By omitting the USDT altogether in the binary, the profiler gets the hint it doesn't need to track allocations at all for a process, so its kind of a nice optimisation for folks who really really sweat resources, but I figure the regular user is going to probably going to want to tune at runtime |
e37b55f to
df7f0a4
Compare
12d47f0 to
1f8f760
Compare
8006548 to
1d2a694
Compare
1f8f760 to
77fb65c
Compare
1d2a694 to
33e53d4
Compare
77fb65c to
0c95d87
Compare
33e53d4 to
8fb8c04
Compare
15f4534 to
4bbefac
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Completes the producer-side “live heap” native heap profiling stack by exposing whether the loaded heap-gotter cdylib was compiled with live-heap tracking (alloc + free), surfacing that in the Python activator and profiler logs, and updating build/release-note plumbing accordingly.
Changes:
- Add a stable C ABI symbol (
ddtrace_heap_gotter_live_heap_enabled) and a Python wrapper (heap_gotter.live_heap_enabled()) to report live-heap capability from the loaded cdylib. - Improve profiler arming observability by logging whether “live-heap” vs “allocation-only” was armed.
- Update wheel build helper script to more robustly stash/remove/reinsert the gotter artifact around
auditwheel, and add a consolidated release note.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/profiling/test_native_heap_gotter.py | Extends smoke test to assert live_heap_enabled() is safe when cdylib is absent and returns a bool when present. |
| src/native_heap_gotter/lib.rs | Exposes ddtrace_heap_gotter_live_heap_enabled() over the C ABI using cfg!(feature = "live-heap"). |
| src/native_heap_gotter/Cargo.toml | Adds crate-local live-heap feature (default-on) intended to mirror upstream live-heap compilation. |
| releasenotes/notes/native-heap-profiling-live-heap-336bc489c93989da.yaml | Adds a single release note describing the end-to-end experimental native heap profiling feature. |
| ddtrace/profiling/profiler.py | Logs which native-heap mode was armed based on heap_gotter.live_heap_enabled(). |
| ddtrace/internal/datadog/profiling/heap_gotter/init.py | Defensively binds the new cdylib symbol and exposes live_heap_enabled() fail-closed. |
| .gitlab/scripts/build-wheel-helpers.sh | Switches to a pure-Python stash/remove flow for the heap-gotter artifact before auditwheel, and reinserts only the runtime .so. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fff0a7e to
94fe7be
Compare
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fff0a7ee02
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
ddtrace/internal/datadog/profiling/heap_gotter/init.py:5
- This module docstring uses reStructuredText-style inline literals (
...) even though this is an internal module (not Sphinx-rendered). This makes the docstring noisier to read in editors; prefer plain prose for internal docstrings.
"""Activator for native (C/C++) heap allocation profiling via GOT rewriting.
Dlopens ``libdd_heap_gotter`` (see ``src/native_heap_gotter``) and drives:
bool ddtrace_heap_gotter_install(void);
2046272 to
c4faf56
Compare
ddheap:free USDT in native heap gotter (PROF-15754)c4faf56 to
50cbaab
Compare
Review asked how ddtrace config and the libdatadog process-start bypass interact; record the two independent AND-gates on the install path.
69b9642 to
0dd603e
Compare
There was a problem hiding this comment.
When trying to build native heap profiling from this PR's code, I get several failing jobs (see #19701).
error: cannot produce cdylib for `ddtrace-heap-gotter v0.1.0 (/go/src/github.com/DataDog/apm-reliability/dd-trace-py/src/native_heap_gotter)` as the target `x86_64-unknown-linux-musl` does not support these crate types
That doesn't seem intended, since trying to build with heap profiling should probably soft fail if unsupported on a platform, can you look into it?
There was a problem hiding this comment.
fixed here, and being handled gracefully on #19715
Align help text with setup.py and runtime config (DD_PROFILING_NATIVE_HEAP_ENABLED).
2c59e60 to
664c8fa
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
ddtrace/internal/settings/profiling.py:562
- This help text conflates the runtime enable flag with the wheel build requirement.
DD_PROFILING_NATIVE_HEAP_ENABLEDis used to enable the feature at runtime, but the wheel must also include the gotter cdylib (which is currently opt-in at build time persetup.py). Suggest updating the help string to explicitly say it requires a wheel that ships the native heap gotter artifact (and, if applicable, how that artifact is produced), instead of implying setting the runtime env var affects the wheel contents.
"DD_PROFILING_NATIVE_HEAP_ENABLED=1. Disabled by default."
releasenotes/notes/native-heap-profiling-live-heap-336bc489c93989da.yaml:5
- The release note implies that setting
DD_PROFILING_NATIVE_HEAP_ENABLED=trueis sufficient, but the implementation still requires a wheel that actually contains thelibdd_heap_gotterartifact (currently opt-in at build time). Please amend the note to mention the wheel/artifact requirement (or adjust build defaults if the intent is to ship this in mainline wheels). Also remove the trailing whitespace afterallocationson line 4.
profiling: Enable collection of samples for native (C/C++) allocations
in Python processes. Enable with ``DD_PROFILING_NATIVE_HEAP_ENABLED=true``.
This setting works for Linux only.
tests/profiling/test_native_heap_gotter.py:33
- This test hard-requires
live_heap_enabled()to beTruewhenever the cdylib is available, but the PR explicitly supports alloc-only builds via--no-default-features(andlive_heap_enabled()is documented to return False in that case). To avoid configuration-dependent failures, either (a) assert only thatlive_heap_enabled()returns a boolean / does not raise, or (b) gate the expected value on an explicit build/test signal (e.g., an env var indicating the wheel was built withlive-heap).
assert heap_gotter.is_installed() is False
assert heap_gotter.install() is True
assert heap_gotter.is_installed() is True
assert heap_gotter.install() is True # idempotent
# Default gotter builds enable the live-heap Cargo feature (ddheap:free).
assert heap_gotter.live_heap_enabled() is True
ddtrace/internal/datadog/profiling/heap_gotter/init.py:17
- The docstring labels
DD_PROFILING_NATIVE_HEAP_ENABLEDas a 'setup.py build gate' and a runtime gate. The build decision happens at package build time, not at runtime, and users cannot make an already-built wheel include the cdylib by setting a runtime env var. Please reword this section to distinguish: (1) runtime arming (DD_PROFILING_NATIVE_HEAP_ENABLED) vs (2) whether the installed wheel actually contains the gotter artifact (controlled during wheel build).
Two independent runtime gates, both required for GOT to be patched:
* ``DD_PROFILING_NATIVE_HEAP_ENABLED`` — ddtrace install gate (also the
setup.py build gate). When false, this module is not imported from the
profiler and ``install()`` is never called.
Clarify wheel build vs runtime gates in the release note and heap_gotter docs; relax live_heap_enabled smoke assertion for alloc-only builds.
Description
Multiple cleanup to close off the native-heap feature code commits in Python profiler.
Changes
live-heapas the default-on gate for upstream).ddtrace_heap_gotter_live_heap_enabled) + Python query + mode log (live-heapvsallocation-only).DD_PROFILING_NATIVE_HEAP_BUILDintoDD_PROFILING_NATIVE_HEAP_ENABLED..so.debugstash/restore.Testing