Skip to content

remove async storage from graphql #5966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 86 additions & 119 deletions packages/datadog-instrumentations/src/graphql.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

const {
addHook,
channel,
AsyncResource
channel
} = require('./helpers/instrument')
const shimmer = require('../../datadog-shimmer')

Expand Down Expand Up @@ -89,29 +88,28 @@ function wrapParse (parse) {
return parse.apply(this, arguments)
}

const asyncResource = new AsyncResource('bound-anonymous-fn')

return asyncResource.runInAsyncScope(() => {
parseStartCh.publish()
let document
const ctx = { source }
return parseStartCh.runStores(ctx, () => {
try {
document = parse.apply(this, arguments)
const operation = getOperation(document)
ctx.document = parse.apply(this, arguments)
const operation = getOperation(ctx.document)

if (!operation) return document
if (!operation) return ctx.document

if (source) {
documentSources.set(document, source.body || source)
documentSources.set(ctx.document, source.body || source)
}
ctx.docSource = documentSources.get(ctx.document)

return document
return ctx.document
} catch (err) {
err.stack
parseErrorCh.publish(err)
ctx.error = err
parseErrorCh.publish(ctx)

throw err
} finally {
parseFinishCh.publish({ source, document, docSource: documentSources.get(document) })
parseFinishCh.publish(ctx)
}
})
}
Expand All @@ -123,25 +121,25 @@ function wrapValidate (validate) {
return validate.apply(this, arguments)
}

const asyncResource = new AsyncResource('bound-anonymous-fn')

return asyncResource.runInAsyncScope(() => {
validateStartCh.publish({ docSource: documentSources.get(document), document })

const ctx = { docSource: documentSources.get(document), document }
return validateStartCh.runStores(ctx, () => {
let errors
try {
errors = validate.apply(this, arguments)
if (errors && errors[0]) {
validateErrorCh.publish(errors && errors[0])
ctx.error = errors && errors[0]
validateErrorCh.publish(ctx)
}
return errors
} catch (err) {
err.stack
validateErrorCh.publish(err)
ctx.error = err
validateErrorCh.publish(ctx)

throw err
} finally {
validateFinishCh.publish({ document, errors })
ctx.errors = errors
validateFinishCh.publish(ctx)
}
})
}
Expand All @@ -155,15 +153,22 @@ function wrapExecute (execute) {
return exe.apply(this, arguments)
}

const asyncResource = new AsyncResource('bound-anonymous-fn')
return asyncResource.runInAsyncScope(() => {
const args = normalizeArgs(arguments, defaultFieldResolver)
const schema = args.schema
const document = args.document
const source = documentSources.get(document)
const contextValue = args.contextValue
const operation = getOperation(document, args.operationName)

const args = normalizeArgs(arguments, defaultFieldResolver)
const schema = args.schema
const document = args.document
const source = documentSources.get(document)
const contextValue = args.contextValue
const operation = getOperation(document, args.operationName)

const ctx = {
operation,
args,
docSource: documentSources.get(document),
source,
fields: {},
abortController: new AbortController()
}
return startExecuteCh.runStores(ctx, () => {
if (contexts.has(contextValue)) {
return exe.apply(this, arguments)
}
Expand All @@ -173,26 +178,20 @@ function wrapExecute (execute) {
wrapFields(schema._mutationType)
}

startExecuteCh.publish({
operation,
args,
docSource: documentSources.get(document)
})

const context = { source, asyncResource, fields: {}, abortController: new AbortController() }

contexts.set(contextValue, context)
contexts.set(contextValue, ctx)

return callInAsyncScope(exe, asyncResource, this, arguments, context.abortController, (err, res) => {
if (finishResolveCh.hasSubscribers) finishResolvers(context)
return callInAsyncScope(exe, this, arguments, ctx.abortController, (err, res) => {
if (finishResolveCh.hasSubscribers) finishResolvers(ctx)

const error = err || (res && res.errors && res.errors[0])

if (error) {
executeErrorCh.publish(error)
ctx.error = error
executeErrorCh.publish(ctx)
}

finishExecuteCh.publish({ res, args, context })
ctx.res = res
finishExecuteCh.publish(ctx)
})
})
}
Expand All @@ -205,14 +204,17 @@ function wrapResolve (resolve) {
function resolveAsync (source, args, contextValue, info) {
if (!startResolveCh.hasSubscribers) return resolve.apply(this, arguments)

const context = contexts.get(contextValue)
const ctx = contexts.get(contextValue)

if (!context) return resolve.apply(this, arguments)
if (!ctx) return resolve.apply(this, arguments)

const field = assertField(context, info, args)
const field = assertField(ctx, info, args)

return callInAsyncScope(resolve, field.asyncResource, this, arguments, context.abortController, (err) => {
updateFieldCh.publish({ field, info, err })
return callInAsyncScope(resolve, this, arguments, ctx.abortController, (err) => {
field.ctx.error = err
field.ctx.info = info
field.ctx.field = field
updateFieldCh.publish(field.ctx)
})
}

Expand All @@ -221,32 +223,30 @@ function wrapResolve (resolve) {
return resolveAsync
}

function callInAsyncScope (fn, aR, thisArg, args, abortController, cb) {
function callInAsyncScope (fn, thisArg, args, abortController, cb) {
cb = cb || (() => {})

return aR.runInAsyncScope(() => {
if (abortController?.signal.aborted) {
cb(null, null)
throw new AbortError('Aborted')
}
if (abortController?.signal.aborted) {
cb(null, null)
throw new AbortError('Aborted')
}

try {
const result = fn.apply(thisArg, args)
if (result && typeof result.then === 'function') {
// bind callback to this scope
result.then(
aR.bind(res => cb(null, res)),
aR.bind(err => cb(err))
)
} else {
cb(null, result)
}
return result
} catch (err) {
cb(err)
throw err
try {
const result = fn.apply(thisArg, args)
if (result && typeof result.then === 'function') {
// bind callback to this scope
result.then(
res => cb(null, res),
err => cb(err)
)
} else {
cb(null, result)
}
})
return result
} catch (err) {
cb(err)
throw err
}
}

function pathToArray (path) {
Expand All @@ -259,59 +259,26 @@ function pathToArray (path) {
return flattened.reverse()
}

function assertField (context, info, args) {
function assertField (parentCtx, info, args) {
const pathInfo = info && info.path

const path = pathToArray(pathInfo)

const pathString = path.join('.')
const fields = context.fields
const fields = parentCtx.fields

let field = fields[pathString]

if (!field) {
const parent = getParentField(context, path)

// we want to spawn the new span off of the parent, not a new async resource
parent.asyncResource.runInAsyncScope(() => {
/* this child resource will run a branched scope off of the parent resource, which
accesses the parent span from the storage unit in its own scope */
const childResource = new AsyncResource('bound-anonymous-fn')

childResource.runInAsyncScope(() => {
startResolveCh.publish({
info,
context,
args
})
})

field = fields[pathString] = {
parent,
asyncResource: childResource,
error: null
}
})
}

return field
}

function getParentField (context, path) {
for (let i = path.length - 1; i > 0; i--) {
const field = getField(context, path.slice(0, i))
if (field) {
return field
const fieldCtx = { info, ctx: parentCtx, args }
startResolveCh.publish(fieldCtx)
field = fields[pathString] = {
Comment on lines +274 to +275
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are replacing context property by ctx here, is the reason why appsec tests are failing.

If you want to maintain that, you have to modify the references of context in appsec subscriptions, like here:

error: null,
ctx: fieldCtx
}
}

return {
asyncResource: context.asyncResource
}
}

function getField (context, path) {
return context.fields[path.join('.')]
return field
}

function wrapFields (type) {
Expand Down Expand Up @@ -349,13 +316,13 @@ function wrapFieldType (field) {
function finishResolvers ({ fields }) {
Object.keys(fields).reverse().forEach(key => {
const field = fields[key]
const asyncResource = field.asyncResource
asyncResource.runInAsyncScope(() => {
if (field.error) {
resolveErrorCh.publish(field.error)
}
finishResolveCh.publish(field.finishTime)
})
field.ctx.finishTime = field.finishTime
field.ctx.field = field
if (field.error) {
field.ctx.error = field.error
resolveErrorCh.publish(field.ctx)
}
finishResolveCh.publish(field.ctx)
})
}

Expand Down
17 changes: 12 additions & 5 deletions packages/datadog-plugin-graphql/src/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ class GraphQLExecutePlugin extends TracingPlugin {
static get type () { return 'graphql' }
static get kind () { return 'server' }

start ({ operation, args, docSource }) {
bindStart (ctx) {
const { operation, args, docSource } = ctx

const type = operation && operation.operation
const name = operation && operation.name && operation.name.value
const document = args.document
Expand All @@ -27,20 +29,25 @@ class GraphQLExecutePlugin extends TracingPlugin {
'graphql.operation.name': name,
'graphql.source': source
}
})
}, ctx)

addVariableTags(this.config, span, args.variableValues)

return ctx.currentStore
}

finish ({ res, args }) {
const span = this.activeSpan
finish (ctx) {
const { res, args } = ctx
const span = ctx?.currentStore?.span || this.activeSpan
this.config.hooks.execute(span, args, res)
if (res?.errors) {
for (const err of res.errors) {
extractErrorIntoSpanEvent(this._tracerConfig, span, err)
}
}
super.finish()
super.finish(ctx)

return ctx.parentStore
}
}

Expand Down
15 changes: 10 additions & 5 deletions packages/datadog-plugin-graphql/src/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,29 @@ class GraphQLParsePlugin extends TracingPlugin {
static get id () { return 'graphql' }
static get operation () { return 'parser' }

start () {
bindStart (ctx) {
this.startSpan('graphql.parse', {
service: this.config.service,
type: 'graphql',
meta: {}
})
}, ctx)

return ctx.currentStore
}

finish ({ source, document, docSource }) {
const span = this.activeSpan
finish (ctx) {
const { source, document, docSource } = ctx
const span = ctx?.currentStore?.span || this.activeSpan

if (this.config.source && document) {
span.setTag('graphql.source', docSource)
}

this.config.hooks.parse(span, source, document)

super.finish()
super.finish(ctx)

return ctx.parentStore
}
}

Expand Down
Loading
Loading