refactor(graphql): migrate shimmer to orchestrion instrumentation#7757
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7757 +/- ##
==========================================
- Coverage 93.69% 93.67% -0.03%
==========================================
Files 889 889
Lines 50846 50856 +10
Branches 11801 11830 +29
==========================================
- Hits 47640 47637 -3
- Misses 3206 3219 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Overall package sizeSelf size: 6.39 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: dcea155 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-06-26 23:18:26 Comparing candidate commit dcea155 in PR branch Found 10 performance improvements and 18 performance regressions! Performance is the same for 2213 metrics, 45 unstable metrics.
|
28daea8 to
297e8ac
Compare
297e8ac to
36022cf
Compare
Benchmark update (revised — supersedes the original analysis posted above)Since the original comment landed, three things changed:
Current numbers (PR head vs master, run 2026-05-26)
The depth=0 residual
Local subtraction confirms it:
That ~130ms is the orchestrion-vs-shimmer publish-format cost on the depth=0 specific path. Behavior is unchanged from master: depth=0 still emits Closing it requires either changing depth=0's semantics (breaking change for users) or changing orchestrion's publish payload to carry pre-computed values (out of scope for this PR, would benefit every plugin migrating to orchestrion). Separate concern:
|
fe1544c to
38b97c3
Compare
9696d95 to
b83c40b
Compare
34f8007 to
131cbcd
Compare
Replaces the shimmer-based graphql instrumentation in
packages/datadog-instrumentations/src/graphql.js with orchestrion AST
rewriting for the top-level execute / parse / validate entry points
(both CJS and ESM in graphql >= 0.10 and @graphql-tools/executor).
Plugin architecture changes:
- packages/datadog-plugin-graphql/src/execute.js: takes over resolver
wrapping that used to live in instrumentations; uses a single
rootCtx per execute() call to coordinate execute-span creation and
per-resolver bookkeeping. Resolve spans are recorded inline at
first-encounter and finished from the resolver callback so encoder
state stays hot across span finalization.
- packages/datadog-plugin-graphql/src/resolve.js: deleted. The
resolve-plugin sub-class was needed only because the old shimmer
architecture published per-field events through a separate
plugin; the orchestrion-based design folds that work into the
execute plugin and uses a module-level _depthDisabled flag to
short-circuit when tracer.use('graphql', { depth: 0 }).
- packages/datadog-plugin-graphql/src/parse.js,validate.js: hooked
via orchestrion publishes; documentSources mapping moved onto the
parse plugin so the execute plugin can read raw query source for
graphql.source tags + IAST taint tracking.
Correctness fixes wired up alongside the migration:
- IAST taint tracking: orchestrion's bindStart now fires the same
apm:graphql:resolve:start payload the old shimmer wrapper produced,
using the actual args object graphql constructs internally rather
than an AST-rebuilt copy. Restores taint propagation for hardcoded
literal resolver arguments (e.g. books(title: "ls")) which the
initial orchestrion port broke.
- AppSec resolver blocking: passes the abort controller directly to
the channel payload so AppSec subscribers can abort the resolver
synchronously. packages/dd-trace/src/appsec/graphql.js updated to
read req via getActiveRequest() (the storage('legacy') call only
worked under the old shimmer setup).
- Channel payload shape change in packages/dd-trace/src/appsec/
channels.js to match the new bindStart contract.
Performance work folded in (per-execute and per-resolver):
- Lazy-build info object and resolver args; only construct when the
plugin actually needs them.
- Eliminate per-field channel publishing — the per-execute rootCtx
holds field records and we publish nothing per resolver in the
APM-only path.
- Collapse redundant path walks in the resolve bindStart so
pathToArray runs once per resolver call.
- Consolidate per-field stash + finish handler into one shared
object; avoids reallocating ctx objects per resolver.
- Short-circuit re-entrant execute() on the same contextValue
(covers yoga's normalizedExecutor pattern that would otherwise
double-span).
- Hot fast-path when depth=0: cache the disabled state on a
module-level flag in configure() so resolveAsync bails on a single
property read, matching master's startResolveCh.hasSubscribers
cost.
- Port of #8309's signature-pipeline optimization onto the migrated
execute plugin.
All 231 plugin-graphql tests pass.
Local sirun on plugin-graphql-long (100q × 5 iters per Node version)
shows per-process user-CPU vs master:
with-depth-on-max: -340 ms (-31%)
with-depth-and-collapse-off: tied
with-tracer-graphql-disabled: -50 ms (-7%)
with-depth-off: +130 ms (+21%)
The depth=0 regression is the only configuration where PR is slower
than master. It traces to orchestrion's publish format carrying raw
function args (master's shimmer wrapper pre-computed
{operation, args, docSource} before publishing). For variants with any
per-resolver work, PR's structural wins overtake this cost many times
over.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds the public TypeScript declaration for the new resolve hook and the
field context object it receives, surfaced as part of the orchestrion
migration. The runtime invocation lives in the execute plugin
(see prior commit); this commit declares the user-facing contract.
```ts
interface FieldContext {
fieldName: string
path: string
error: Error | null
result: unknown
}
interface plugins.graphql.hooks {
resolve?: (span?: Span, field?: FieldContext) => void
...
}
```
Hook fires once per resolved field (post-resolve, before span.finish),
giving users a hook to add custom tags to graphql.resolve spans or
inspect the resolver result without subscribing directly to the
diagnostic channel.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…t + microbench harness
Two additions to the plugin-graphql-long sirun bench:
1. New variant `with-tracer-graphql-disabled`: runs `tracer.init() +
tracer.use('graphql', false)`. dd-trace is fully loaded but the
graphql plugin is explicitly off. The variant is equivalent on master
and on PR 7757 (both honor the explicit-false), so comparing the
other tracer-on variants against this one lets a reviewer subtract
shared dd-trace startup cost from variant deltas to isolate
graphql-plugin-specific overhead.
2. `microbench.js`: standalone microbenchmarks for the graphql plugin's
hot-path functions (wrapResolve, assertField, channel publish costs,
schema-walk wrap setup). Lets contributors pinpoint per-function
regressions during plugin work without going through the full sirun
matrix. Run with `node benchmark/sirun/plugin-graphql-long/microbench.js`.
Modes: PURE=1 (no tracer), NOSUB=1 (tracer loaded + plugin disabled),
default (full tracing on). Not picked up by CI — sirun only runs
meta.json variants.
Together these provide both a baseline subtraction in CI and a faster
local feedback loop when iterating on the plugin.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…th,FieldCtx> optimization Test fix: agent.load() between mocha suites replaces globalThis[dd-trace] with a new object, but graphql.js's module-level ddGlobal capture still pointed to the old one. IITM hook for execution/execute.js doesn't re-fire (module cached), so graphql_defaultFieldResolver was never set on the new ddGlobal. Replaced with a local defaultFieldResolver function in execute.js that mirrors graphql's behavior and survives any number of agent.load() reloads. Also removed the dedicated execution/execute.js addHook that existed only to capture defaultFieldResolver; fixed printer/visitor/utilities hooks to read ddGlobal lazily instead of at module load time. Perf (BridgeAR a39f889 port): fields: Object.create(null) -> new Map(); non-collapse keyed by path node identity (O(1), no string construction per repeated resolver call); shouldInstrumentNode operates on linked-list before Map lookup; path strings computed lazily on first-encounter only; getParentField walks path.prev chain for non-collapse; full GraphQLResolveInfo not retained per field - pluck fieldNode/fieldName/returnType. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
131cbcd to
e216b9f
Compare
…s objects setWrappedFieldResolver was unconditionally writing fieldResolver onto rawArgs[0], which throws TypeError when the caller passes a frozen args object and silently double-wraps caller-supplied resolvers on reuse. - Skip mutation entirely when Object.isExtensible returns false - Skip when caller already has own fieldResolver property - Same contract for positional-call path (rawArgs[6]) Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…imitives
The previous `normalizeContextValue` helper mutated the caller's execute args
to force `contextValue` to be an object (so it could key the contexts WeakMap),
substituting `{}` for any falsy/primitive value. Two regressions ride on that:
- resolvers received a synthetic `{}` instead of what the caller passed, so
user code observed a different contextValue with the tracer enabled than
without — `forwards undefined to resolvers (positional form)` and its
seven sibling tests (false/0/string/null/42/'request-1'/Symbol) caught
this on master in #8502;
- truthy primitives (`42`, `'request-1'`, `Symbol()`) bypassed the
substitution and crashed at `contexts.set(contextValue, ctx)` with
`TypeError: Invalid value used as weak map key`, killing the test runner
process partway through the suite.
Drop the mutation. Use the caller's contextValue verbatim. Guard the WeakMap
write with `isWeakMapKey`; for non-keyable values, store rootCtx in a module-
level AsyncLocalStorage so `resolveAsync` can still find it via
`primitiveContextAls.getStore()` when the WeakMap lookup misses.
Also pre-compute `baseTypeName` (List/NonNull-unwrapped type name) at field-
record creation instead of reading `returnType?.name` at span-tag time —
master #8502's fix for the field-type-tag bug, ported here so resolver spans
for wrapped types report the underlying name instead of `undefined`.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
…e-wrap guard @BridgeAR's nits on packages/datadog-instrumentations/src/graphql.js: - Replace the hand-rolled `patchExecuteExport` (writable-vs-getter branch via try/catch + Object.defineProperty) with `shimmer.wrap`. shimmer handles both shapes via Object.getOwnPropertyDescriptor and the descriptor.writable/.configurable/.enumerable branches in packages/datadog-shimmer/src/shimmer.js (~L130-200). - Replace the `wrapped.__dd_wrapped = true` boolean property with a module-level `WeakSet` tracking already-wrapped function instances. The set is checked at the top of `wrapExecute`; idempotent across repeated module-load hook fires without polluting the function with a tracer-specific property. - Drop the conditional object-form narrowing of arguments[0]. The receiver (AppSec subscriber) is responsible for deciding what to do with the value it sees — passing through whatever the caller actually invoked execute() with is cleaner and matches the master pattern. - Trim the wall-of-text comments. Kept only the one paragraph explaining *why* the wrap exists outside the orchestrion try-block (dc-polyfill catches subscriber throws), since that's the non-obvious bit. Co-Authored-By: Claude Opus 4.7 <[email protected]>
…nch rework The earlier revert (90a60ba) used a stale local master ref and pulled an older version of these files. Master has since picked up #8787 (profiler workload + llmobs guard + shortened benches) and #8734 (integration tuning) which both touch these paths, so the branch went CONFLICTING and Actions stopped triggering on new commits. Pulls in origin/master's current shape for both meta.json (3 variants, adjusted query counts) and index.js (startup guard + assert + the no-WITH_ASYNC_HOOKS path). No graphql plugin code is affected. Co-Authored-By: Claude Opus 4.7 <[email protected]>
…y under the startup-guard ceiling The orchestrion-migrated resolveAsync trims a handful of cycles per call relative to master's shimmer path — `_getTime()` unconditional (no ternary), single-pass template-literal path-build over the graphql Path linked list, dropped result-truthy precheck before the thenable test. Net effect at depth=4 + collapse=on is the 800-query loop now finishes fast enough that setup + module load + tracer init push past the 10% startup-share ceiling the bench asserts at the bottom of the loop, tripping: AssertionError: startup-guard: load+setup was 10.0% of the run (max 10%) at /app/candidate/benchmark/sirun/startup-guard.js:44 (Reported on dd-gitlab/benchmark: [24, 4], wall time 14.263s for the 800- query variant on this branch.) Bump the variant's QUERIES to 1100 — same value `with-depth-off` already uses — so the loop dominates total runtime by a comfortable margin again. Other variants (`with-depth-off` at 1100, `with-depth-and-collapse-off` at 200) keep their existing calibration and pass. Co-Authored-By: Claude Opus 4.7 <[email protected]>
…ide bindStart
The previous design wrapped graphql.execute twice — once with shimmer
(for the apm:graphql:execute:start synchronous-abort contract) and again
with the orchestrion-emitted tracingChannel runtime. The earlier analysis
stopped at "channel.publish and bindStore transforms can't propagate
throws" and reached for shimmer as the only mechanism that could surface
AbortError to graphql.execute's caller.
That missed the orchestrion-emitted wrapper's own propagation path. The
rewriter generates (captured from a live run, graphql 16.12.0):
function execute(__apm$arg0, ...__apm$args) {
const __apm$arguments = [__apm$arg0, ...__apm$args].slice(0, arguments.length);
const __apm$ctx = { arguments: __apm$arguments, self: this, ... };
const __apm$traced = () => {
const __apm$wrapped = function (args) {
const { schema, document, ... } = args; // <- destructures immediately
// ...
};
return __apm$wrapped.apply(this, __apm$arguments);
};
if (!tr_ch_apm_hasSubscribers(ch)) return __apm$traced();
return ch.start.runStores(__apm$ctx, () => {
try {
const result = __apm$traced();
__apm$ctx.result = result;
return result;
} catch (err) {
__apm$ctx.error = err;
ch.error.publish(__apm$ctx);
throw err; // <- THIS rethrows fn's error
} finally {
ch.end.publish(__apm$ctx);
}
});
}
The catch+rethrow propagates errors thrown by __apm$traced (the wrapped
fn) synchronously to graphql.execute's caller. Subscriber and transform
throws are caught and queued via process.nextTick → triggerUncaughtException
(confirmed in Node 25's lib/diagnostics_channel.js source), but a throw
from the wrapped fn itself escapes cleanly.
The bindStart transform receives __apm$ctx, and __apm$ctx.arguments is
the SAME array reference that __apm$wrapped.apply spreads at call time.
Mutations to ctx.arguments[0] from inside bindStart are visible to
graphql's body when it runs.
So the orchestrion-pure abort gate:
1. bindStart synchronously publishes apm:graphql:execute:start with
{ abortController, args }. AppSec/WAF subscribers run inline.
2. After publish returns, bindStart inspects abortController.signal.aborted.
3. If aborted, bindStart replaces ctx.arguments[0] with a Proxy whose
get/has traps throw AbortError, sets ctx.ddAborted = true, and
returns ctx.currentStore (skipping resolver-wrapping work since no
resolvers will run anyway).
4. The wrapper then calls __apm$wrapped(ctx.arguments[0]); graphql's
first statement `const { schema, ... } = args` triggers the Proxy
trap → throws AbortError → the wrapper's catch catches → rethrows
→ graphql.execute's caller catches it via assert.throws.
The plugin's error(ctx) checks ctx.ddAborted and skips the error tag
(per the contract, opSpan.error stays 0 for the abort case). end(ctx)
sees ctx.error and #drain()s the span clean.
Drops:
- packages/datadog-instrumentations/src/graphql.js: removes the
shimmer wrap, the wrappedExecutes WeakSet, the AbortError class,
the patchExecuteExport function, the apm:graphql:execute:start
publish, and the datadog:graphql:execute:abort internal channel.
File is back to a thin orchestrion-only stub plus the ddGlobal
capture hooks.
- packages/datadog-plugin-graphql/src/execute.js: removes the
`addSub(abortExecuteCh.name, ...)` subscription and the
#handleAbort method (no longer needed — bindStart handles span
creation and the wrapper handles the AbortError propagation).
Adds the apm:graphql:execute:start publish + abort gate inline in
bindStart.
Wins:
- One wrap layer instead of two. Closes the per-execute perf delta
the bench dashboard was showing vs master.
- PR title "feat(graphql): migrate shimmer to orchestrion
instrumentation" is now accurate.
- Aligns with the wider tracer's move toward pure orchestrion
instrumentation.
Verified: all 4 originally-failing tests (the abort pair, updateField
sibling-finish, forwards-undefined-positional) pass; the no-mutation
guard tests (`should not overwrite the caller-supplied fieldResolver`,
`should not add fieldResolver to a frozen caller-owned execute args
object`) pass; full sweep on graphql 0.10.0 + 16.x + 15.2.0 shows
238 passing 0 failing locally.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
BridgeAR
left a comment
There was a problem hiding this comment.
This has done a huge leap and it is almost ready in my opinion. The depthDisabled still needs addressing and a few smaller comments would polish it a bit before landing :)
…ddress review nits
Targets the per-resolver overhead that's been driving the +7-15%
instructions delta vs master on with-depth-on-max across Node 20/24/26.
execute.js — resolveAsync
- Combined fast-path at the top: when depth=0 AND neither IAST nor
AppSec has a subscriber, skip everything (rootCtx lookup, path walk,
publishes, span machinery). This is the common production case.
- pathString is computed exactly ONCE per resolver call, collapse-
aware (`buildCollapsedPathStringFromNode` when collapse, else
`buildPathStringFromNode`). The IAST publish and the field record
share the same computed value — previously the field-record path
recomputed it inside the `isFirst` branch and the collapse branch
recomputed `collapsedKey` separately.
- Same for `collapsedKey`: assigned once, reused for `mapKey` and the
field record.
- depth gate is now a single combined check
`if (depthDisabled || !shouldInstrumentNode(...))` after publishes,
not two separate checks.
execute.js — other
- Drop the duplicate local `getSignature` — it already lives in
`./utils.js` and exports cleanly. Import from there.
- `getBaseTypeName` rename `t` → `cursor`, add a defensive null guard
on the loop condition (`while (cursor && cursor.ofType)`).
- `getParentField` inner shadow: rename the looked-up `field` to
`innerField` so it doesn't shadow the function parameter.
- `setWrappedFieldResolver` now returns the (possibly cloned) args
view directly; `bindStart` assigns `ctx.ddArgs = setWrappedFieldResolver(...)`
in one step instead of the previous `setWrappedFieldResolver(...);
ctx.ddArgs = readArgs(ctx.arguments)` pair. Saves one function call +
one property lookup per execute.
parse.js
- Hoist the `if (document)` outer so the documentSources logic is one
branch: read the cached source for an already-known document, or
cache the new (document, source) mapping when both are present.
index.js
- Add a one-line comment explaining why `resolve` has no noop fallback
— it runs per-field (hot path), and `finishResolveSpan` gates with
`if (this.config.hooks.resolve)` so absent-hook callers skip both
the call and the per-field payload-object allocation.
benchmark/sirun/plugin-graphql-long/meta.json
- Bump `with-depth-on-max` QUERIES 1100 → 1500 per @BridgeAR's note
that the operations are faster now; gives the startup-guard ratio
more margin and the bench more samples per partition.
.agents/skills/apm-integrations/references/orchestrion.md
- New section "Propagating Synchronous Errors From bindStart" — captures
the Proxy-on-arguments pattern we used for the
`apm:graphql:execute:start` WAF abort contract. The wrapped fn's
destructure triggers the Proxy trap, the orchestrion wrapper's
catch+rethrow propagates the AbortError synchronously to the caller.
Documents the constraint (subscriber/transform throws are swallowed
by Node's diagnostics_channel), the contract, and the
`error()`/`end()` shape that keeps `opSpan.error === 0` on the abort
path.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
The 1100 → 1500 bumps were defending against the startup-guard 10% threshold firing on our branch (the orchestrion-pure path runs the loop faster per-iteration, so setup ratio creeps higher). But the bench dashboard compares raw total instruction counts vs master's baseline at QUERIES=800 — running ~2x more queries on our branch inflates the total proportionally and reads as a +55-60% regression even though per-query we're ~17% faster than master. Math on the +55.4% Node 24 number: ours_total / master_total = 1.554 ours_total = 1500 × ours_per_query, master_total = 800 × master_per_query → ours_per_query / master_per_query = 1.554 × 800 / 1500 = 0.829 We do ~17% fewer instructions per query on Node 24/26; Node 20 is near-parity. The dashboard "regression" is QUERIES inflation. Revert to master's 800. If startup-guard fires from our faster per-query loop, we'll investigate the bench framework rather than papering over with more queries. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Addresses BridgeAR's review on the orchestrion execute plugin: - Primitive/non-keyable contextValue no longer uses `primitiveContextAls.enterWith()` — that sets the ALS store on the caller's async frame and can leak past execute. Carry rootCtx on the orchestrion-scoped store (`ctx.currentStore.graphqlRootCtx`) that `runStores` already enters for execute instead; resolvers read it via `legacyStorage.getStore()` and it unwinds with the frame. Drops the AsyncLocalStorage import entirely. - Restore the per-node pathString cache (`rootCtx.pathCache`) that master had. The branch was re-walking the whole path linked-list on every resolver call (O(depth) per call); the cache builds each pathString incrementally off the parent's cached value (O(1) amortized). Microbench (deeply nested resolvers): ~3.9x at depth 30, ~6.7x at depth 60, ~parity shallow. Reproducible across fresh processes. - Guard the pathString build on `infoPath` so a missing `info.path` doesn't throw in either the collapse or non-collapse branch (`pathToArray`/`shouldInstrumentNode` already tolerate undefined). - Extract the duplicated object-form arg detection shared by `readArgs` and `setWrappedFieldResolver` into one `isObjectForm` helper. Co-Authored-By: Claude Opus 4.8 <[email protected]>
BridgeAR
left a comment
There was a problem hiding this comment.
This has been a major undertaking!
Rewriting this instrumentation is definitely a very hard task!
I believe this is now in a very good state! Thank you for the hard work!
LGTM
) feat(graphql): migrate instrumentation to orchestrion Migrates GraphQL instrumentation from shimmer wrappers to orchestrion AST rewriting for graphql execute / parse / validate entry points, including CJS and ESM paths for graphql >=0.10 and @graphql-tools/executor. Moves resolver instrumentation into the GraphQL execute plugin. The execute plugin now owns per-execute root context, resolver wrapping, resolve-span lifecycle, source tracking, and resolver hook invocation. The old separate resolve plugin is removed. Preserves and tests the existing cross-feature contracts: - IAST still receives one apm:graphql:resolve:start publish per resolver call, using the actual GraphQL args object. - AppSec still receives resolver payloads through datadog:graphql:resolver:start and can abort synchronously through the shared abort controller. - depth only limits resolve-span creation; IAST/AppSec resolver publishes still happen for depth-gated fields. - depth-gated resolvers now honor abort signals before falling through the no-span fast path. - caller-owned execute args and contextValue are preserved without mutation. - default field resolver behavior matches graphql for primitive parent values. - graphql-yoga / @graphql-tools/executor execution is instrumented. Adds public TypeScript declarations for the GraphQL resolve hook and FieldContext payload. Keeps the implementation orchestrion-only, with no shimmer fallback, and updates the GraphQL long benchmark calibration for the migrated hot path. Regression coverage was added for: - resolver abort behavior past the configured depth - depth: 0 AppSec resolver-channel publishing - primitive-source defaultFieldResolver parity - caller-supplied and frozen execute args - primitive contextValue forwarding - Yoga normalized executor instrumentation - IAST/AppSec per-resolver channel cardinality Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: Ruben Bridgewater <[email protected]>
) feat(graphql): migrate instrumentation to orchestrion Migrates GraphQL instrumentation from shimmer wrappers to orchestrion AST rewriting for graphql execute / parse / validate entry points, including CJS and ESM paths for graphql >=0.10 and @graphql-tools/executor. Moves resolver instrumentation into the GraphQL execute plugin. The execute plugin now owns per-execute root context, resolver wrapping, resolve-span lifecycle, source tracking, and resolver hook invocation. The old separate resolve plugin is removed. Preserves and tests the existing cross-feature contracts: - IAST still receives one apm:graphql:resolve:start publish per resolver call, using the actual GraphQL args object. - AppSec still receives resolver payloads through datadog:graphql:resolver:start and can abort synchronously through the shared abort controller. - depth only limits resolve-span creation; IAST/AppSec resolver publishes still happen for depth-gated fields. - depth-gated resolvers now honor abort signals before falling through the no-span fast path. - caller-owned execute args and contextValue are preserved without mutation. - default field resolver behavior matches graphql for primitive parent values. - graphql-yoga / @graphql-tools/executor execution is instrumented. Adds public TypeScript declarations for the GraphQL resolve hook and FieldContext payload. Keeps the implementation orchestrion-only, with no shimmer fallback, and updates the GraphQL long benchmark calibration for the migrated hot path. Regression coverage was added for: - resolver abort behavior past the configured depth - depth: 0 AppSec resolver-channel publishing - primitive-source defaultFieldResolver parity - caller-supplied and frozen execute args - primitive contextValue forwarding - Yoga normalized executor instrumentation - IAST/AppSec per-resolver channel cardinality Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: Ruben Bridgewater <[email protected]>
) feat(graphql): migrate instrumentation to orchestrion Migrates GraphQL instrumentation from shimmer wrappers to orchestrion AST rewriting for graphql execute / parse / validate entry points, including CJS and ESM paths for graphql >=0.10 and @graphql-tools/executor. Moves resolver instrumentation into the GraphQL execute plugin. The execute plugin now owns per-execute root context, resolver wrapping, resolve-span lifecycle, source tracking, and resolver hook invocation. The old separate resolve plugin is removed. Preserves and tests the existing cross-feature contracts: - IAST still receives one apm:graphql:resolve:start publish per resolver call, using the actual GraphQL args object. - AppSec still receives resolver payloads through datadog:graphql:resolver:start and can abort synchronously through the shared abort controller. - depth only limits resolve-span creation; IAST/AppSec resolver publishes still happen for depth-gated fields. - depth-gated resolvers now honor abort signals before falling through the no-span fast path. - caller-owned execute args and contextValue are preserved without mutation. - default field resolver behavior matches graphql for primitive parent values. - graphql-yoga / @graphql-tools/executor execution is instrumented. Adds public TypeScript declarations for the GraphQL resolve hook and FieldContext payload. Keeps the implementation orchestrion-only, with no shimmer fallback, and updates the GraphQL long benchmark calibration for the migrated hot path. Regression coverage was added for: - resolver abort behavior past the configured depth - depth: 0 AppSec resolver-channel publishing - primitive-source defaultFieldResolver parity - caller-supplied and frozen execute args - primitive contextValue forwarding - Yoga normalized executor instrumentation - IAST/AppSec per-resolver channel cardinality Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: Ruben Bridgewater <[email protected]>
What does this PR do?
Migrates the graphql plugin's instrumentation strategy from
shimmer.wrapto orchestrion AST rewriting. The plugin no longer dynamically wraps individual field resolvers at execute time; instead orchestrion rewritesgraphql'sexecute,parse, andvalidateentry points at module load to publish on diagnostic channels. The plugin subscribes to those channels and owns the per-execute / per-resolver work inpackages/datadog-plugin-graphql/src/execute.js.Also wires up two public API additions (
FieldContexttype +hooks.resolveuser callback) and adds one benchmark variant for measuring shared dd-trace startup cost separately from graphql plugin overhead.Commits
feat(graphql): migrate from shimmer to orchestrion instrumentationfeat(graphql): expose FieldContext and hooks.resolve public typesbench(graphql-long): add tracer-loaded graphql-disabled baseline variant + microbench harnesswith-tracer-graphql-disabledvariant for shared-cost subtraction, plus a microbench harness for local plugin workMigration overview
Before (master)
packages/datadog-instrumentations/src/graphql.jsusesshimmer.wrapagainstgraphql.execute, then walks the schema's type map at execution time to wrap every field resolverGraphQLResolvePluginsub-class handles resolver spans via channel subscriptionsAfter (this PR)
packages/datadog-instrumentations/src/helpers/rewriter/instrumentations/graphql.js(new) — orchestrion config declaring which functions to rewrite (execute,parse,validate) for bothgraphql(CJS + ESM, versions ≥0.10) and@graphql-tools/executor(for graphql-yoga)packages/datadog-instrumentations/src/graphql.js— stripped to justaddHookregistrations + module-export captures for the IAST taint-tracking signature tools (printer,visitor,utilities)packages/datadog-plugin-graphql/src/execute.js— owns the migrated execute and resolver work in one plugin. Resolver wrapping happens at the schema-walk stage of the first execute; resolve spans are created inline at first-encounter and finished from the resolver's then-callback so encoder state stays hot.packages/datadog-plugin-graphql/src/resolve.js— deleted. The separate resolve plugin was needed by the old shimmer architecture (per-field channel publishes routed through a different plugin); with the orchestrion design the work folds into the execute plugin and thetracer.use('graphql', { depth: 0 })short-circuit is handled by a module-level_depthDisabledflag set inconfigure().Correctness fixes folded into the migration commit
AppSec resolver blocking — channel publish payload aligned with the AppSec subscriber's destructure. The old shimmer path sent
{ ctx, resolverInfo }but the AppSec handler destructured{ context, ... }, so blocking never actually aborted a resolve. New payload is{ abortController, resolverInfo }and the handler callsabortController?.abort()directly.IAST taint tracking for hardcoded resolver args — initial orchestrion port published a reconstructed
resolverArgsobject built from AST nodes.taintObject()mutated that object, but graphql'sgetArgumentValues()builds its own separate args object insideexecuteField, so the taint never reached the resolver body. Fix: whenapm:graphql:resolve:starthas subscribers, temporarily replacefieldDef.resolvewith a wrapper that fires the IAST channel with the actual args object graphql constructs internally, restoring the wrapper synchronously as its first action (no async gap betweenbindStartand the resolver call).AppSec request lookup —
packages/dd-trace/src/appsec/graphql.jsnow readsreqviagetActiveRequest()instead ofstorage('legacy').getStore()?.req. The legacy storage call only worked under the old shimmer setup; under orchestrion it would have hit aReferenceErrorbecause the helper was never imported.Performance work folded in
infoand resolverargsonly when the plugin needs themrootCtxholds field records, no per-resolver publish in the APM-only pathpathToArrayruns once per resolverexecute()short-circuit (covers yoga'snormalizedExecutorpattern)_depthDisabledflag set inconfigure()soresolveAsyncbails on a single property read (matching master'sstartResolveCh.hasSubscriberscost shape)tools/signature.jsWeakMap cache) via this PR's rebasePublic API additions (commit 2)
Usage:
Hook fires once per resolved field (post-resolve, before
span.finish()), giving users a place to add custom tags or inspect resolver results without subscribing directly to the diagnostic channel.Bench changes (commit 3)
Adds
with-tracer-graphql-disabledvariant toplugin-graphql-long:Behaviour equivalent on master (uses shimmer; explicit-false unsubscribes) and on PR 7757 (orchestrion publish goes to no subscribers). Comparing other tracer-on variants against this one isolates plugin-specific overhead from shared dd-trace startup cost.
Also adds
microbench.js, a standalone harness that microbenchmarks the plugin's hot-path functions for fast local iteration (not picked up by sirun, which only runsmeta.jsonvariants).Performance results
plugin-graphql-longbenchmark (100 queries/process, 5 sirun iterations, Node 18/20/22/24). All numbers are per-process user-CPU on PR vs master:with-depth-on-max(typical production config)with-depth-and-collapse-onwith-depth-and-collapse-off(1700 spans/qry)with-tracer-graphql-disabled(new)with-depth-off(resolver tracing explicitly off)Why
with-depth-off-22regressesOrchestrion's publish format carries raw function arguments. Master's shimmer wrapper pre-computed
{ operation, args, docSource }before publishing, so master'sbindStartgot them for free. PR 7757'sbindStartre-extracts them per call (readArgs,getOperation,documentSources.get). For depth=0 (no resolve spans created), this per-execute payload extraction is the only thing happening, so it dominates the variant.For any configuration with resolver work (the other tracer-on variants), per-resolver wins overtake this cost many times over.
Local measurement confirmed the subtraction:
depth=0≈ mastertracer-graphql-disabled(≈0 plugin work)depth=0= PRtracer-graphql-disabled+ 130ms per processThe +130ms / process on depth=0 vs master's effectively-zero is the cost. Master's resolve plugin self-disables via
super.configure(false)when depth=0 and its execute plugin gets a pre-computed payload from shimmer's wrap — together producing near-zero per-execute work. Neither is available on the orchestrion architecture without restructuring orchestrion's publish payload itself, which is out of scope.What this regression doesn't affect
tracer.use('graphql', { depth: 0 })still emitsgraphql.executespans and skipsgraphql.resolvespans, identical to master's behaviourdepth>0and gets the wins from other variantswith-tracer-graphql-disabledvariant ties between master and PR)Related but separate
startup-with-tracer-everything(a separate bench, not inplugin-graphql-long) shows +8-13% on tracer-init cost when every integration's hooks register. PR 7757's orchestrion AST rewriting of graphql at module load is the most likely contributor. Worth flagging here even though it's outside the graphql query-time question.Test results
🤖 Generated with Claude Code