Port OTEP-4947 thread-context writer from polarsignals/custom-labels#347
Draft
szegedi wants to merge 4 commits into
Draft
Port OTEP-4947 thread-context writer from polarsignals/custom-labels#347szegedi wants to merge 4 commits into
szegedi wants to merge 4 commits into
Conversation
Ports the in-development OpenTelemetry thread-context writer that lives on the otel-thread-ctx-node branch of polarsignals/custom-labels (szegedi fork) into this project. The two codebases will likely diverge again later; for now this is a snapshot of the current state. Structurally: - bindings/otel-thread-ctx.cc/.hh: the native addon code, namespaced in `dd::` and exposed via OtelThreadCtx::Init(exports) called from binding.cc. The thread_local otel_thread_ctx_nodejs_v1 discovery symbol stays in extern "C" at file scope so it's exported by name through the dd_pprof.node dynsym table. - ts/src/otel-thread-ctx.ts: the runWithContext / enterWithContext / makeNamedContext API, loading the native addon via node-gyp-build like the rest of this project. - ts/test/test-otel-thread-ctx.ts: mocha port of the node:test suite. Skipped wholesale on non-Linux. - binding.gyp: adds bindings/otel-thread-ctx.cc to both target source lists and the -mtls-dialect=gnu2 cflag on x86_64 Linux (required by the OTEP-4947 spec; on arm64 TLSDESC is the only dynamic TLS model so no flag is needed). Verified by mocha against the built dd_pprof.node in a Linux container (Node 22 with --experimental-async-context-frame): 35 passing.
Mirrors the test:docker mechanism in custom-labels/js: a Dockerfile under scripts/docker/ extending node:24-bookworm with python3 and build-essential, plus a launcher script that builds the image (cached), mounts the repo read-only, copies it into /tmp/work inside the container, and runs `npm install && npm test`. The host tree is never modified (no stray node_modules/, build/, out/). Node 24 is used so the full test suite — including the new OTEP-4947 thread-context tests, which need AsyncContextFrame — runs without extra Node flags. Run via `npm run test:docker`.
|
Overall package sizeSelf size: 2.41 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
- Add a static_assert that offsetof(CtxWrap, record_) == sizeof(node::ObjectWrap), since that offset is part of the reader ABI. Restructure CtxWrap so record_, capacity_, and truncated_ live in a single public access section: C++ leaves cross-access-control field ordering implementation-defined, so splitting them would allow a conforming compiler to reorder the bookkeeping fields ahead of record_. - Add an acq_rel signal fence between the pointer swap and free() in the reallocate path. The pre-existing release fence only constrains prior writes; nothing was stopping the compiler from hoisting free() above the publication store, which would let a stopped reader follow self->record_ into freed memory. - Restore the [[unlikely]] annotation on the IsConstructCall() check. - Misc local cleanups (std::min/max in two spots, assert valid==1 after the memcpy instead of redundantly setting it).
The CtxWrap accessor that returns the raw record as a Uint8Array is only intended for tests and out-of-process-reader development. Naming it DebugBytes (and exposing it as wrap.debugBytes() on the JS prototype) makes that explicit at every call site.
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.
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.
The two codebases will diverge again later; for now this is a snapshot of
the current state.
Draft because the upstream OTEP is still in development and the upstream
custom-labels PR isn't merged either — this PR is here for review and
parallel development, not for landing as-is.
What's in this branch
Native addon (
bindings/)otel-thread-ctx.cc+otel-thread-ctx.hh: the writer, namespaced asdd::OtelThreadCtx::Init(exports)and called frombinding.cc.otel_thread_ctx_nodejs_v1stays inextern \"C\"atfile scope so it's exported by name through the
dd_pprof.nodedynsymtable. It's a 32-byte struct holding the four fields a reader needs:
cped_slot(V8 isolate'sContinuationPreservedEmbedderDataslotpointer),
als_handle(Global<Object>to the writer'sAsyncLocalStorage),als_identity_hash(JS identity hash for hash-bucketnarrowing), and
undefined_addr(cached per-isolate undefined singletonfor clean "no context" detection by the reader).
the encoded attribute payload, with a 36-byte attrs_data floor (one
cache line total — matches the OTEP "frugal writer" guidance) and ×2
geometric growth on append, capped at the OTEP-recommended 612-byte
attrs_data ceiling.
binding.gypadds-mtls-dialect=gnu2on x86_64 Linux (required byOTEP-4947 for TLSDESC; on arm64 TLSDESC is the only dynamic TLS model
so no flag is needed).
TypeScript layer (
ts/src/otel-thread-ctx.ts)API surface (Linux only; no-op stubs elsewhere):
runWithContext(fn, opts)— wrapsAsyncLocalStorage.run; scopes thecontext to
fn.enterWithContext(opts)— wrapsAsyncLocalStorage.enterWith; attachesto the current async scope without a callback boundary.
clearContext()— detaches any active context (enterWith(undefined)).Idempotent.
appendAttributes(attributes)— appends attributes to the activerecord. Append-only: existing entries are not overwritten. Either
updates the record in place (when the encoded bytes fit in the current
allocation's slack) or reallocates with geometric growth.
isContextTruncated()— sticky boolean; returns true if at any pointin this context's lifetime an attribute had to be dropped because the
encoded payload would have exceeded the 612-byte cap.
makeNamedContext(keys)— returns aNamedContextexposingname-addressed variants of all five top-level functions plus
processContextAttributes, a snapshot of the OTEP-4719 process-contextentries (schema version,
attribute_key_map, plus the V8 layoutconstants — see below) the caller should publish.
opts.traceId/opts.spanIdare rawUint8Array(16 and 8 bytes;Bufferworks as a subclass).opts.attributesis positional: index Nis the value for uint8 key index N on the wire; null/undefined/holes are
skipped. Per-value cap of 255 UTF-8 bytes (uint8 length prefix), total
attrs_data cap of 612 bytes (OTEP-recommended 640-byte total record
minus 28-byte header).
Process-context attributes
makeNamedContext(keys).processContextAttributesexposes a frozensnapshot ready to spread into whatever OTEP-4719 process-context
publisher the application uses:
```js
{
'threadlocal.schema_version': 'nodejs_v1',
'threadlocal.attribute_key_map': [...keys],
// V8 layout constants captured from the V8 headers the addon was
// compiled against — let the reader walk our wrapper and V8's
// OrderedHashMap layout without doing its own V8-internal-symbol
// lookups for the pointer-compression / sandbox state.
'threadlocal.nodejs_v1.wrapped_object_offset': 24,
'threadlocal.nodejs_v1.tagged_size': 8,
}
```
Tests
65-case mocha suite under
ts/test/test-otel-thread-ctx.tscovering:input parsing and validation, on-the-wire record encoding (including
multibyte UTF-8 truncation at the 255-byte per-value cap), the 612-byte
cap and the
isContextTruncatedflag, in-place append vs geometricrealloc, async propagation,
processContextAttributesshape andimmutability,
clearContextsemantics, and areadelf --dyn-symscheck that the TLS symbol is exported with the right binding /
visibility / type. The whole describe block is skipped on non-Linux.
A second commit adds a
scripts/docker/Dockerfile + launcher and atest:dockernpm script that builds the addon and runs the full testsuite in a Linux container — useful for running the new tests from
macOS dev machines. End-to-end verified at 161 passing (96
existing pprof tests + the new suite).