From 08a64dd1ef8706ec329bcf33666faa6bfa360f1d Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Thu, 5 Feb 2026 22:22:21 +0100 Subject: [PATCH 01/14] Fix plugin_env script from #4852 to load correct yml file and mention it in CONTRIBUTING.md --- CONTRIBUTING.md | 2 ++ plugin-env | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 182a99016f0..6eebf860aa4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -407,6 +407,8 @@ You can also run the tests for multiple plugins at once by separating them with PLUGINS="amqplib|bluebird" yarn test:plugins ``` +The necessary shell commands for the setup can also be executed at once by the `yarn env` script. + ### Other Unit Tests There are several types of unit tests, for various types of components. The diff --git a/plugin-env b/plugin-env index 78166b8ca72..4d3b04390b5 100755 --- a/plugin-env +++ b/plugin-env @@ -13,7 +13,7 @@ if [ -z "$plugin_name" ]; then node - << EOF const fs=require('fs'); const yaml = require('yaml'); - const pluginsData = fs.readFileSync('.github/workflows/plugins.yml', 'utf8'); + const pluginsData = fs.readFileSync('.github/workflows/apm-integrations.yml', 'utf8'); const env=Object.keys(yaml.parse(pluginsData).jobs); console.log(...env); EOF @@ -36,7 +36,7 @@ fi read -r PLUGINS SERVICES <<<$(node - << EOF const fs=require('fs'); const yaml = require('yaml'); -const pluginsData = fs.readFileSync('.github/workflows/plugins.yml', 'utf8'); +const pluginsData = fs.readFileSync('.github/workflows/apm-integrations.yml', 'utf8'); const { PLUGINS, SERVICES } = yaml.parse(pluginsData).jobs['$plugin_name'].env; console.log(PLUGINS || '', SERVICES || '') EOF From c0b2fe229e8a88c7f2c4268d4c40479f08bff106 Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Fri, 6 Feb 2026 00:25:30 +0100 Subject: [PATCH 02/14] Rename `startGraphqlResolve` channel to `startGraphqlResolver` to match its name --- packages/dd-trace/src/appsec/channels.js | 2 +- packages/dd-trace/src/appsec/graphql.js | 10 +++---- packages/dd-trace/test/appsec/graphql.spec.js | 26 +++++++++---------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/dd-trace/src/appsec/channels.js b/packages/dd-trace/src/appsec/channels.js index eb434fc8297..1aa2552996c 100644 --- a/packages/dd-trace/src/appsec/channels.js +++ b/packages/dd-trace/src/appsec/channels.js @@ -41,6 +41,6 @@ module.exports = { routerMiddlewareError: dc.channel('apm:router:middleware:error'), setCookieChannel: dc.channel('datadog:iast:set-cookie'), setUncaughtExceptionCaptureCallbackStart: dc.channel('datadog:process:setUncaughtExceptionCaptureCallback:start'), - startGraphqlResolve: dc.channel('datadog:graphql:resolver:start'), + startGraphqlResolver: dc.channel('datadog:graphql:resolver:start'), wafRunFinished: dc.channel('datadog:waf:run:finish'), } diff --git a/packages/dd-trace/src/appsec/graphql.js b/packages/dd-trace/src/appsec/graphql.js index 07dc437bff2..e802f307db7 100644 --- a/packages/dd-trace/src/appsec/graphql.js +++ b/packages/dd-trace/src/appsec/graphql.js @@ -12,7 +12,7 @@ const { const waf = require('./waf') const addresses = require('./addresses') const { - startGraphqlResolve, + startGraphqlResolver, graphqlMiddlewareChannel, apolloHttpServerChannel, apolloChannel, @@ -32,7 +32,7 @@ function disable () { disableGraphql() } -function onGraphqlStartResolve ({ context, resolverInfo }) { +function onGraphqlStartResolver ({ context, resolverInfo }) { const req = storage('legacy').getStore()?.req if (!req) return @@ -46,7 +46,7 @@ function onGraphqlStartResolve ({ context, resolverInfo }) { if (requestData?.isInGraphqlRequest) { requestData.blocked = true requestData.wafAction = blockingAction - context?.abortController?.abort() + context.abortController?.abort() } } } @@ -153,11 +153,11 @@ function disableApollo () { } function enableGraphql () { - startGraphqlResolve.subscribe(onGraphqlStartResolve) + startGraphqlResolver.subscribe(onGraphqlStartResolver) } function disableGraphql () { - if (startGraphqlResolve.hasSubscribers) startGraphqlResolve.unsubscribe(onGraphqlStartResolve) + if (startGraphqlResolver.hasSubscribers) startGraphqlResolver.unsubscribe(onGraphqlStartResolver) } module.exports = { diff --git a/packages/dd-trace/test/appsec/graphql.spec.js b/packages/dd-trace/test/appsec/graphql.spec.js index 89b0b31c919..b4ac64528ad 100644 --- a/packages/dd-trace/test/appsec/graphql.spec.js +++ b/packages/dd-trace/test/appsec/graphql.spec.js @@ -11,7 +11,7 @@ const addresses = require('../../src/appsec/addresses') const waf = require('../../src/appsec/waf') const web = require('../../src/plugins/util/web') const { - startGraphqlResolve, + startGraphqlResolver, graphqlMiddlewareChannel, apolloChannel, apolloServerCoreChannel, @@ -63,7 +63,7 @@ describe('GraphQL', () => { assert.strictEqual(apolloChannel.asyncEnd.hasSubscribers, false) assert.strictEqual(apolloServerCoreChannel.start.hasSubscribers, false) assert.strictEqual(apolloServerCoreChannel.asyncEnd.hasSubscribers, false) - assert.strictEqual(startGraphqlResolve.hasSubscribers, false) + assert.strictEqual(startGraphqlResolver.hasSubscribers, false) graphql.enable() @@ -72,7 +72,7 @@ describe('GraphQL', () => { assert.strictEqual(apolloChannel.asyncEnd.hasSubscribers, true) assert.strictEqual(apolloServerCoreChannel.start.hasSubscribers, true) assert.strictEqual(apolloServerCoreChannel.asyncEnd.hasSubscribers, true) - assert.strictEqual(startGraphqlResolve.hasSubscribers, true) + assert.strictEqual(startGraphqlResolver.hasSubscribers, true) }) }) @@ -85,7 +85,7 @@ describe('GraphQL', () => { assert.strictEqual(apolloChannel.asyncEnd.hasSubscribers, true) assert.strictEqual(apolloServerCoreChannel.start.hasSubscribers, true) assert.strictEqual(apolloServerCoreChannel.asyncEnd.hasSubscribers, true) - assert.strictEqual(startGraphqlResolve.hasSubscribers, true) + assert.strictEqual(startGraphqlResolver.hasSubscribers, true) graphql.disable() @@ -94,11 +94,11 @@ describe('GraphQL', () => { assert.strictEqual(apolloChannel.asyncEnd.hasSubscribers, false) assert.strictEqual(apolloServerCoreChannel.start.hasSubscribers, false) assert.strictEqual(apolloServerCoreChannel.asyncEnd.hasSubscribers, false) - assert.strictEqual(startGraphqlResolve.hasSubscribers, false) + assert.strictEqual(startGraphqlResolver.hasSubscribers, false) }) }) - describe('onGraphqlStartResolve', () => { + describe('onGraphqlStartResolver', () => { beforeEach(() => { sinon.stub(waf, 'run').returns(['']) sinon.stub(storage('legacy'), 'getStore').returns({ req: {} }) @@ -116,7 +116,7 @@ describe('GraphQL', () => { resolver: undefined, } - startGraphqlResolve.publish({ context }) + startGraphqlResolver.publish({ context }) sinon.assert.notCalled(waf.run) }) @@ -126,7 +126,7 @@ describe('GraphQL', () => { resolver: '', } - startGraphqlResolve.publish({ context }) + startGraphqlResolver.publish({ context }) sinon.assert.notCalled(waf.run) }) @@ -139,7 +139,7 @@ describe('GraphQL', () => { storage('legacy').getStore().req = undefined - startGraphqlResolve.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ context, resolverInfo }) sinon.assert.notCalled(waf.run) }) @@ -151,7 +151,7 @@ describe('GraphQL', () => { user: [{ id: '1234' }], } - startGraphqlResolve.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ context, resolverInfo }) sinon.assert.calledOnceWithExactly(waf.run, { ephemeral: { @@ -199,7 +199,7 @@ describe('GraphQL', () => { sinon.stub(waf, 'run').returns(['']) - startGraphqlResolve.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ context, resolverInfo }) sinon.assert.calledOnceWithExactly(waf.run, { ephemeral: { @@ -225,7 +225,7 @@ describe('GraphQL', () => { sinon.stub(web, 'root').returns(rootSpan) - startGraphqlResolve.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ context, resolverInfo }) sinon.assert.calledOnceWithExactly(waf.run, { ephemeral: { @@ -257,7 +257,7 @@ describe('GraphQL', () => { sinon.stub(web, 'root').returns(rootSpan) - startGraphqlResolve.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ context, resolverInfo }) sinon.assert.calledOnceWithExactly(waf.run, { ephemeral: { From 137f6e7875f37bfec2f802f66c1e8fd2eaedc2df Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Fri, 6 Feb 2026 00:37:57 +0100 Subject: [PATCH 03/14] Fix and simplify graphqls `callInAsyncScope` * pass only `AbortSignal`, not `AbortController` * throw the abort `.reason`, not a custom `AbortError` * `cb` is always passed, never falsy * do not call `cb` again if `cb(null, result)` threw an error --- .../datadog-instrumentations/src/graphql.js | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/packages/datadog-instrumentations/src/graphql.js b/packages/datadog-instrumentations/src/graphql.js index bf8053e3150..8fda480729f 100644 --- a/packages/datadog-instrumentations/src/graphql.js +++ b/packages/datadog-instrumentations/src/graphql.js @@ -38,13 +38,6 @@ const validateStartCh = channel('apm:graphql:validate:start') const validateFinishCh = channel('apm:graphql:validate:finish') const validateErrorCh = channel('apm:graphql:validate:error') -class AbortError extends Error { - constructor (message) { - super(message) - this.name = 'AbortError' - } -} - const types = new Set(['query', 'mutation', 'subscription']) function getOperation (document, operationName) { @@ -183,7 +176,7 @@ function wrapExecute (execute) { contexts.set(contextValue, ctx) - return callInAsyncScope(exe, this, arguments, ctx.abortController, (err, res) => { + return callInAsyncScope(exe, this, arguments, ctx.abortController.signal, (err, res) => { if (finishResolveCh.hasSubscribers) finishResolvers(ctx) const error = err || (res && res.errors && res.errors[0]) @@ -213,7 +206,7 @@ function wrapResolve (resolve) { const field = assertField(ctx, info, args) - return callInAsyncScope(resolve, this, arguments, ctx.abortController, (err) => { + return callInAsyncScope(resolve, this, arguments, ctx.abortController.signal, (err) => { field.ctx.error = err field.ctx.info = info field.ctx.field = field @@ -226,16 +219,15 @@ function wrapResolve (resolve) { return resolveAsync } -function callInAsyncScope (fn, thisArg, args, abortController, cb) { - cb = cb || (() => {}) - - if (abortController?.signal.aborted) { +function callInAsyncScope (fn, thisArg, args, abortSignal, cb) { + if (abortSignal.aborted) { cb(null, null) - throw new AbortError('Aborted') + throw abortSignal.reason } + let result try { - const result = fn.apply(thisArg, args) + result = fn.apply(thisArg, args) if (result && typeof result.then === 'function') { return result.then( res => { @@ -248,12 +240,12 @@ function callInAsyncScope (fn, thisArg, args, abortController, cb) { } ) } - cb(null, result) - return result } catch (err) { cb(err) throw err - } + } // else + cb(null, result) + return result } function pathToArray (path) { From ca38dff5b2427d1e672090b27256f7f0a360514c Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Fri, 6 Feb 2026 00:50:44 +0100 Subject: [PATCH 04/14] Publish only `abortController` in `datadog:graphql:resolver:start` channel not the entire `rootCtx` --- .../datadog-instrumentations/src/graphql.js | 2 +- .../datadog-plugin-graphql/src/resolve.js | 5 ++- packages/dd-trace/src/appsec/graphql.js | 4 +- packages/dd-trace/test/appsec/graphql.spec.js | 37 +++++++------------ 4 files changed, 20 insertions(+), 28 deletions(-) diff --git a/packages/datadog-instrumentations/src/graphql.js b/packages/datadog-instrumentations/src/graphql.js index 8fda480729f..d446c7c7a61 100644 --- a/packages/datadog-instrumentations/src/graphql.js +++ b/packages/datadog-instrumentations/src/graphql.js @@ -165,7 +165,7 @@ function wrapExecute (execute) { docSource: documentSources.get(document), source, fields: {}, - abortController: new AbortController(), + abortController: new AbortController(), // allow startExecuteCh/startResolveCh subscribers to block execution } return startExecuteCh.runStores(ctx, () => { diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 801867a892c..35bfaf99cbe 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -66,7 +66,10 @@ class GraphQLResolvePlugin extends TracingPlugin { } if (this.resolverStartCh.hasSubscribers) { - this.resolverStartCh.publish({ ctx: rootCtx, resolverInfo: getResolverInfo(info, args) }) + this.resolverStartCh.publish({ + abortController: rootCtx.abortController, + resolverInfo: getResolverInfo(info, args), + }) } return fieldCtx.currentStore diff --git a/packages/dd-trace/src/appsec/graphql.js b/packages/dd-trace/src/appsec/graphql.js index e802f307db7..0419222dc79 100644 --- a/packages/dd-trace/src/appsec/graphql.js +++ b/packages/dd-trace/src/appsec/graphql.js @@ -32,7 +32,7 @@ function disable () { disableGraphql() } -function onGraphqlStartResolver ({ context, resolverInfo }) { +function onGraphqlStartResolver ({ abortController, resolverInfo }) { const req = storage('legacy').getStore()?.req if (!req) return @@ -46,7 +46,7 @@ function onGraphqlStartResolver ({ context, resolverInfo }) { if (requestData?.isInGraphqlRequest) { requestData.blocked = true requestData.wafAction = blockingAction - context.abortController?.abort() + abortController?.abort() } } } diff --git a/packages/dd-trace/test/appsec/graphql.spec.js b/packages/dd-trace/test/appsec/graphql.spec.js index b4ac64528ad..0418c0af09f 100644 --- a/packages/dd-trace/test/appsec/graphql.spec.js +++ b/packages/dd-trace/test/appsec/graphql.spec.js @@ -116,7 +116,7 @@ describe('GraphQL', () => { resolver: undefined, } - startGraphqlResolver.publish({ context }) + startGraphqlResolver.publish(context) sinon.assert.notCalled(waf.run) }) @@ -126,32 +126,29 @@ describe('GraphQL', () => { resolver: '', } - startGraphqlResolver.publish({ context }) + startGraphqlResolver.publish(context) sinon.assert.notCalled(waf.run) }) it('Should not call waf if req is unavailable', () => { - const context = {} const resolverInfo = { user: [{ id: '1234' }], } storage('legacy').getStore().req = undefined - startGraphqlResolver.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ resolverInfo }) sinon.assert.notCalled(waf.run) }) it('Should call waf if resolvers is well formatted', () => { - const context = {} - const resolverInfo = { user: [{ id: '1234' }], } - startGraphqlResolver.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ resolverInfo }) sinon.assert.calledOnceWithExactly(waf.run, { ephemeral: { @@ -173,7 +170,7 @@ describe('GraphQL', () => { grpc_status_code: 10, } - let context, rootSpan + let abortController, rootSpan beforeEach(() => { sinon.stub(storage('legacy'), 'getStore').returns({ req, res }) @@ -181,10 +178,8 @@ describe('GraphQL', () => { graphql.enable() graphqlMiddlewareChannel.start.publish({ req, res }) apolloChannel.start.publish() - context = { - abortController: { - abort: sinon.stub(), - }, + abortController = { + abort: sinon.stub(), } rootSpan = { setTag: sinon.stub() } }) @@ -195,11 +190,9 @@ describe('GraphQL', () => { }) it('Should not call abort', () => { - const abortController = {} - sinon.stub(waf, 'run').returns(['']) - startGraphqlResolver.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ abortController, resolverInfo }) sinon.assert.calledOnceWithExactly(waf.run, { ephemeral: { @@ -207,16 +200,14 @@ describe('GraphQL', () => { }, }, {}) - sinon.assert.notCalled(context.abortController.abort) + sinon.assert.notCalled(abortController.abort) - apolloChannel.asyncEnd.publish({ abortController }) + apolloChannel.asyncEnd.publish({ abortController: {} }) sinon.assert.notCalled(blocking.getBlockingData) }) it('Should call abort', () => { - const abortController = context.abortController - sinon.stub(waf, 'run').returns({ actions: { block_request: blockParameters, @@ -225,7 +216,7 @@ describe('GraphQL', () => { sinon.stub(web, 'root').returns(rootSpan) - startGraphqlResolver.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ abortController, resolverInfo }) sinon.assert.calledOnceWithExactly(waf.run, { ephemeral: { @@ -233,7 +224,7 @@ describe('GraphQL', () => { }, }, {}) - sinon.assert.called(context.abortController.abort) + sinon.assert.called(abortController.abort) const abortData = {} apolloChannel.asyncEnd.publish({ abortController, abortData }) @@ -247,8 +238,6 @@ describe('GraphQL', () => { it('Should catch error when block fails', () => { blocking.getBlockingData.returns(undefined) - const abortController = context.abortController - sinon.stub(waf, 'run').returns({ actions: { block_request: blockParameters, @@ -257,7 +246,7 @@ describe('GraphQL', () => { sinon.stub(web, 'root').returns(rootSpan) - startGraphqlResolver.publish({ context, resolverInfo }) + startGraphqlResolver.publish({ abortController, resolverInfo }) sinon.assert.calledOnceWithExactly(waf.run, { ephemeral: { From 04c90653cc37b8104bab66a8a7402399473e8eec Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Fri, 6 Feb 2026 03:11:16 +0100 Subject: [PATCH 05/14] Merge graphql `field` and `fieldCtx` into single object Avoid allocating unnecessary object and copying property values back and forth. --- .../datadog-instrumentations/src/graphql.js | 59 ++++++++----------- .../datadog-plugin-graphql/src/resolve.js | 21 +++---- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/packages/datadog-instrumentations/src/graphql.js b/packages/datadog-instrumentations/src/graphql.js index d446c7c7a61..4bc1875615a 100644 --- a/packages/datadog-instrumentations/src/graphql.js +++ b/packages/datadog-instrumentations/src/graphql.js @@ -164,7 +164,7 @@ function wrapExecute (execute) { args, docSource: documentSources.get(document), source, - fields: {}, + fields: {}, // fields to be published in `finishResolveCh` before finishing execution abortController: new AbortController(), // allow startExecuteCh/startResolveCh subscribers to block execution } @@ -177,7 +177,7 @@ function wrapExecute (execute) { contexts.set(contextValue, ctx) return callInAsyncScope(exe, this, arguments, ctx.abortController.signal, (err, res) => { - if (finishResolveCh.hasSubscribers) finishResolvers(ctx) + if (finishResolveCh.hasSubscribers) finishResolvers(ctx.fields) const error = err || (res && res.errors && res.errors[0]) @@ -204,13 +204,18 @@ function wrapResolve (resolve) { if (!ctx) return resolve.apply(this, arguments) - const field = assertField(ctx, info, args) + const fieldCtx = createField(ctx, info, args) - return callInAsyncScope(resolve, this, arguments, ctx.abortController.signal, (err) => { - field.ctx.error = err - field.ctx.info = info - field.ctx.field = field - updateFieldCh.publish(field.ctx) + startResolveCh.publish(fieldCtx) + + const path = pathToArray(info.path) + const pathString = path.join('.') + ctx.fields[pathString] = fieldCtx // register for later publishing in `finishResolveCh` (see `finishResolvers`) + + return callInAsyncScope(resolve, this, arguments, ctx.abortController.signal, (err, res) => { + fieldCtx.error = err + fieldCtx.res = res + updateFieldCh.publish(fieldCtx) }) } @@ -258,26 +263,17 @@ function pathToArray (path) { return flattened.reverse() } -function assertField (rootCtx, info, args) { - const pathInfo = info && info.path - - const path = pathToArray(pathInfo) - - const pathString = path.join('.') - const fields = rootCtx.fields - - let field = fields[pathString] - - if (!field) { - const fieldCtx = { info, rootCtx, args } - startResolveCh.publish(fieldCtx) - field = fields[pathString] = { - error: null, - ctx: fieldCtx, - } +function createField (rootCtx, info, args) { + return { + rootCtx, + info, + args, + error: null, + res: null, + parent: null, // populated by GraphQLResolvePlugin in `resolve:start` handler + finishTime: 0, // populated by GraphQLResolvePlugin in `resolve:updateField` handler + // currentStore, parentStore - sometimes populated by GraphQLResolvePlugin.startSpan in `resolve:start` handler } - - return field } function wrapFields (type) { @@ -312,16 +308,13 @@ function wrapFieldType (field) { wrapFields(unwrappedType) } -function finishResolvers ({ fields }) { +function finishResolvers (fields) { for (const key of Object.keys(fields).reverse()) { const field = fields[key] - field.ctx.finishTime = field.finishTime - field.ctx.field = field if (field.error) { - field.ctx.error = field.error - resolveErrorCh.publish(field.ctx) + resolveErrorCh.publish(field) } - finishResolveCh.publish(field.ctx) + finishResolveCh.publish(field) } } diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 35bfaf99cbe..a78fe206d8f 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -17,7 +17,7 @@ class GraphQLResolvePlugin extends TracingPlugin { // we need to get the parent span to the field if it exists for correct span parenting // of nested fields const parentField = getParentField(rootCtx, pathToArray(info && info.path)) - const childOf = parentField?.ctx?.currentStore?.span + const childOf = parentField?.currentStore?.span fieldCtx.parent = parentField @@ -78,16 +78,13 @@ class GraphQLResolvePlugin extends TracingPlugin { constructor (...args) { super(...args) - this.addTraceSub('updateField', (ctx) => { - const { field, info, error } = ctx - - const path = getPath(info, this.config) + this.addTraceSub('updateField', (fieldCtx) => { + const path = getPath(fieldCtx.info, this.config) if (!shouldInstrument(this.config, path)) return - const span = ctx?.currentStore?.span || this.activeSpan - field.finishTime = span._getTime ? span._getTime() : 0 - field.error = field.error || error + const span = fieldCtx?.currentStore?.span || this.activeSpan + fieldCtx.finishTime = span._getTime ? span._getTime() : 0 }) this.resolverStartCh = dc.channel('datadog:graphql:resolver:start') @@ -98,13 +95,13 @@ class GraphQLResolvePlugin extends TracingPlugin { super.configure(config.depth === 0 ? false : config) } - finish (ctx) { - const { finishTime } = ctx + finish (fieldCtx) { + const { finishTime } = fieldCtx - const span = ctx?.currentStore?.span || this.activeSpan + const span = fieldCtx?.currentStore?.span || this.activeSpan span.finish(finishTime) - return ctx.parentStore + return fieldCtx.parentStore } } From 38de20845873fb8b43611c95f21a3f7cfc698460 Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Fri, 6 Feb 2026 04:36:38 +0100 Subject: [PATCH 06/14] Optimise graphql path handling * make `getParentField` look up `path.prev` in a `Map`, instead of relying on `pathToArray`. * simplify `getPath`: change higher-order `withCollapse` function into `collapsePath` * remove duplicated `pathToArray` code and call from wrapped `resolve` function --- .../datadog-instrumentations/src/graphql.js | 19 ++------- .../datadog-plugin-graphql/src/resolve.js | 39 +++++++------------ 2 files changed, 17 insertions(+), 41 deletions(-) diff --git a/packages/datadog-instrumentations/src/graphql.js b/packages/datadog-instrumentations/src/graphql.js index 4bc1875615a..6d3072b8e8c 100644 --- a/packages/datadog-instrumentations/src/graphql.js +++ b/packages/datadog-instrumentations/src/graphql.js @@ -164,7 +164,7 @@ function wrapExecute (execute) { args, docSource: documentSources.get(document), source, - fields: {}, // fields to be published in `finishResolveCh` before finishing execution + fields: new Map(), // fields to be published in `finishResolveCh` before finishing execution; keyed by Path abortController: new AbortController(), // allow startExecuteCh/startResolveCh subscribers to block execution } @@ -208,9 +208,7 @@ function wrapResolve (resolve) { startResolveCh.publish(fieldCtx) - const path = pathToArray(info.path) - const pathString = path.join('.') - ctx.fields[pathString] = fieldCtx // register for later publishing in `finishResolveCh` (see `finishResolvers`) + ctx.fields.set(info.path, fieldCtx) // register for later publishing in `finishResolveCh` (see `finishResolvers`) return callInAsyncScope(resolve, this, arguments, ctx.abortController.signal, (err, res) => { fieldCtx.error = err @@ -253,16 +251,6 @@ function callInAsyncScope (fn, thisArg, args, abortSignal, cb) { return result } -function pathToArray (path) { - const flattened = [] - let curr = path - while (curr) { - flattened.push(curr.key) - curr = curr.prev - } - return flattened.reverse() -} - function createField (rootCtx, info, args) { return { rootCtx, @@ -309,8 +297,7 @@ function wrapFieldType (field) { } function finishResolvers (fields) { - for (const key of Object.keys(fields).reverse()) { - const field = fields[key] + for (const field of [...fields.values()].reverse()) { if (field.error) { resolveErrorCh.publish(field) } diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index a78fe206d8f..73e1e04aec8 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -12,21 +12,18 @@ class GraphQLResolvePlugin extends TracingPlugin { start (fieldCtx) { const { info, rootCtx, args } = fieldCtx - const path = getPath(info, this.config) - // we need to get the parent span to the field if it exists for correct span parenting // of nested fields - const parentField = getParentField(rootCtx, pathToArray(info && info.path)) + const parentField = getParentField(rootCtx, info.path) const childOf = parentField?.currentStore?.span fieldCtx.parent = parentField + const path = getPath(info, this.config) if (!shouldInstrument(this.config, path)) return const computedPathString = path.join('.') if (this.config.collapse) { - if (rootCtx.fields[computedPathString]) return - if (!rootCtx[collapsedPathSym]) { rootCtx[collapsedPathSym] = {} } else if (rootCtx[collapsedPathSym][computedPathString]) { @@ -119,10 +116,10 @@ function shouldInstrument (config, path) { } function getPath (info, config) { - const responsePathAsArray = config.collapse - ? withCollapse(pathToArray) - : pathToArray - return responsePathAsArray(info && info.path) + const path = pathToArray(info.path) + return config.collapse + ? collapsePath(path) + : path } function pathToArray (path) { @@ -135,11 +132,8 @@ function pathToArray (path) { return flattened.reverse() } -function withCollapse (responsePathAsArray) { - return function () { - return responsePathAsArray.apply(this, arguments) - .map(segment => typeof segment === 'number' ? '*' : segment) - } +function collapsePath (pathArray) { + return pathArray.map(segment => typeof segment === 'number' ? '*' : segment) } function getResolverInfo (info, args) { @@ -174,18 +168,13 @@ function getResolverInfo (info, args) { } function getParentField (parentCtx, path) { - for (let i = path.length - 1; i > 0; i--) { - const field = getField(parentCtx, path.slice(0, i)) - if (field) { - return field - } + let curr = path.prev + // Skip segments in (nested) lists + // Could also be done by `while (!parentCtx.fields.has(curr))` + while (curr && typeof curr.key === 'number') { + curr = curr.prev } - - return null -} - -function getField (parentCtx, path) { - return parentCtx.fields[path.join('.')] + return parentCtx.fields.get(curr) ?? null } module.exports = GraphQLResolvePlugin From f3b61d1e63e79a4e4924b788ff3c28e4cc9f25b3 Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Fri, 6 Feb 2026 19:29:35 +0100 Subject: [PATCH 07/14] Setup `field.parentField` already before publishing to `apm:graphql:resolve:start` channel --- packages/datadog-instrumentations/src/graphql.js | 12 +++++++++++- packages/datadog-plugin-graphql/src/resolve.js | 15 +-------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/datadog-instrumentations/src/graphql.js b/packages/datadog-instrumentations/src/graphql.js index 6d3072b8e8c..a366161cec7 100644 --- a/packages/datadog-instrumentations/src/graphql.js +++ b/packages/datadog-instrumentations/src/graphql.js @@ -258,12 +258,22 @@ function createField (rootCtx, info, args) { args, error: null, res: null, - parent: null, // populated by GraphQLResolvePlugin in `resolve:start` handler + parentField: getParentField(rootCtx, info.path), finishTime: 0, // populated by GraphQLResolvePlugin in `resolve:updateField` handler // currentStore, parentStore - sometimes populated by GraphQLResolvePlugin.startSpan in `resolve:start` handler } } +function getParentField (rootCtx, path) { + let curr = path.prev + // Skip segments in (nested) lists + // Could also be done by `while (!rootCtx.fields.has(curr))` + while (curr && typeof curr.key === 'number') { + curr = curr.prev + } + return rootCtx.fields.get(curr) ?? null +} + function wrapFields (type) { if (!type || !type._fields || patchedTypes.has(type)) { return diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 73e1e04aec8..7628304a62f 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -10,15 +10,12 @@ class GraphQLResolvePlugin extends TracingPlugin { static operation = 'resolve' start (fieldCtx) { - const { info, rootCtx, args } = fieldCtx + const { info, rootCtx, args, parentField } = fieldCtx // we need to get the parent span to the field if it exists for correct span parenting // of nested fields - const parentField = getParentField(rootCtx, info.path) const childOf = parentField?.currentStore?.span - fieldCtx.parent = parentField - const path = getPath(info, this.config) if (!shouldInstrument(this.config, path)) return const computedPathString = path.join('.') @@ -167,14 +164,4 @@ function getResolverInfo (info, args) { return resolverInfo } -function getParentField (parentCtx, path) { - let curr = path.prev - // Skip segments in (nested) lists - // Could also be done by `while (!parentCtx.fields.has(curr))` - while (curr && typeof curr.key === 'number') { - curr = curr.prev - } - return parentCtx.fields.get(curr) ?? null -} - module.exports = GraphQLResolvePlugin From 7653e9b2b7905c5d2eef6a9587232211ff14825f Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Mon, 9 Feb 2026 09:30:38 +0100 Subject: [PATCH 08/14] Collect only context objects of instrumented fields for publishing in `apm:graphql:resolve:finish` channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … to avoid growing `ctx.fields` to a considerable size even when not necessary. Also rename `fieldCtx` to just `field`. --- .../datadog-instrumentations/src/graphql.js | 34 ++++++++++++------- .../datadog-plugin-graphql/src/resolve.js | 19 ++++++----- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/packages/datadog-instrumentations/src/graphql.js b/packages/datadog-instrumentations/src/graphql.js index a366161cec7..dbfd15a63f1 100644 --- a/packages/datadog-instrumentations/src/graphql.js +++ b/packages/datadog-instrumentations/src/graphql.js @@ -164,7 +164,8 @@ function wrapExecute (execute) { args, docSource: documentSources.get(document), source, - fields: new Map(), // fields to be published in `finishResolveCh` before finishing execution; keyed by Path + fields: new WeakMap(), // fields keyed by their Path object + finishContexts: [], // field contexts to be published in `finishResolveCh` before finishing execution abortController: new AbortController(), // allow startExecuteCh/startResolveCh subscribers to block execution } @@ -177,7 +178,9 @@ function wrapExecute (execute) { contexts.set(contextValue, ctx) return callInAsyncScope(exe, this, arguments, ctx.abortController.signal, (err, res) => { - if (finishResolveCh.hasSubscribers) finishResolvers(ctx.fields) + if (finishResolveCh.hasSubscribers && ctx.finishContexts.length) { + finishResolvers(ctx.finishContexts) + } const error = err || (res && res.errors && res.errors[0]) @@ -204,16 +207,20 @@ function wrapResolve (resolve) { if (!ctx) return resolve.apply(this, arguments) - const fieldCtx = createField(ctx, info, args) + const field = createField(ctx, info, args) + ctx.fields.set(info.path, field) - startResolveCh.publish(fieldCtx) + startResolveCh.publish(field) - ctx.fields.set(info.path, fieldCtx) // register for later publishing in `finishResolveCh` (see `finishResolvers`) + if (field.finishCtx) { + // register for later publishing in `finishResolveCh` (see `finishResolvers`) + ctx.finishContexts.push(field.finishCtx) + } return callInAsyncScope(resolve, this, arguments, ctx.abortController.signal, (err, res) => { - fieldCtx.error = err - fieldCtx.res = res - updateFieldCh.publish(fieldCtx) + field.error = err + field.res = res + updateFieldCh.publish(field) }) } @@ -259,6 +266,7 @@ function createField (rootCtx, info, args) { error: null, res: null, parentField: getParentField(rootCtx, info.path), + finishCtx: null, // sometimes populated by GraphQLResolvePlugin in `resolve:start` handler finishTime: 0, // populated by GraphQLResolvePlugin in `resolve:updateField` handler // currentStore, parentStore - sometimes populated by GraphQLResolvePlugin.startSpan in `resolve:start` handler } @@ -306,12 +314,12 @@ function wrapFieldType (field) { wrapFields(unwrappedType) } -function finishResolvers (fields) { - for (const field of [...fields.values()].reverse()) { - if (field.error) { - resolveErrorCh.publish(field) +function finishResolvers (finishContexts) { + for (const fieldCtx of finishContexts.reverse()) { + if (fieldCtx.error) { + resolveErrorCh.publish(fieldCtx) } - finishResolveCh.publish(field) + finishResolveCh.publish(fieldCtx) } } diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 7628304a62f..177b16ae11c 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -9,8 +9,8 @@ class GraphQLResolvePlugin extends TracingPlugin { static id = 'graphql' static operation = 'resolve' - start (fieldCtx) { - const { info, rootCtx, args, parentField } = fieldCtx + start (field) { + const { info, rootCtx, args, parentField } = field // we need to get the parent span to the field if it exists for correct span parenting // of nested fields @@ -24,6 +24,7 @@ class GraphQLResolvePlugin extends TracingPlugin { if (!rootCtx[collapsedPathSym]) { rootCtx[collapsedPathSym] = {} } else if (rootCtx[collapsedPathSym][computedPathString]) { + field.finishCtx = field return } @@ -46,7 +47,9 @@ class GraphQLResolvePlugin extends TracingPlugin { 'graphql.field.type': info.returnType.name, 'graphql.source': source, }, - }, fieldCtx) + }, field) + // make this.finish(fieldCtx) be called before operation execution ends: + field.finishCtx = field if (fieldNode && this.config.variables && fieldNode.arguments) { const variables = this.config.variables(info.variableValues) @@ -66,19 +69,19 @@ class GraphQLResolvePlugin extends TracingPlugin { }) } - return fieldCtx.currentStore + return field.currentStore } constructor (...args) { super(...args) - this.addTraceSub('updateField', (fieldCtx) => { - const path = getPath(fieldCtx.info, this.config) + this.addTraceSub('updateField', (field) => { + const path = getPath(field.info, this.config) if (!shouldInstrument(this.config, path)) return - const span = fieldCtx?.currentStore?.span || this.activeSpan - fieldCtx.finishTime = span._getTime ? span._getTime() : 0 + const span = field?.currentStore?.span || this.activeSpan + field.finishTime = span._getTime ? span._getTime() : 0 }) this.resolverStartCh = dc.channel('datadog:graphql:resolver:start') From 67bf37a855f993d57a38288539c52a1cbe9a73b2 Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Mon, 9 Feb 2026 18:51:03 +0100 Subject: [PATCH 09/14] Optimise `shouldInstrument` check for graphql resolver spans * compute depth from depth of parent field (with complications due to https://github.com/DataDog/dd-trace-js/issues/7468) * check field depth against `config.depth` before even calling `pathToArray` and `collapsePath`, avoiding array allocations * hoist `config.depth < 0` check out of the resolver instrumentation, memoise it on the plugin instance --- .../datadog-instrumentations/src/graphql.js | 4 ++- .../datadog-plugin-graphql/src/resolve.js | 29 +++++++++++-------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/datadog-instrumentations/src/graphql.js b/packages/datadog-instrumentations/src/graphql.js index dbfd15a63f1..56f56acea4f 100644 --- a/packages/datadog-instrumentations/src/graphql.js +++ b/packages/datadog-instrumentations/src/graphql.js @@ -259,13 +259,15 @@ function callInAsyncScope (fn, thisArg, args, abortSignal, cb) { } function createField (rootCtx, info, args) { + const parentField = getParentField(rootCtx, info.path) return { rootCtx, info, args, error: null, res: null, - parentField: getParentField(rootCtx, info.path), + parentField, + depth: parentField ? parentField.depth + 1 : 1, finishCtx: null, // sometimes populated by GraphQLResolvePlugin in `resolve:start` handler finishTime: 0, // populated by GraphQLResolvePlugin in `resolve:updateField` handler // currentStore, parentStore - sometimes populated by GraphQLResolvePlugin.startSpan in `resolve:start` handler diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 177b16ae11c..3be1248f3e6 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -11,13 +11,15 @@ class GraphQLResolvePlugin extends TracingPlugin { start (field) { const { info, rootCtx, args, parentField } = field + // FIXME - https://github.com/DataDog/dd-trace-js/issues/7468 + if (this.config.collapse) field.depth += getListLevel(info.path) + if (!this.shouldInstrument(field)) return // we need to get the parent span to the field if it exists for correct span parenting // of nested fields const childOf = parentField?.currentStore?.span const path = getPath(info, this.config) - if (!shouldInstrument(this.config, path)) return const computedPathString = path.join('.') if (this.config.collapse) { @@ -76,20 +78,23 @@ class GraphQLResolvePlugin extends TracingPlugin { super(...args) this.addTraceSub('updateField', (field) => { - const path = getPath(field.info, this.config) - - if (!shouldInstrument(this.config, path)) return + if (!this.shouldInstrument(field)) return const span = field?.currentStore?.span || this.activeSpan field.finishTime = span._getTime ? span._getTime() : 0 }) this.resolverStartCh = dc.channel('datadog:graphql:resolver:start') + this.shouldInstrument = _field => false } configure (config) { // this will disable resolve subscribers if `config.depth` is set to 0 super.configure(config.depth === 0 ? false : config) + + this.shouldInstrument = config.depth < 0 + ? _field => true + : field => config.depth >= field.depth } finish (fieldCtx) { @@ -104,15 +109,15 @@ class GraphQLResolvePlugin extends TracingPlugin { // helpers -function shouldInstrument (config, path) { - let depth = 0 - for (const item of path) { - if (typeof item === 'string') { - depth += 1 - } +/** Count `*` chars in front of last path segment for a collapsed path */ +function getListLevel (path) { + let lvl = 0 + let curr = path.prev + while (curr && typeof curr.key === 'number') { + lvl++ + curr = curr.prev } - - return config.depth < 0 || config.depth >= depth + return lvl } function getPath (info, config) { From 78480b582d88cc8c4da5a6afac59d09617949936 Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Mon, 23 Feb 2026 09:20:50 +0100 Subject: [PATCH 10/14] Comment out superfluous/unused return values --- packages/datadog-plugin-graphql/src/execute.js | 2 +- packages/datadog-plugin-graphql/src/resolve.js | 4 ++-- packages/datadog-plugin-graphql/src/validate.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/datadog-plugin-graphql/src/execute.js b/packages/datadog-plugin-graphql/src/execute.js index 90ecac1f2a6..76994f79f60 100644 --- a/packages/datadog-plugin-graphql/src/execute.js +++ b/packages/datadog-plugin-graphql/src/execute.js @@ -47,7 +47,7 @@ class GraphQLExecutePlugin extends TracingPlugin { } super.finish(ctx) - return ctx.parentStore + // return ctx.parentStore } } diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 3be1248f3e6..280e357ef38 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -71,7 +71,7 @@ class GraphQLResolvePlugin extends TracingPlugin { }) } - return field.currentStore + // return field.currentStore // seems unused? This is not `bindStart`! } constructor (...args) { @@ -103,7 +103,7 @@ class GraphQLResolvePlugin extends TracingPlugin { const span = fieldCtx?.currentStore?.span || this.activeSpan span.finish(finishTime) - return fieldCtx.parentStore + // return fieldCtx.parentStore } } diff --git a/packages/datadog-plugin-graphql/src/validate.js b/packages/datadog-plugin-graphql/src/validate.js index 789df6a9da8..09b73a9e75c 100644 --- a/packages/datadog-plugin-graphql/src/validate.js +++ b/packages/datadog-plugin-graphql/src/validate.js @@ -33,7 +33,7 @@ class GraphQLValidatePlugin extends TracingPlugin { } super.finish(ctx) - return ctx.parentStore + // return ctx.parentStore } } From 259a03e82811d7e0eca26e62905a907f24244301 Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Mon, 23 Feb 2026 09:22:25 +0100 Subject: [PATCH 11/14] Simplify `graphql.variables` handling --- packages/datadog-plugin-graphql/src/resolve.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 280e357ef38..5987cf164e0 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -34,7 +34,7 @@ class GraphQLResolvePlugin extends TracingPlugin { } const document = rootCtx.source - const fieldNode = info.fieldNodes.find(fieldNode => fieldNode.kind === 'Field') + const fieldNode = info.fieldNodes.find(fieldNode => fieldNode.kind === 'Field') // FIXME: https://github.com/graphql/graphql-js/issues/605#issuecomment-266160864 const loc = this.config.source && document && fieldNode && fieldNode.loc const source = loc && document.slice(loc.start, loc.end) @@ -56,10 +56,12 @@ class GraphQLResolvePlugin extends TracingPlugin { if (fieldNode && this.config.variables && fieldNode.arguments) { const variables = this.config.variables(info.variableValues) - for (const arg of fieldNode.arguments) { - if (arg.value?.name && arg.value.kind === 'Variable' && variables[arg.value.name.value]) { - const name = arg.value.name.value - span.setTag(`graphql.variables.${name}`, variables[name]) + for (const { value: argValue } of fieldNode.arguments) { + if (argValue.kind === 'Variable') { + const varName = argValue.name.value + if (variables[varName] != null) { + span.setTag(`graphql.variables.${varName}`, variables[varName]) + } } } } From af6dff4b4f331d731b9bd372c2d8062910bf9692 Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Mon, 23 Feb 2026 09:43:07 +0100 Subject: [PATCH 12/14] Refactor `graphql.resolve` handling of collapsed fields * instead of ignoring subsequent resolver calls, use a shared context object for them * add the (single) tracer span to this context, and re-enter it * optimise finding existing shared context by looking up `fieldName` in `parentField.shared.collapsedChildren` (or in `rootCtx.toplevelCollapsed` for top-level fields), not by constructing a `computedPathString` (avoiding unnecessary `pathToArray(info.path)` calls) --- .../datadog-plugin-graphql/src/resolve.js | 82 ++++++++++++------- 1 file changed, 52 insertions(+), 30 deletions(-) diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 5987cf164e0..dfbec7d201c 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -3,8 +3,6 @@ const dc = require('dc-polyfill') const TracingPlugin = require('../../dd-trace/src/plugins/tracing') -const collapsedPathSym = Symbol('collapsedPaths') - class GraphQLResolvePlugin extends TracingPlugin { static id = 'graphql' static operation = 'resolve' @@ -17,20 +15,42 @@ class GraphQLResolvePlugin extends TracingPlugin { // we need to get the parent span to the field if it exists for correct span parenting // of nested fields - const childOf = parentField?.currentStore?.span - - const path = getPath(info, this.config) - const computedPathString = path.join('.') + const childOf = parentField ? this.getFieldContext(parentField).currentStore?.span : undefined + let path, ctx if (this.config.collapse) { - if (!rootCtx[collapsedPathSym]) { - rootCtx[collapsedPathSym] = {} - } else if (rootCtx[collapsedPathSym][computedPathString]) { - field.finishCtx = field + const sharedContexts = parentField + ? parentField.shared.collapsedChildren + : (rootCtx.toplevelCollapsed ??= {}) + ctx = sharedContexts[info.fieldName] + if (ctx) { + // Add reference to the shared context object, + // for the `updateField` to store the `.finishTime` on + // and for children resolvers to find `.collapsedChildren`. + // Should not be `finish`ed again and again though, so we don't use .finishCtx` + field.shared = ctx + this.enter(ctx.currentStore.span) // TODO: test this! return } - - rootCtx[collapsedPathSym][computedPathString] = true + path = collapsePath(pathToArray(info.path)) + // create shared context + ctx = sharedContexts[info.fieldName] = { + fieldName: info.fieldName, + path, + parent: parentField?.shared, + finishTime: 0, + collapsedChildren: {}, // slightly pointless on leaf fields? + // more to be added by `startSpan` + } + field.shared = ctx + // make this.finish(fieldCtx) be called before operation execution ends: + field.finishCtx = ctx + } else { + path = pathToArray(info.path) + ctx = field + // make this.finish(fieldCtx) be called before operation execution ends: + // and also to receive events when there's an `.error` on them + field.finishCtx = field } const document = rootCtx.source @@ -45,13 +65,11 @@ class GraphQLResolvePlugin extends TracingPlugin { type: 'graphql', meta: { 'graphql.field.name': info.fieldName, - 'graphql.field.path': computedPathString, + 'graphql.field.path': path.join('.'), 'graphql.field.type': info.returnType.name, 'graphql.source': source, }, - }, field) - // make this.finish(fieldCtx) be called before operation execution ends: - field.finishCtx = field + }, ctx) if (fieldNode && this.config.variables && fieldNode.arguments) { const variables = this.config.variables(info.variableValues) @@ -73,7 +91,7 @@ class GraphQLResolvePlugin extends TracingPlugin { }) } - // return field.currentStore // seems unused? This is not `bindStart`! + // return ctx.currentStore // seems unused? This is not `bindStart`! } constructor (...args) { @@ -82,12 +100,20 @@ class GraphQLResolvePlugin extends TracingPlugin { this.addTraceSub('updateField', (field) => { if (!this.shouldInstrument(field)) return - const span = field?.currentStore?.span || this.activeSpan - field.finishTime = span._getTime ? span._getTime() : 0 + const fieldCtx = this.getFieldContext(field) + const span = fieldCtx.currentStore?.span || this.activeSpan + fieldCtx.finishTime = span._getTime ? span._getTime() : 0 + + if (this.config.collapse && field.error) { + // the `field.shared` context is used to publish `error` events in `finishResolvers` :-/ + // FIXME: this should probably use `extractErrorIntoSpanEvent` + fieldCtx.error ??= field.error // notice the first error wins (like in `validate` and `execute`) + } }) this.resolverStartCh = dc.channel('datadog:graphql:resolver:start') this.shouldInstrument = _field => false + this.getFieldContext = _field => null } configure (config) { @@ -97,12 +123,15 @@ class GraphQLResolvePlugin extends TracingPlugin { this.shouldInstrument = config.depth < 0 ? _field => true : field => config.depth >= field.depth + this.getFieldContext = config.collapse + ? field => field.shared + : field => field } - finish (fieldCtx) { - const { finishTime } = fieldCtx - - const span = fieldCtx?.currentStore?.span || this.activeSpan + finish ({ finishTime, currentStore }) { + const span = currentStore?.span // no `|| this.activeSpan`, which might be anything + if (!span) return + // we care only about the span that was opened for the respective context span.finish(finishTime) // return fieldCtx.parentStore @@ -122,13 +151,6 @@ function getListLevel (path) { return lvl } -function getPath (info, config) { - const path = pathToArray(info.path) - return config.collapse - ? collapsePath(path) - : path -} - function pathToArray (path) { const flattened = [] let curr = path From 0931f362e189df721f84b9688d520b859c7e1fca Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Mon, 23 Feb 2026 19:04:37 +0100 Subject: [PATCH 13/14] Add a `resolve` hook for the graphql plugin --- index.d.ts | 36 +++++++++++++------ packages/datadog-plugin-graphql/src/index.js | 4 ++- .../datadog-plugin-graphql/src/resolve.js | 2 ++ 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/index.d.ts b/index.d.ts index 1356472514b..5b9c9b84605 100644 --- a/index.d.ts +++ b/index.d.ts @@ -2009,16 +2009,31 @@ declare namespace tracer { */ interface google_genai extends Integration {} - /** @hidden */ - interface ExecutionArgs { - schema: any, - document: any, - rootValue?: any, - contextValue?: any, - variableValues?: any, - operationName?: string, - fieldResolver?: any, - typeResolver?: any, + /** @hidden - the `graphql.ExecutionArgs` passed to the `execute` call */ + interface ExecutionArgs { + schema: any, + document: any, + rootValue?: any, + contextValue?: any, + variableValues?: any, + operationName?: string, + fieldResolver?: any, + typeResolver?: any, + } + + interface FieldContext { + /** The `graphql.GraphQLResolveInfo` for the resolver call */ + info: any; + /** The arguments passed to the resolver */ + args: any; + /** The error thrown by the resolver, if any */ + error: null | Error; + /** The result returned by the resolver, if any */ + res: unknown; + /** The field context from the resolver of the parent field (a level up on the path) */ + parentField: FieldContext | null; + /** The nesting depth of the field in the query */ + depth: number; } /** @@ -2099,6 +2114,7 @@ declare namespace tracer { execute?: (span?: Span, args?: ExecutionArgs, res?: any) => void; validate?: (span?: Span, document?: any, errors?: any) => void; parse?: (span?: Span, source?: any, document?: any) => void; + resolve?: (span?: Span, field: FieldContext) => void; } } diff --git a/packages/datadog-plugin-graphql/src/index.js b/packages/datadog-plugin-graphql/src/index.js index 91e79a2f4b4..13acc7ce9d7 100644 --- a/packages/datadog-plugin-graphql/src/index.js +++ b/packages/datadog-plugin-graphql/src/index.js @@ -61,12 +61,14 @@ function getVariablesFilter (config) { const noop = () => {} +// FIXME: should not be necessary given `TracingPlugin.configure` already does this defaulting function getHooks ({ hooks }) { const execute = hooks?.execute ?? noop const parse = hooks?.parse ?? noop const validate = hooks?.validate ?? noop + const resolve = hooks?.resolve ?? noop - return { execute, parse, validate } + return { execute, parse, validate, resolve } } module.exports = GraphQLPlugin diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index dfbec7d201c..748275e03a3 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -109,6 +109,8 @@ class GraphQLResolvePlugin extends TracingPlugin { // FIXME: this should probably use `extractErrorIntoSpanEvent` fieldCtx.error ??= field.error // notice the first error wins (like in `validate` and `execute`) } + + this.config.hooks.resolve(span, field) }) this.resolverStartCh = dc.channel('datadog:graphql:resolver:start') From 472ab35be743b2c643826a0773fc4fbcff64635a Mon Sep 17 00:00:00 2001 From: Andreas Bergmaier Date: Mon, 23 Feb 2026 20:01:13 +0100 Subject: [PATCH 14/14] Simplify graphql instrumentation * get rid of `apm:graphql:resolve:updateField` channel * use `apm:graphql:resolve:finish` channel in normal purpose, publishing to it after every `resolve()` execution * adjust `GraphQLResolvePlugin` methods * add optional `.finalize` hook to resolver field contexts instead * use normal `GraphQLResolvePlugin.error` method for handling non-collapsed fields --- .../datadog-instrumentations/src/graphql.js | 31 ++++---- .../datadog-plugin-graphql/src/resolve.js | 78 +++++++++---------- 2 files changed, 52 insertions(+), 57 deletions(-) diff --git a/packages/datadog-instrumentations/src/graphql.js b/packages/datadog-instrumentations/src/graphql.js index 56f56acea4f..3e281e4cd97 100644 --- a/packages/datadog-instrumentations/src/graphql.js +++ b/packages/datadog-instrumentations/src/graphql.js @@ -25,7 +25,6 @@ const executeErrorCh = channel('apm:graphql:execute:error') // resolve channels const startResolveCh = channel('apm:graphql:resolve:start') const finishResolveCh = channel('apm:graphql:resolve:finish') -const updateFieldCh = channel('apm:graphql:resolve:updateField') const resolveErrorCh = channel('apm:graphql:resolve:error') // parse channels @@ -165,7 +164,7 @@ function wrapExecute (execute) { docSource: documentSources.get(document), source, fields: new WeakMap(), // fields keyed by their Path object - finishContexts: [], // field contexts to be published in `finishResolveCh` before finishing execution + finalizations: [], // fields whose `.finalize()` method needs to be invoked before finishing execution abortController: new AbortController(), // allow startExecuteCh/startResolveCh subscribers to block execution } @@ -178,8 +177,8 @@ function wrapExecute (execute) { contexts.set(contextValue, ctx) return callInAsyncScope(exe, this, arguments, ctx.abortController.signal, (err, res) => { - if (finishResolveCh.hasSubscribers && ctx.finishContexts.length) { - finishResolvers(ctx.finishContexts) + if (ctx.finalizations.length) { + finalizeResolvers(ctx.finalizations) } const error = err || (res && res.errors && res.errors[0]) @@ -212,15 +211,18 @@ function wrapResolve (resolve) { startResolveCh.publish(field) - if (field.finishCtx) { - // register for later publishing in `finishResolveCh` (see `finishResolvers`) - ctx.finishContexts.push(field.finishCtx) + if (field.finalize) { + // register for `.finalize()` invocation before execution finishes + ctx.finalizations.push(field) } return callInAsyncScope(resolve, this, arguments, ctx.abortController.signal, (err, res) => { - field.error = err + if (err) { + field.error = err + resolveErrorCh.publish(field) + } field.res = res - updateFieldCh.publish(field) + finishResolveCh.publish(field) }) } @@ -268,7 +270,7 @@ function createField (rootCtx, info, args) { res: null, parentField, depth: parentField ? parentField.depth + 1 : 1, - finishCtx: null, // sometimes populated by GraphQLResolvePlugin in `resolve:start` handler + finalize: null, // sometimes populated by GraphQLResolvePlugin in `resolve:start` handler finishTime: 0, // populated by GraphQLResolvePlugin in `resolve:updateField` handler // currentStore, parentStore - sometimes populated by GraphQLResolvePlugin.startSpan in `resolve:start` handler } @@ -316,12 +318,9 @@ function wrapFieldType (field) { wrapFields(unwrappedType) } -function finishResolvers (finishContexts) { - for (const fieldCtx of finishContexts.reverse()) { - if (fieldCtx.error) { - resolveErrorCh.publish(fieldCtx) - } - finishResolveCh.publish(fieldCtx) +function finalizeResolvers (contexts) { + for (const fieldCtx of contexts) { + fieldCtx.finalize() } } diff --git a/packages/datadog-plugin-graphql/src/resolve.js b/packages/datadog-plugin-graphql/src/resolve.js index 748275e03a3..50bc843f2bd 100644 --- a/packages/datadog-plugin-graphql/src/resolve.js +++ b/packages/datadog-plugin-graphql/src/resolve.js @@ -13,44 +13,40 @@ class GraphQLResolvePlugin extends TracingPlugin { if (this.config.collapse) field.depth += getListLevel(info.path) if (!this.shouldInstrument(field)) return - // we need to get the parent span to the field if it exists for correct span parenting - // of nested fields - const childOf = parentField ? this.getFieldContext(parentField).currentStore?.span : undefined + let childOf, path, ctx - let path, ctx if (this.config.collapse) { + const parent = parentField?.shared const sharedContexts = parentField - ? parentField.shared.collapsedChildren + ? parent.collapsedChildren : (rootCtx.toplevelCollapsed ??= {}) ctx = sharedContexts[info.fieldName] if (ctx) { // Add reference to the shared context object, - // for the `updateField` to store the `.finishTime` on + // for the `finish` subscription to store the `.finishTime` on // and for children resolvers to find `.collapsedChildren`. - // Should not be `finish`ed again and again though, so we don't use .finishCtx` field.shared = ctx this.enter(ctx.currentStore.span) // TODO: test this! return } + childOf = parent ? parent.currentStore?.span : undefined path = collapsePath(pathToArray(info.path)) // create shared context ctx = sharedContexts[info.fieldName] = { fieldName: info.fieldName, path, - parent: parentField?.shared, + parent, finishTime: 0, collapsedChildren: {}, // slightly pointless on leaf fields? // more to be added by `startSpan` } field.shared = ctx - // make this.finish(fieldCtx) be called before operation execution ends: - field.finishCtx = ctx + // make `field.finalize()` be called before operation execution ends: + field.finalize = finalizeCollapsedField } else { + childOf = parentField ? parentField.currentStore?.span : undefined path = pathToArray(info.path) ctx = field - // make this.finish(fieldCtx) be called before operation execution ends: - // and also to receive events when there's an `.error` on them - field.finishCtx = field } const document = rootCtx.source @@ -61,7 +57,7 @@ class GraphQLResolvePlugin extends TracingPlugin { const span = this.startSpan('graphql.resolve', { service: this.config.service, resource: `${info.fieldName}:${info.returnType}`, - childOf, + childOf, // span used by parent field (if it exists) for correct span parenting of nested fields type: 'graphql', meta: { 'graphql.field.name': info.fieldName, @@ -94,28 +90,30 @@ class GraphQLResolvePlugin extends TracingPlugin { // return ctx.currentStore // seems unused? This is not `bindStart`! } - constructor (...args) { - super(...args) - - this.addTraceSub('updateField', (field) => { - if (!this.shouldInstrument(field)) return - - const fieldCtx = this.getFieldContext(field) - const span = fieldCtx.currentStore?.span || this.activeSpan - fieldCtx.finishTime = span._getTime ? span._getTime() : 0 + finish (field) { + if (!this.shouldInstrument(field)) return - if (this.config.collapse && field.error) { - // the `field.shared` context is used to publish `error` events in `finishResolvers` :-/ - // FIXME: this should probably use `extractErrorIntoSpanEvent` - fieldCtx.error ??= field.error // notice the first error wins (like in `validate` and `execute`) + if (this.config.collapse) { + const fieldCtx = field.shared + const span = fieldCtx.currentStore?.span + if (field.error) { + fieldCtx.error ??= field.error + // TODO: use `extractErrorIntoSpanEvent` } - + fieldCtx.finishTime = span._getTime ? span._getTime() : 0 + this.config.hooks.resolve(span, field) + } else { + const span = field.currentStore?.span this.config.hooks.resolve(span, field) - }) + span?.finish() + } + } + + constructor (...args) { + super(...args) this.resolverStartCh = dc.channel('datadog:graphql:resolver:start') this.shouldInstrument = _field => false - this.getFieldContext = _field => null } configure (config) { @@ -125,23 +123,21 @@ class GraphQLResolvePlugin extends TracingPlugin { this.shouldInstrument = config.depth < 0 ? _field => true : field => config.depth >= field.depth - this.getFieldContext = config.collapse - ? field => field.shared - : field => field } +} - finish ({ finishTime, currentStore }) { - const span = currentStore?.span // no `|| this.activeSpan`, which might be anything - if (!span) return - // we care only about the span that was opened for the respective context - span.finish(finishTime) +// helpers - // return fieldCtx.parentStore +/** The method that is put as `.finalize` on a field when a shared context is created */ +function finalizeCollapsedField () { + const { shared } = this + const span = shared.currentStore?.span + if (shared.error) { // an error from any of the fields (that failed first) + span.setTag('error', shared.error) // like `TracingPlugin.prototype.error(shared)` } + span?.finish(shared.finishTime) } -// helpers - /** Count `*` chars in front of last path segment for a collapsed path */ function getListLevel (path) { let lvl = 0