v5.16.0 proposal#364
Merged
Merged
Conversation
GitHub deprecated the Node.js 20 runtime; bump the actions/* to versions that run on Node.js 24, matching DataDog/action-prebuildify: - actions/checkout -> v7 - actions/setup-node -> v6 - actions/download-artifact -> v7 (release.yml) - the release download-artifact steps now fetch the prebuilds artifact by name into ./prebuilds, since v5+ no longer nests a single nameless artifact under its own directory
Yarn Berry (Plug'n'Play) prints "YN0007: @datadog/pprof must be built" for any dependency that declares an install/preinstall/postinstall script, even the no-op "exit 0". The package ships prebuilt binaries and excludes binding.gyp from the published tarball, so a consumer has nothing to build; the script only suppressed npm's implicit node-gyp rebuild in the dev tree, which CI and the prepare script do explicitly. Removing it stops the spurious warning for Yarn Berry consumers. Refs: DataDog/dd-trace-js#5432
…stom-labels (#347) Port OTEP-4947 thread-context writer from polarsignals/custom-labels Adds a Node.js writer for the OpenTelemetry Thread Local Context Record (OTEP-4947), ported from the in-development upstream at polarsignals/custom-labels (#16, #17). Native addon (bindings/otel-thread-ctx.cc/.hh): the writer, namespaced as dd::OtelThreadCtx::Init(exports) and called from binding.cc. The thread_local discovery symbol otel_thread_ctx_nodejs_v1 stays in extern "C" at file scope so a reader can find it by name in the dd_pprof.node dynsym table. Records use a flexible-array tail, are right-sized to the encoded payload (36-byte attrs_data floor, ×2 growth on append, 612-byte cap), and realloc-on-append updates the wrapper's record_ pointer in place so every async-context frame holding the same reference observes the new buffer. binding.gyp adds -mtls-dialect=gnu2 on x86_64 Linux (TLSDESC; arm64 needs no flag). Compiles across Node 18–26 (V8 ABI guards) and MSVC. TS layer (ts/src/otel-thread-ctx.ts), Linux-only with no-op stubs elsewhere: a ThreadContext class constructed with (traceId, spanId, attributes?) and re-installed per async-context fire. Instance methods enter()/run(fn) route through the prototype so only a real ThreadContext can enter the AsyncLocalStorage; appendAttributes(), isTruncated(), debugBytes(). Module-level getContext(), clearContext(), and getProcessContextAttributes(keys) — the frozen OTEP-4719 snapshot (schema version, uint8-key→name map, V8 layout constants) a reader needs. Surfaced on the package root as require('@datadog/pprof'). otelThreadCtx. Tests: a mocha suite (skipped on non-Linux) covering construction, wire encoding incl. multibyte UTF-8 truncation, the cap and isTruncated, in-place append vs realloc, async propagation, enter/run/clearContext lifecycle, getProcessContextAttributes shape, and a readelf check that the TLS symbol is exported correctly. A scripts/docker/ harness + `npm run test:docker` runs the full suite in a Linux container from any host. Verified 158 passing.
Overall package sizeSelf size: 2.39 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | pprof-format | 2.2.2 | 500.53 kB | 500.53 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
…365) The lint job ran `yarn` with no `yarn.lock` in the repo, so yarn ignored package-lock.json and resolved every dependency fresh from the package.json ranges. prettier isn't a direct dependency — it comes in transitively via gts (^3.6.2) — so CI floated to the newest prettier (3.9.4), while local development pins 3.8.1 through package-lock.json. prettier 3.9.x changed how it wraps union return types (collapsing the leading-`|` multiline form 3.8.1 produces), so `gts check` failed in CI on heap-profiler.ts / heap-profiler-bindings.ts even though the code was correctly formatted for the pinned prettier — and the failure never reproduced locally. Switch install to `npm ci` (and the script to `npm run lint`) so CI uses the exact package-lock.json versions the repo is developed against. This fixes the prettier drift and prevents any future lockless-yarn float for every other tool in the lint chain.
18c2345 to
81a5f31
Compare
A JS Map always uses the regular OrderedHashMap as its backing table: V8's Map constructor hardcodes AllocateOrderedHashMap(), and the only path that could install a SmallOrderedHashMap (OrderedHashMapHandler / AdjustRepresentation) is test-only, never used by the JSMap/JSSet builtins. We only ever read AsyncContextFrame (CPED) maps, which are ordinary JS Maps, so the SmallOrderedHashMap handling was dead code. Drop the SmallOrderedHashMapLayout struct, IsSmallOrderedHashMap header sniffing, and the now-unnecessary templating on FindEntryByHash / FindValueByHash. The public GetValueFromMap signature is unchanged. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The v5.x lockfile carried newer versions than package.json declared — a desync that lockless `yarn` never validated but `npm ci` (now used by the lint job) rejects: - @types/node: package.json 25.9.1 vs lock 25.9.2 - tmp: package.json 0.2.6 vs lock 0.2.7 Align package.json up to the already-locked versions rather than down: tmp 0.2.6 carries a High-severity advisory (GHSA-7c78-jf6q-g5cm) and the lock was already advanced to the fixed 0.2.7. This changes nothing about what gets installed; it only makes the manifest honest. Regenerating the lockfile also drops the stale root `hasInstallScript` flag (the install script was removed in #363) and realigns the eslint-plugin-n / semver range mirrors with package.json. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
IlyasShabi
approved these changes
Jul 3, 2026
jakebailey
reviewed
Jul 6, 2026
| "compile": "tsc -p .", | ||
| "fix": "gts fix", | ||
| "format": "clang-format --style file -i --glob='bindings/**/*.{h,hh,cpp,cc}'", | ||
| "install": "exit 0", |
There was a problem hiding this comment.
This seems to break things as on install, it seems as though npm tries to find binding.gyp but it is not included in the package.
There was a problem hiding this comment.
e.g. https://github.com/jakebailey/pprof-it/actions/runs/28768139023/job/85296355430
removed 1 package, and changed 37 packages in 4s
15 packages are looking for funding
npm error code 1
run `npm fund` for details
npm error path /home/runner/work/pprof-it/pprof-it/node_modules/@datadog/pprof
npm error command failed
npm error command sh -c node-gyp rebuild
npm error gyp info it worked if it ends with ok
npm error gyp info using node-gyp@12.2.0
npm error gyp info using node@24.18.0 | linux | x64
npm error gyp info find Python using Python version 3.12.3 found at "/usr/bin/python3"
npm error gyp http GET https://nodejs.org/download/release/v24.18.0/node-v24.18.0-headers.tar.gz
npm error gyp http 200 https://nodejs.org/download/release/v24.18.0/node-v24.18.0-headers.tar.gz
npm error gyp http GET https://nodejs.org/download/release/v24.18.0/SHASUMS256.txt
npm error gyp http 200 https://nodejs.org/download/release/v24.18.0/SHASUMS256.txt
npm error gyp info spawn /usr/bin/python3
npm error gyp info spawn args [
npm error gyp info spawn args '/opt/hostedtoolcache/node/24.18.0/x64/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
npm error gyp info spawn args 'binding.gyp',
npm error gyp info spawn args '-f',
npm error gyp info spawn args 'make',
npm error gyp info spawn args '-I',
npm error gyp info spawn args '/home/runner/work/pprof-it/pprof-it/node_modules/@datadog/pprof/build/config.gypi',
npm error gyp info spawn args '-I',
npm error gyp info spawn args '/opt/hostedtoolcache/node/24.18.0/x64/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
npm error gyp info spawn args '-I',
npm error gyp info spawn args '/home/runner/.cache/node-gyp/24.18.0/include/node/common.gypi',
npm error gyp info spawn args '-Dlibrary=shared_library',
npm error gyp info spawn args '-Dvisibility=default',
npm error gyp info spawn args '-Dnode_root_dir=/home/runner/.cache/node-gyp/24.18.0',
npm error gyp info spawn args '-Dnode_gyp_dir=/opt/hostedtoolcache/node/24.18.0/x64/lib/node_modules/npm/node_modules/node-gyp',
npm error gyp info spawn args '-Dnode_lib_file=/home/runner/.cache/node-gyp/24.18.0/<(target_arch)/node.lib',
npm error gyp info spawn args '-Dmodule_root_dir=/home/runner/work/pprof-it/pprof-it/node_modules/@datadog/pprof',
npm error gyp info spawn args '-Dnode_engine=v8',
npm error gyp info spawn args '--depth=.',
npm error gyp info spawn args '--no-parallel',
npm error gyp info spawn args '--generator-output',
npm error gyp info spawn args 'build',
npm error gyp info spawn args '-Goutput_dir=.'
npm error gyp info spawn args ]
npm error gyp: binding.gyp not found (cwd: /home/runner/work/pprof-it/pprof-it/node_modules/@datadog/pprof) while trying to load binding.gyp
npm error gyp ERR! configure error
npm error gyp ERR! stack Error: `gyp` failed with exit code: 1
npm error gyp ERR! stack at ChildProcess.<anonymous> (/opt/hostedtoolcache/node/24.18.0/x64/lib/node_modules/npm/node_modules/node-gyp/lib/configure.js:317:18)
npm error gyp ERR! stack at ChildProcess.emit (node:events:509:28)
npm error gyp ERR! stack at ChildProcess._handle.onexit (node:internal/child_process:295:12)
npm error gyp ERR! System Linux 6.17.0-1018-azure
npm error gyp ERR! command "/opt/hostedtoolcache/node/24.18.0/x64/bin/node" "/opt/hostedtoolcache/node/24.18.0/x64/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
npm error gyp ERR! cwd /home/runner/work/pprof-it/pprof-it/node_modules/@datadog/pprof
npm error gyp ERR! node -v v24.18.0
npm error gyp ERR! node-gyp -v v12.2.0
npm error gyp ERR! $npm_package_name @datadog/pprof
npm error gyp ERR! $npm_package_version 5.16.0
npm error gyp ERR! not ok
npm error A complete log of this run can be found in: /home/runner/.npm/_logs/2026-07-06T04_37_24_657Z-debug-0.log
Author
There was a problem hiding this comment.
Ah, oops, I should have found that PR!
Member
There was a problem hiding this comment.
I already feared this might happen due to npm automatically doing this. I will try to think about alternatives that work everywhere
BridgeAR
added a commit
that referenced
this pull request
Jul 7, 2026
Dropping the no-op install script in #363 stopped Yarn Berry's YN0007 warning, but it let npm's publish-time normalization take over: with binding.gyp present in the dev tree (and excluded from the tarball) and no install/preinstall script, npm synthesizes "gypfile": true and "install": "node-gyp rebuild" into the published manifest. Consumers on npm then run node-gyp rebuild against a binding.gyp that isn't in the package and the install fails, which shipped broken in 5.16.0. Setting "gypfile": false suppresses that implicit hook while keeping the manifest free of lifecycle scripts, so both npm and Yarn Berry install the prebuilt binaries without a build step. The regression test now also pins gypfile:false alongside the no-lifecycle-scripts assertion. Fixes: #364 (review)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New features
Improvements
Bug fixes
Other (build, dev)