diff --git a/.github/dependabot.yml b/.github/dependabot.yml index cdf3668e..931c899d 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -21,6 +21,12 @@ updates: # Re-enable once typescript-eslint ships TS >=7 support. - dependency-name: "typescript" update-types: ["version-update:semver-major"] + # mocha 12 is ESM-only and requires Node ^20.19 || >=22.12, but we still + # build and test on Node 18. Its lib/cli/options.cjs require()s an ESM + # module, which fails with ERR_REQUIRE_ESM on Node 18. + # Re-enable once we drop Node 18 support. + - dependency-name: "mocha" + update-types: ["version-update:semver-major"] versioning-strategy: "increase" labels: - dependabot diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d31dccb3..30a51876 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -49,9 +49,34 @@ jobs: run: | content=`cat ./package.json | tr '\n' ' '` echo "json=$content" >> $GITHUB_OUTPUT - - run: | - git tag v${{ fromJson(steps.pkg.outputs.json).version }} - git push https://x-access-token:${{ steps.octo-sts.outputs.token }}@github.com/${{ github.repository }}.git v${{ fromJson(steps.pkg.outputs.json).version }} + - name: Tag release + run: | + version="${{ fromJson(steps.pkg.outputs.json).version }}" + remote="https://x-access-token:${{ steps.octo-sts.outputs.token }}@github.com/${{ github.repository }}.git" + # Idempotent, so that a rerun after a partial failure can still + # repair the tag and the release notes. + if git ls-remote --tags "$remote" "v$version" | grep -q "v$version"; then + echo "Tag v$version already exists, skipping" + else + git tag "v$version" + git push "$remote" "v$version" + fi + - name: Release notes + env: + GH_TOKEN: ${{ steps.octo-sts.outputs.token }} + VERSION: ${{ fromJson(steps.pkg.outputs.json).version }} + run: | + # The release notes are the body of the proposal PR, looked up by its + # head branch. This resolves even after the branch is deleted on merge. + notes="$RUNNER_TEMP/v$VERSION.md" + gh pr view "v$VERSION-proposal" --json body --jq .body > "$notes" + # v5.x is currently the only release line. Once a second one ships, + # --latest has to be decided per branch rather than hardcoded. + if gh release view "v$VERSION" > /dev/null 2>&1; then + gh release edit "v$VERSION" --title "$VERSION" -F "$notes" --latest + else + gh release create "v$VERSION" --target "${{ github.ref_name }}" --title "$VERSION" -F "$notes" --latest + fi publish_dev: needs: build diff --git a/benchmark/name-caching.js b/benchmark/name-caching.js new file mode 100644 index 00000000..bb17eb9f --- /dev/null +++ b/benchmark/name-caching.js @@ -0,0 +1,108 @@ +'use strict' + +const { Script } = require('vm') +const { isMainThread } = require('worker_threads') +const { TimeProfiler } = require('../out/src/time-profiler-bindings') + +const scriptCount = Number(process.env.SCRIPT_COUNT || '500') +const functionsPerScript = Number(process.env.FUNCTIONS_PER_SCRIPT || '8') +const rounds = Number(process.env.ROUNDS || '8') +const work = Number(process.env.WORK || '1000') +const lines = Number(process.env.LINES || '32') +const iterations = Number(process.env.ITERATIONS || '9') +const warmup = Number(process.env.WARMUP || '3') +const nonAsciiNames = process.env.NON_ASCII_NAMES === '1' + +function compileScript (scriptIndex) { + const handlers = [] + const calls = [] + + for (let i = 0; i < functionsPerScript; i++) { + const name = `${nonAsciiNames ? '处理器' : 'handler'}_${scriptIndex}_${i}` + const statements = Array.from({ length: lines }, (_, line) => + `for (let k = 0; k < ${work}; k++) total += Math.sqrt(k * n + ${i + line})` + ) + handlers.push(` + function ${name}(n) { + let total = 0 + ${statements.join('\n')} + return total + }`) + calls.push(`total += ${name}(n + ${i})`) + } + + return new Script(` + (() => { + ${handlers.join('\n')} + return function ${nonAsciiNames ? '运行' : 'run'}_${scriptIndex}(n) { + let total = 0 + ${calls.join('\n')} + return total + } + })() + `, { filename: `/opt/${nonAsciiNames ? '服务' : 'service'}/dist/modules/module-${scriptIndex}.js` }) + .runInThisContext() +} + +function countNodes (node) { + let count = 1 + for (const child of node.children) count += countNodes(child) + return count +} + +function median (values) { + return [...values].sort((a, b) => a - b)[Math.floor(values.length / 2)] +} + +const scripts = Array.from({ length: scriptCount }, (_, i) => compileScript(i)) +const stopMicros = [] +const nanosPerNode = [] +const nodeCounts = [] + +for (let iteration = 0; iteration < warmup + iterations; iteration++) { + const profiler = new TimeProfiler({ + intervalMicros: 50, + durationMillis: 60000, + lineNumbers: true, + withContexts: false, + workaroundV8Bug: false, + collectCpuTime: false, + collectAsyncId: false, + isMainThread, + useCPED: false + }) + profiler.start() + + let result = 0 + for (let round = 0; round < rounds; round++) { + for (const run of scripts) result += run(round + 1) + } + + const start = process.hrtime.bigint() + const profile = profiler.stop(false) + const elapsed = Number(process.hrtime.bigint() - start) + profiler.dispose() + + if (!Number.isFinite(result)) throw new Error('benchmark workload failed') + if (iteration < warmup) continue + + const nodes = countNodes(profile.topDownRoot) + stopMicros.push(elapsed / 1000) + nanosPerNode.push(elapsed / nodes) + nodeCounts.push(nodes) +} + +console.log(JSON.stringify({ + name: 'profile-name-caching', + scripts: scriptCount, + functionsPerScript, + lines, + nonAsciiNames, + rounds, + work, + iterations, + warmup, + medianNodes: median(nodeCounts), + medianStopMicros: median(stopMicros), + medianNanosPerNode: median(nanosPerNode) +})) diff --git a/binding.gyp b/binding.gyp index 8b918472..c264af5c 100644 --- a/binding.gyp +++ b/binding.gyp @@ -109,6 +109,30 @@ }, } ], + ["address_sanitizer != 'true' and thread_sanitizer != 'true'", { + 'xcode_settings': { + 'GCC_GENERATE_DEBUGGING_SYMBOLS': 'NO', + 'GCC_SYMBOLS_PRIVATE_EXTERN': 'YES', # -fvisibility=hidden + 'GCC_INLINES_ARE_PRIVATE_EXTERN': 'YES', + 'DEAD_CODE_STRIPPING': 'YES', # -dead_strip + }, + "conditions": [ + ["OS == 'linux'", { + "cflags": [ + "-fvisibility=hidden", + "-ffunction-sections", + "-fdata-sections", + ], + "cflags_cc": ["-fvisibility-inlines-hidden"], + "ldflags": [ "-Wl,--gc-sections" ], + }], + ["OS == 'win'", { + 'msvs_settings': { + 'VCLinkerTool': { 'OptimizeReferences': 2 }, # /OPT:REF + }, + }], + ], + }], ["address_sanitizer == 'true' and OS == 'mac'", { 'xcode_settings': { 'OTHER_CFLAGS+': [ diff --git a/bindings/otel-thread-ctx.cc b/bindings/otel-thread-ctx.cc index b7c7dfd4..5c43eb93 100644 --- a/bindings/otel-thread-ctx.cc +++ b/bindings/otel-thread-ctx.cc @@ -48,6 +48,24 @@ #include #include +// Byte offset, within an object created from an API template such as +// ThreadContext's, of the pointer stored in internal field 0. Different +// in some Node.js versions. +#if NODE_MAJOR_VERSION >= 23 +constexpr int kRecordSlotOffset = + v8::internal::Internals::kJSAPIObjectWithEmbedderSlotsHeaderSize + + v8::internal::Internals::kEmbedderDataSlotExternalPointerOffset; +#elif NODE_MAJOR_VERSION >= 22 +constexpr int kRecordSlotOffset = + v8::internal::Internals::kJSObjectHeaderSize + + v8::internal::Internals::kEmbedderDataSlotExternalPointerOffset; +#else +// not used +constexpr int kRecordSlotOffset = 0; +#endif +static_assert(kRecordSlotOffset >= 0 && kRecordSlotOffset <= UINT8_MAX, + "record_slot_offset must fit its uint8 field"); + // Single thread-local read from outside the process via TLSDESC. It // identifies, for the current V8 isolate's thread: // @@ -60,13 +78,16 @@ // AsyncContextFrame map (`als_handle`), // - that instance's JS identity hash (`als_identity_hash`), so the // reader can restrict the lookup to a single hash bucket. +// - the byte offset of internal field 0 within the wrapper JSObject the +// frame maps our key to (`record_slot_offset`), which holds the record +// pointer. // - the (per-isolate) tagged address of the `undefined` singleton // (`undefined_addr`). After looking up the value for our ALS key in // the ACF map, the reader can compare against this to skip the // JSObject / internal-field-0 dereference when no ThreadContext is // currently attached; without it, a reader walking through undefined // would have to rely on structural validation of the bytes at -// undefined+js_object_record_offset to detect the absence. +// undefined+ to detect the absence. // // Layout is part of the reader ABI: see the README "Discovery contract" // section and the static_asserts below. @@ -75,9 +96,11 @@ using v8::Global; using v8::Object; struct otel_thread_ctx_nodejs_v1_t { - v8::internal::Address* cped_slot; // offset 0 - Global als_handle; // offset sizeof(void*); 1 V8 ptr - int als_identity_hash; // offset 2 * sizeof(void*); 4 + 4 pad + v8::internal::Address* cped_slot; // offset 0 + Global als_handle; // offset sizeof(void*); 1 V8 ptr + int als_identity_hash; // offset 2 * sizeof(void*) + uint8_t record_slot_offset = kRecordSlotOffset; // 2 * sizeof(void*) + 4 + uint8_t reserved[3] = {}; // 2 * sizeof(void*) + 5 v8::internal::Address undefined_addr; // offset 3 * sizeof(void*); tagged }; @@ -100,9 +123,12 @@ static_assert(offsetof(otel_thread_ctx_nodejs_v1_t, als_handle) == static_assert(offsetof(otel_thread_ctx_nodejs_v1_t, als_identity_hash) == 2 * sizeof(void*), "als_identity_hash must immediately follow als_handle"); +static_assert(offsetof(otel_thread_ctx_nodejs_v1_t, record_slot_offset) == + 2 * sizeof(void*) + 4, + "record_slot_offset must immediately follow als_identity_hash"); static_assert(offsetof(otel_thread_ctx_nodejs_v1_t, undefined_addr) == 3 * sizeof(void*), - "undefined_addr must follow als_identity_hash + padding"); + "undefined_addr must follow record_slot_offset + reserved"); namespace dd { namespace { @@ -880,8 +906,13 @@ void StoreAls(const FunctionCallbackInfo& args) { // Cache the per-isolate undefined singleton's tagged address. Undefined // is a read-only-roots heap object, never moves, so a cached numeric // address is fine — no Global<> tracking needed. +#if NODE_MAJOR_VERSION >= 22 otel_thread_ctx_nodejs_v1.undefined_addr = - reinterpret_cast(*v8::Undefined(isolate)); + v8::internal::ValueHelper::ValueAsAddress(*v8::Undefined(isolate)); +#else + // Unreachable from JS; nonzero for the cleanup-hook bookkeeping. + otel_thread_ctx_nodejs_v1.undefined_addr = 1; +#endif // Write `cped_slot` last with signal fence + volatile. It is what a reader // tests before it dereferences anything, so publishing it after every other @@ -901,40 +932,60 @@ void GetStoredAlsHash(const FunctionCallbackInfo& args) { Integer::New(isolate, otel_thread_ctx_nodejs_v1.als_identity_hash)); } -// V8 layout constants captured at addon-compile time from the same V8 -// headers Node bundles. Published via the discovery contract so an -// out-of-process reader can decode V8's JSObject / internal hashmap -// layout without doing its own V8-internal-symbol lookups for the -// pointer-compression / sandbox state. Note that nothing published here -// describes our own wrapper: internal field 0 points straight at the -// record, so the reader needs no offset of ours to reach it. +// The nodejs_v1 discovery schema does not publish V8's object layout; it +// fixes it, presuming the V8 Node.js builds by default: 64-bit, pointer +// compression off, sandbox off. These assertions check that presumption +// against the V8 headers we are compiled with, so a build not matching +// the schema will fail to compile. +// +// Each value the reader needs equals one of V8's public constants: +// tagged size (8) kApiTaggedSize +// JSMap table offset (0x18) kJSObjectHeaderSize, because JSCollection +// adds a single `table` field to JSObject +// (deps/v8/src/objects/js-collection.h) +// OrderedHashMap header kFixedArrayHeaderSize, because +// size (0x10) OrderedHashTable derives from FixedArray +// (deps/v8/src/objects/ordered-hash-table.h) +static_assert(v8::internal::kApiTaggedSize == 8, + "nodejs_v1 assumes a V8 built without pointer compression"); +static_assert(v8::internal::Internals::kJSObjectHeaderSize == 0x18, + "unexpected V8 JSObject header size"); +static_assert(v8::internal::Internals::kFixedArrayHeaderSize == 0x10, + "unexpected V8 FixedArray header size"); #if NODE_MAJOR_VERSION >= 22 -constexpr int JS_OBJECT_RECORD_OFFSET = - v8::internal::Internals::kJSObjectHeaderSize + +// Node < 22 lacks this constant; the contract is unusable there anyway, +// as it has no ContinuationPreservedEmbedderData either (see StoreAls). +constexpr int kEmbedderDataSlotExternalPtrOffset = v8::internal::Internals::kEmbedderDataSlotExternalPointerOffset; +static_assert(kEmbedderDataSlotExternalPtrOffset == 0, + "nodejs_v1 assumes a V8 built without the sandbox"); +#endif + +// Whether internal field 0 of an object created from an API template really +// sits at kRecordSlotOffset. Set the field on a probe object and read it back +// at the offset. +bool RecordSlotOffsetHolds(v8::Isolate* isolate, + v8::Local context) { +#if NODE_MAJOR_VERSION >= 22 + v8::HandleScope scope(isolate); + v8::Local tpl = v8::ObjectTemplate::New(isolate); + tpl->SetInternalFieldCount(1); + v8::Local probe; + if (!tpl->NewInstance(context).ToLocal(&probe)) return false; + // Any aligned address will do; this one is ours and can't collide. + static int marker; + SetAlignedPointerInInternalField(probe, 0, &marker); + const char* object = reinterpret_cast( + v8::internal::ValueHelper::ValueAsAddress(*probe) - + v8::internal::kHeapObjectTag); + void* at_offset; + memcpy(&at_offset, object + kRecordSlotOffset, sizeof(at_offset)); + return at_offset == ▮ #else -// Node < 22 lacks kEmbedderDataSlotExternalPointerOffset. The discovery -// contract isn't usable on these versions (no ContinuationPreservedEmbedderData -// either — see StoreAls), so this value is published only to keep the -// addon's exported surface consistent across Node majors. A would-be -// reader cannot reach a live record through it. -constexpr int JS_OBJECT_RECORD_OFFSET = 0; + // No ContinuationPreservedEmbedderData, so nothing to publish anyway. + return false; #endif -constexpr int TAGGED_SIZE = v8::internal::kApiTaggedSize; - -// V8 JSMap layout: kTableOffset within the JSMap object holds a tagged -// pointer to the backing OrderedHashMap table. Not exposed in V8's -// public headers; kept in sync with -// deps/v8/src/objects/js-collection.h (JSCollection::kTableOffset) -// and the torque-generated JSCollection layout. -constexpr int JS_MAP_TABLE_OFFSET = 0x18; - -// V8 OrderedHashMap layout: the on-heap table starts with a 16-byte -// header before the element_count / deleted_element_count / -// number_of_buckets fields. Not exposed in V8's public headers; kept in -// sync with deps/v8/src/objects/ordered-hash-table.h -// (OrderedHashTable base layout). -constexpr int ORDERED_HASH_MAP_HEADER_SIZE = 0x10; +} } // namespace @@ -943,20 +994,14 @@ void OtelThreadCtx::Init(Local exports) { NODE_SET_METHOD(exports, "otelThreadCtxStoreAls", StoreAls); NODE_SET_METHOD(exports, "otelThreadCtxGetStoredAlsHash", GetStoredAlsHash); - Isolate* isolate = Isolate::GetCurrent(); - Local ctx = isolate->GetCurrentContext(); - auto publish_int = [&](const char* name, int value) { - exports - ->Set(ctx, - String::NewFromUtf8(isolate, name).ToLocalChecked(), - Integer::New(isolate, value)) - .FromJust(); - }; - publish_int("otelThreadCtxJsMapTableOffset", JS_MAP_TABLE_OFFSET); - publish_int("otelThreadCtxOrderedHashMapHeaderSize", - ORDERED_HASH_MAP_HEADER_SIZE); - publish_int("otelThreadCtxTaggedSize", TAGGED_SIZE); - publish_int("otelThreadCtxJsObjectRecordOffset", JS_OBJECT_RECORD_OFFSET); + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Local context = isolate->GetCurrentContext(); + exports + ->Set(context, + v8::String::NewFromUtf8Literal( + isolate, "otelThreadCtxRecordSlotOffsetHolds"), + v8::Boolean::New(isolate, RecordSlotOffsetHolds(isolate, context))) + .FromJust(); } } // namespace dd diff --git a/bindings/translate-heap-profile.cc b/bindings/translate-heap-profile.cc index bd97d549..ad4ed0e0 100644 --- a/bindings/translate-heap-profile.cc +++ b/bindings/translate-heap-profile.cc @@ -170,11 +170,6 @@ std::shared_ptr TranslateAllocationProfileToCpp( return new_node; } -v8::Local TranslateAllocationProfile( - v8::AllocationProfile::Node* node) { - return HeapProfileTranslator().TranslateAllocationProfile(node); -} - v8::Local TranslateAllocationProfile( v8::AllocationProfile::Node* node, const AllocationProfileNodeStatsMap* allocation_stats) { diff --git a/bindings/translate-heap-profile.hh b/bindings/translate-heap-profile.hh index e62f14f4..502c9195 100644 --- a/bindings/translate-heap-profile.hh +++ b/bindings/translate-heap-profile.hh @@ -41,8 +41,6 @@ std::shared_ptr TranslateAllocationProfileToCpp( v8::AllocationProfile::Node* node); v8::Local TranslateAllocationProfile(Node* node); -v8::Local TranslateAllocationProfile( - v8::AllocationProfile::Node* node); v8::Local TranslateAllocationProfile( v8::AllocationProfile::Node* node, const AllocationProfileNodeStatsMap* allocation_stats); diff --git a/bindings/translate-time-profile.cc b/bindings/translate-time-profile.cc index 4a915a6f..fb997da2 100644 --- a/bindings/translate-time-profile.cc +++ b/bindings/translate-time-profile.cc @@ -344,6 +344,8 @@ class TimeProfileTranslator : ProfileTranslator { unsigned int hitLineCount = node->GetHitLineCount(); unsigned int hitCount = node->GetHitCount(); + auto name = node->GetFunctionName(); + auto scriptName = node->GetScriptResourceName(); auto scriptId = NewInteger(node->GetScriptId()); if (hitLineCount > 0) { std::vector entries(hitLineCount); @@ -352,8 +354,8 @@ class TimeProfileTranslator : ProfileTranslator { for (const v8::CpuProfileNode::LineTick entry : entries) { Set(children, index++, - CreateTimeNode(node->GetFunctionName(), - node->GetScriptResourceName(), + CreateTimeNode(name, + scriptName, scriptId, NewInteger(entry.line), // V8 14+ (Node.js 25+) added column field to LineTick struct @@ -372,8 +374,8 @@ class TimeProfileTranslator : ProfileTranslator { children = NewArray(count + 1); Set(children, index++, - CreateTimeNode(node->GetFunctionName(), - node->GetScriptResourceName(), + CreateTimeNode(name, + scriptName, scriptId, NewInteger(node->GetLineNumber()), NewInteger(node->GetColumnNumber()), @@ -387,17 +389,21 @@ class TimeProfileTranslator : ProfileTranslator { for (int32_t i = 0; i < count; i++) { Set(children, index++, - TranslateLineNumbersTimeProfileNode(node, node->GetChild(i))); + TranslateLineNumbersTimeProfileNode( + name, scriptName, scriptId, node->GetChild(i))); }; return children; } v8::Local TranslateLineNumbersTimeProfileNode( - const v8::CpuProfileNode* parent, const v8::CpuProfileNode* node) { - return CreateTimeNode(parent->GetFunctionName(), - parent->GetScriptResourceName(), - NewInteger(parent->GetScriptId()), + v8::Local name, + v8::Local scriptName, + v8::Local scriptId, + const v8::CpuProfileNode* node) { + return CreateTimeNode(name, + scriptName, + scriptId, NewInteger(node->GetLineNumber()), NewInteger(node->GetColumnNumber()), zero, diff --git a/package-lock.json b/package-lock.json index 100a76d9..2a6d4ec5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,32 +1,32 @@ { "name": "@datadog/pprof", - "version": "5.19.0", + "version": "5.20.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@datadog/pprof", - "version": "5.19.0", + "version": "5.20.0", "license": "Apache-2.0", "dependencies": { "node-gyp-build": "^4.8.4", - "pprof-format": "^2.3.1", + "pprof-format": "^2.3.2", "source-map": "^0.8.0" }, "devDependencies": { "@types/mocha": "^10.0.1", - "@types/node": "26.4.1", + "@types/node": "26.6.3", "@types/semver": "^7.8.0", "@types/sinon": "^22.0.0", "@types/tmp": "^0.2.3", "clang-format": "^1.8.0", "codecov": "^3.8.3", "deep-copy": "^1.4.2", - "eslint-plugin-n": "^18.3.0", + "eslint-plugin-n": "^18.4.0", "gts": "^7.0.0", "js-green-licenses": "^4.0.0", "mocha": "^11.8.0", - "nan": "^2.28.0", + "nan": "^2.29.0", "nyc": "^18.0.0", "semver": "^7.8.5", "sinon": "^22.1.0", @@ -962,13 +962,13 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "26.4.1", - "resolved": "https://registry.npmjs.org/@types/node/-/node-26.4.1.tgz", - "integrity": "sha512-k97ENvZWtvA6yqz5/FS6a7duDgOPEeOQOc2iKS/nY6mX6qJUKtLnWzQS+Xj6tXweyj6ZcTAK2Qecetnvi9nCLA==", + "version": "26.6.3", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.6.3.tgz", + "integrity": "sha512-dsqMQQoeTLqu9wynDD00q573mNzso3IdQOAfHRJqLCcmCFPoGo9A1bDpUcv/9tnKpErQWv9uKeGfl37EIS02Yg==", "dev": true, "license": "MIT", "dependencies": { - "undici-types": "~8.3.0" + "undici-types": "~8.9.0" } }, "node_modules/@types/normalize-package-data": { @@ -2245,9 +2245,9 @@ } }, "node_modules/eslint-plugin-n": { - "version": "18.3.0", - "resolved": "https://registry.npmjs.org/eslint-plugin-n/-/eslint-plugin-n-18.3.0.tgz", - "integrity": "sha512-cPVguuDe6DrIPb/qUXHf8P89MaVTUmiYWwpt5gX5AILsvRIiZAxMFXcFR6QHYBksqKJpjfUBlL/RleCJUWcD7w==", + "version": "18.4.0", + "resolved": "https://registry.npmjs.org/eslint-plugin-n/-/eslint-plugin-n-18.4.0.tgz", + "integrity": "sha512-TEm6Nqn9+l7EWfJXOL6pmrEdpiM2ZTgo4fNMRDvBTM70jeQ7x3X3Akcx+sRCIRfpcSVwMypH3ZAA33PLyNKmYw==", "dev": true, "license": "MIT", "dependencies": { @@ -2258,6 +2258,7 @@ "globals": "^15.11.0", "globrex": "^0.1.2", "ignore": "^5.3.2", + "js-yaml": "^5.4.1", "semver": "^7.6.3" }, "engines": { @@ -2280,6 +2281,36 @@ } } }, + "node_modules/eslint-plugin-n/node_modules/argparse": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", + "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", + "dev": true, + "license": "Python-2.0" + }, + "node_modules/eslint-plugin-n/node_modules/js-yaml": { + "version": "5.4.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-5.4.2.tgz", + "integrity": "sha512-m+aqu+LwO1O6sIopafj8HUVl5aawITwZQe/yHpMCKjaWBaA/d07B/QdMb3529REftiU+RMMHL3Vlsw3hON7vWg==", + "dev": true, + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/nodeca" + } + ], + "license": "MIT", + "dependencies": { + "argparse": "^2.0.1" + }, + "bin": { + "js-yaml": "bin/js-yaml.mjs" + } + }, "node_modules/eslint-plugin-prettier": { "version": "5.5.5", "resolved": "https://registry.npmjs.org/eslint-plugin-prettier/-/eslint-plugin-prettier-5.5.5.tgz", @@ -4290,9 +4321,9 @@ "license": "ISC" }, "node_modules/nan": { - "version": "2.28.0", - "resolved": "https://registry.npmjs.org/nan/-/nan-2.28.0.tgz", - "integrity": "sha512-fTsDz99OTq2sVePhGdp4qQhggZFtKr64ZNVyVajRKtMOkJxYekplBh577PiJB12v/D3s2E5cGtOI45LWp6rnLQ==", + "version": "2.29.0", + "resolved": "https://registry.npmjs.org/nan/-/nan-2.29.0.tgz", + "integrity": "sha512-GlGk3HIvitbvs+LT3g6XUP1kpirKNvmDFwF/bmo6XNWSb/eYEs/O4bfgIEIXCZ+lIOTS5xNwDvSGMw6FJdAhtA==", "dev": true, "license": "MIT" }, @@ -5062,9 +5093,9 @@ } }, "node_modules/pprof-format": { - "version": "2.3.1", - "resolved": "https://registry.npmjs.org/pprof-format/-/pprof-format-2.3.1.tgz", - "integrity": "sha512-y51Z83qG2vEQBACPu6lkGFREVkHwQaCaNDdSFEMLIqSo3bmpADsbP6J3F2SSk7tYB741oTQ9Kt5YAdQsmiCRkA==", + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/pprof-format/-/pprof-format-2.3.2.tgz", + "integrity": "sha512-fcMM+sgKMOveSurz7i0BMHfy1hHNnI9+VWjd4kdclqQjSTuiHWaP69DGwvkw3zcLkM/F76LhVIKvPyrxmp6iiw==", "license": "MIT" }, "node_modules/prelude-ls": { @@ -6322,9 +6353,9 @@ } }, "node_modules/undici-types": { - "version": "8.3.0", - "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.3.0.tgz", - "integrity": "sha512-j375ScV60dom+YkPFIfTLcOiPxkN/buHz5GobjLhixFuANaNs3C9l4GmrWqejgXWJ7BbJcFYpTEUkS1Ge8bpZQ==", + "version": "8.9.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.9.0.tgz", + "integrity": "sha512-KTDyRTYX8sWmKXAikPHHSyc63CRPETMctyjKFupcC6OBLXT3xsN0e9aF7m+mIXutFWpUXuedtowG7iLOzp0kQg==", "dev": true, "license": "MIT" }, diff --git a/package.json b/package.json index 5f2a3395..5578f6c8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@datadog/pprof", - "version": "5.19.0", + "version": "5.20.0", "description": "pprof support for Node.js", "repository": { "type": "git", @@ -38,23 +38,23 @@ "license": "Apache-2.0", "dependencies": { "node-gyp-build": "^4.8.4", - "pprof-format": "^2.3.1", + "pprof-format": "^2.3.2", "source-map": "^0.8.0" }, "devDependencies": { "@types/mocha": "^10.0.1", - "@types/node": "26.4.1", + "@types/node": "26.6.3", "@types/semver": "^7.8.0", "@types/sinon": "^22.0.0", "@types/tmp": "^0.2.3", "clang-format": "^1.8.0", "codecov": "^3.8.3", "deep-copy": "^1.4.2", - "eslint-plugin-n": "^18.3.0", + "eslint-plugin-n": "^18.4.0", "gts": "^7.0.0", "js-green-licenses": "^4.0.0", "mocha": "^11.8.0", - "nan": "^2.28.0", + "nan": "^2.29.0", "nyc": "^18.0.0", "semver": "^7.8.5", "sinon": "^22.1.0", diff --git a/ts/src/otel-thread-ctx.ts b/ts/src/otel-thread-ctx.ts index d2a4c1e7..5ea2d725 100644 --- a/ts/src/otel-thread-ctx.ts +++ b/ts/src/otel-thread-ctx.ts @@ -48,10 +48,6 @@ import { export interface ProcessContextAttributes { readonly 'threadlocal.schema_version': 'nodejs_v1_dev'; readonly 'threadlocal.attribute_key_map': readonly string[]; - readonly 'threadlocal.js_object_record_offset': number; - readonly 'threadlocal.tagged_size': number; - readonly 'threadlocal.js_map_table_offset': number; - readonly 'threadlocal.ordered_hash_map_header_size': number; } /** @@ -142,23 +138,14 @@ interface Addon { threadContext: ThreadContextCtor; otelThreadCtxStoreAls(als: AsyncLocalStorage): void; otelThreadCtxGetStoredAlsHash(): number; - otelThreadCtxJsObjectRecordOffset: number; - otelThreadCtxTaggedSize: number; - otelThreadCtxJsMapTableOffset: number; - otelThreadCtxOrderedHashMapHeaderSize: number; + otelThreadCtxRecordSlotOffsetHolds: boolean; } const SCHEMA_VERSION = 'nodejs_v1_dev'; -// V8 layout constants the addon captured from the V8 headers Node bundles. -// On non-Linux these fall back to values matching Node's standard build -// (no V8 pointer compression, no sandbox); the reader is Linux-only per -// the OTEP anyway, so the fallbacks just keep processContextAttributes -// consistent in shape. -let JS_OBJECT_RECORD_OFFSET = 0x18; -let TAGGED_SIZE = 8; -let JS_MAP_TABLE_OFFSET = 0x18; -let ORDERED_HASH_MAP_HEADER_SIZE = 0x10; +// Why this process can't honor the schema, if it can't. Only meaningful on +// Linux, the one platform the reader contract covers. +let whyUnpublishable: () => string | undefined = () => undefined; /** {@inheritDoc ThreadContextCtor} */ export let ThreadContext: ThreadContextCtor; @@ -180,24 +167,29 @@ export let clearContext: () => void; export let _currentRecordBytes: () => Uint8Array | undefined = () => undefined; if (process.platform === 'linux') { - // eslint-disable-next-line @typescript-eslint/no-require-imports const findBinding = require('node-gyp-build'); const addon: Addon = findBinding(join(__dirname, '..', '..')); - JS_OBJECT_RECORD_OFFSET = addon.otelThreadCtxJsObjectRecordOffset; - TAGGED_SIZE = addon.otelThreadCtxTaggedSize; - JS_MAP_TABLE_OFFSET = addon.otelThreadCtxJsMapTableOffset; - ORDERED_HASH_MAP_HEADER_SIZE = addon.otelThreadCtxOrderedHashMapHeaderSize; ThreadContext = addon.threadContext; + whyUnpublishable = () => { + if (!isAsyncContextFrameActive()) { + return `async_context_frame support is unavailable: ${asyncContextFrameHint()}`; + } + if (!addon.otelThreadCtxRecordSlotOffsetHolds) { + return 'V8 does not place internal fields where the addon was built to expect them'; + } + return undefined; + }; + let als: AsyncLocalStorage | undefined; function ensureHook(): AsyncLocalStorage { if (als) return als; - if (!isAsyncContextFrameActive()) { + const reason = whyUnpublishable(); + if (reason) { throw new Error( - 'otel thread-ctx writer requires async_context_frame support, which is ' + - `unavailable: ${asyncContextFrameHint()}.`, + `otel thread-ctx writer can't publish on this Node: ${reason}.`, ); } als = new AsyncLocalStorage(); @@ -278,6 +270,11 @@ if (process.platform === 'linux') { export function getProcessContextAttributes( keys: string[], ): ProcessContextAttributes { + // A reader would find nothing or mis-walk, so don't declare the schema. + const reason = whyUnpublishable(); + if (reason) { + throw new Error(`can't declare ${SCHEMA_VERSION} on this Node: ${reason}.`); + } if (!Array.isArray(keys)) { throw new TypeError('keys must be an array of attribute names'); } @@ -298,9 +295,5 @@ export function getProcessContextAttributes( return Object.freeze({ 'threadlocal.schema_version': SCHEMA_VERSION, 'threadlocal.attribute_key_map': Object.freeze(keys.slice()), - 'threadlocal.js_object_record_offset': JS_OBJECT_RECORD_OFFSET, - 'threadlocal.tagged_size': TAGGED_SIZE, - 'threadlocal.js_map_table_offset': JS_MAP_TABLE_OFFSET, - 'threadlocal.ordered_hash_map_header_size': ORDERED_HASH_MAP_HEADER_SIZE, }) as ProcessContextAttributes; } diff --git a/ts/test/test-async-context-frame.ts b/ts/test/test-async-context-frame.ts index 54a0e970..f2cb7ebc 100644 --- a/ts/test/test-async-context-frame.ts +++ b/ts/test/test-async-context-frame.ts @@ -16,7 +16,7 @@ import {strict as assert} from 'assert'; import {AsyncLocalStorage} from 'node:async_hooks'; -import {fork} from 'node:child_process'; +import {fork, spawnSync} from 'node:child_process'; import {join} from 'node:path'; import {satisfies} from 'semver'; @@ -129,6 +129,38 @@ describe('isAsyncContextFrameActive', () => { }); }); +describe('getProcessContextAttributes', () => { + it('refuses to declare the schema without AsyncContextFrame', function () { + // The reader contract, and so this refusal, is Linux-only. + if (process.platform !== 'linux') return this.skip(); + // Nothing would ever write the CPED slot, so a reader told the schema is + // in use would find nothing. + const off = major >= 24 ? ['--no-async-context-frame'] : []; + const lib = JSON.stringify(join(__dirname, '..', 'src', 'otel-thread-ctx')); + const r = spawnSync( + process.execPath, + [ + ...off, + '-e', + `try { + require(${lib}).getProcessContextAttributes([]); + console.log('declared'); + } catch (e) { + console.log(e.message); + }`, + ], + {encoding: 'utf8', env: {...process.env, NODE_OPTIONS: ''}}, + ); + // Not the exit status: under the sanitizer jobs LeakSanitizer fails the + // child for leaks in Node's own `-e` startup path. + assert.match( + r.stdout, + /can't declare .* async_context_frame support is unavailable/, + r.stderr, + ); + }); +}); + // The detection asks whether the running storage is bound to its own store, // not merely whether the CPED slot holds a Map. These pin that difference: // without them, weakening the helper to a bare IsMap check would still pass diff --git a/ts/test/test-otel-thread-ctx.ts b/ts/test/test-otel-thread-ctx.ts index 372a5fff..61538c0b 100644 --- a/ts/test/test-otel-thread-ctx.ts +++ b/ts/test/test-otel-thread-ctx.ts @@ -940,20 +940,9 @@ function captureBytes(opts: { const pca = getProcessContextAttributes(keys); strictAssert.equal(pca['threadlocal.schema_version'], 'nodejs_v1_dev'); strictAssert.deepEqual(pca['threadlocal.attribute_key_map'], keys); - strictAssert.equal(pca['threadlocal.js_object_record_offset'], 0x18); - strictAssert.equal(pca['threadlocal.tagged_size'], 8); - strictAssert.equal(pca['threadlocal.js_map_table_offset'], 0x18); - strictAssert.equal( - pca['threadlocal.ordered_hash_map_header_size'], - 0x10, - ); strictAssert.deepEqual(Object.keys(pca).sort(), [ 'threadlocal.attribute_key_map', - 'threadlocal.js_map_table_offset', - 'threadlocal.js_object_record_offset', - 'threadlocal.ordered_hash_map_header_size', 'threadlocal.schema_version', - 'threadlocal.tagged_size', ]); }); @@ -976,6 +965,18 @@ function captureBytes(opts: { }); describe('discovery contract', () => { + it('places internal field 0 at the record slot offset it publishes', () => { + // The offset differs between V8 versions, so the addon checks at load + // time that it matches where V8 actually puts the field. + const addon = require('node-gyp-build')( + join(__dirname, '..', '..'), + ) as { + otelThreadCtxRecordSlotOffsetHolds: boolean; + }; + strictAssert.equal(addon.otelThreadCtxRecordSlotOffsetHolds, true); + assert.doesNotThrow(() => getProcessContextAttributes([])); + }); + it('exports otel_thread_ctx_nodejs_v1 as a TLS dynsym', function () { const addon = join( __dirname,