chore(profiling): opt-in native heap gotter build and skip musl (PROF-15423) - #19715
chore(profiling): opt-in native heap gotter build and skip musl (PROF-15423)#19715vlad-scherbich wants to merge 4 commits into
Conversation
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds build-time helpers to standardize environment-flag parsing and prevents building the native heap-gotter artifact on musl-based Python builds (e.g., Alpine/musllinux), where CI/cargo builds are expected to fail.
Changes:
- Introduces
_env_truthy()for consistent parsing of boolean-ish env vars. - Adds
is_musl_libc()detection viasysconfigbuild metadata. - Gates
BUILD_NATIVE_HEAP_GOTTERon both the env flag andnot is_musl_libc().
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
Use _env_truthy consistently, clarify musl docstring, and warn when native heap build is requested on musllinux.
e27c0a6 to
af9a77f
Compare
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
Circular import analysis
|
BenchmarksBenchmark execution time: 2026-08-16 13:47:00 Comparing candidate commit 2566b2e in PR branch Found 0 performance improvements and 6 performance regressions! Performance is the same for 616 metrics, 10 unstable metrics.
|
vlad-scherbich
changed the title
vlad-scherbich
added
the
changelog/no-changelog
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
setup.py:164
_env_truthy(\"DD_PROFILING_NATIVE_HEAP_ENABLED\")andis_musl_libc()are each evaluated twice here. Store their results in local variables (e.g.,native_heap_enabled/musl) and reuse them to avoid duplicated work and keep the logic easier to follow.
# Musl is always a no-op even when the env is set (see is_musl_libc).
if _env_truthy("DD_PROFILING_NATIVE_HEAP_ENABLED") and is_musl_libc():
print(
"WARNING: DD_PROFILING_NATIVE_HEAP_ENABLED is set but the native heap-gotter "
"cdylib is only built on manylinux (glibc); skipping on musllinux."
)
BUILD_NATIVE_HEAP_GOTTER: bool = _env_truthy("DD_PROFILING_NATIVE_HEAP_ENABLED") and not is_musl_libc()
setup.py:162
- Emitting warnings via
print()fromsetup.pycan be noisy and may not be captured consistently by build backends. Prefer routing this through a warning/logging mechanism intended for build output (e.g.,warnings.warn(..., RuntimeWarning)or a setuptools/distutils logger), ideally to stderr.
print(
"WARNING: DD_PROFILING_NATIVE_HEAP_ENABLED is set but the native heap-gotter "
"cdylib is only built on manylinux (glibc); skipping on musllinux."
)
setup.py:170
- Now that
_env_truthy()exists, this line duplicates the prior boolean-parsing pattern. Consider switchingSERVERLESS_BUILDto_env_truthy(\"DD_SERVERLESS_BUILD\")for consistency and to keep the accepted truthy values centralized.
SERVERLESS_BUILD = os.getenv("DD_SERVERLESS_BUILD", "0").lower() in ("1", "yes", "on", "true")
f4338f9 to
a86da7a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (5)
setup.py:135
_env_truthy()will treat values with leading/trailing whitespace as false (e.g.,DD_FOO='true '), which can be surprising when env vars come from CI or shell tooling. Consider normalizing with.strip().lower()before membership checking.
def _env_truthy(name: str, default: str = "0") -> bool:
return os.getenv(name, default).lower() in ("1", "yes", "on", "true")
setup.py:164
is_musl_libc()is called twice; it would be simpler and avoid repeatedsysconfig.get_config_var()lookups to compute it once (e.g.,musl = is_musl_libc()) and reuse it for both the warning branch andBUILD_NATIVE_HEAP_GOTTER.
if _env_truthy("DD_PROFILING_NATIVE_HEAP_ENABLED") and is_musl_libc():
setup.py:164
is_musl_libc()is called twice; it would be simpler and avoid repeatedsysconfig.get_config_var()lookups to compute it once (e.g.,musl = is_musl_libc()) and reuse it for both the warning branch andBUILD_NATIVE_HEAP_GOTTER.
BUILD_NATIVE_HEAP_GOTTER: bool = _env_truthy("DD_PROFILING_NATIVE_HEAP_ENABLED") and not is_musl_libc()
setup.py:161
- Emitting a warning via
print()insetup.pycan be noisy and hard to filter in build logs. Preferwarnings.warn(...)(ordistutils.log.warn(...)if this repo uses distutils logging elsewhere) so users can control visibility and tooling can classify it properly.
print(
"WARNING: DD_PROFILING_NATIVE_HEAP_ENABLED is set but the native heap-gotter "
"cdylib is only built on manylinux (glibc); skipping on musllinux."
)
setup.py:167
- This assignment drops the explicit
: booltype annotation that existed previously for the same flag, whileBUILD_NATIVE_HEAP_GOTTERremains annotated. Consider keepingBUILD_NATIVE_HEAP_GOTTER_TEST_SUPPORT: bool = ...for consistent typing among build flags.
BUILD_NATIVE_HEAP_GOTTER_TEST_SUPPORT = _env_truthy("DD_PROFILING_NATIVE_HEAP_TEST_SUPPORT")
Document setup.py gate: gotter cdylib ships only when DD_PROFILING_NATIVE_HEAP_ENABLED=1 at build time; musl always skips.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
setup.py:164
- Calling
is_musl_libc()twice duplicates the detection work and can also lead to inconsistent outcomes ifsysconfigvalues differ across calls (e.g., due to environment changes during setup execution). Compute the musl flag once (e.g.,musl = is_musl_libc()) and reuse it for both the warning andBUILD_NATIVE_HEAP_GOTTERassignment.
# Musl is always a no-op even when the env is set (see is_musl_libc).
if _env_truthy("DD_PROFILING_NATIVE_HEAP_ENABLED") and is_musl_libc():
print(
"WARNING: DD_PROFILING_NATIVE_HEAP_ENABLED is set but the native heap-gotter "
"cdylib is only built on manylinux (glibc); skipping on musllinux."
)
BUILD_NATIVE_HEAP_GOTTER: bool = _env_truthy("DD_PROFILING_NATIVE_HEAP_ENABLED") and not is_musl_libc()
setup.py:162
- Using
print()insetup.pycan be swallowed or poorly formatted depending on the build frontend (pip/build/PEP 517). Prefer emitting a warning viawarnings.warn(..., RuntimeWarning)(or the project’s existing logging/warning mechanism if one exists) so build logs consistently capture it and tooling can surface it appropriately.
print(
"WARNING: DD_PROFILING_NATIVE_HEAP_ENABLED is set but the native heap-gotter "
"cdylib is only built on manylinux (glibc); skipping on musllinux."
)
Description
Enable building of ddtrace wheels with heap-gotter lib via GitLab.
libdd_heap_gotteris never cargo-built on musllinux/Alpine, even whenDD_PROFILING_NATIVE_HEAP_ENABLED=1is set, as these platforms are incompatible with the feature.Previous PR #19325 compiles
libdd_heap_gotterinto cdylib, wheel and activates the gotter's USDTs via the same flag, so in total there are two separate parts of the codebase that use the same flag in different ways.Testing
1. Default pipeline — flag off
Push the branch without
DD_PROFILING_NATIVE_HEAP_ENABLED.test sdistpip installfrom sdist succeedsbuild linux(musllinux matrix, cp39–cp314)upload manylinux2014_x86_64Log checks: no
build_heap_gotterphase; noBuilt and copied heap-gotter cdylib.Wheel check (optional):
Verified: pipeline 131157157 — sdist + musl cp39–cp314 green, manylinux upload green, no gotter in wheels. Unrelated failures: cp315 (allow_failure), win_arm64.
2. Opt-in pipeline — flag on (manylinux)
Trigger with
DD_PROFILING_NATIVE_HEAP_ENABLED=1.GitLab UI: CI/CD → Run pipeline → branch
vlad/native-heap-ci-opt-in→ variableDD_PROFILING_NATIVE_HEAP_ENABLED=1→ Run.GitLab API:
build linux(manylinux2014, amd64 + arm64)upload manylinux2014_x86_64test sdistprofiling_nativeLog checks (manylinux rows):
build_heap_gotterphase;Built and copied heap-gotter cdylib: …libdd_heap_gotter….Wheel check:
Verified: pipeline 131157771 —
libdd_heap_gotterin manylinux wheels, upload + sdist green,profiling_native20/20.3. Musl skip — flag on
Same pipeline as §2; inspect musllinux matrix rows (
musllinux_1_2_*images).WARNING: DD_PROFILING_NATIVE_HEAP_ENABLED is set but the native heap-gotter cdylib is only built on manylinux (glibc); skipping on musllinux.build_heap_gotter/Built and copied heap-gotter cdylibin musl job logslibdd_heap_gotter*.sobuild linuxmusl jobs (cp39–cp314) passVerified: pipeline 131157771 — musl skips gotter build even with flag set.
Additional Notes
DD_PROFILING_NATIVE_HEAP_ENABLED=1)