Skip to content
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

GraphQL Blocking #3819

Merged
merged 73 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from 71 commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
ca016be
Upload module skeleton.
hoolioh Nov 23, 2023
7233810
Blocking in apollo, very very first version
uurien Nov 23, 2023
007204a
Move graphql implementation to another module.
hoolioh Nov 24, 2023
1bee1ae
Merge branch 'julio/graphql-resolver-handler' into julio/graphql-bloc…
hoolioh Nov 24, 2023
6113ee1
Blocking for apollo-server-core, ugly but it works, lets find a bette…
uurien Nov 24, 2023
f8c52e9
Use real blocking data
uurien Nov 24, 2023
7f05909
Set blocking to true.
hoolioh Nov 24, 2023
ece953f
Throw before resolver execution in order to stop the operation's exec…
hoolioh Nov 24, 2023
2aa7488
Use HttpQueryError in apollo-server-core
uurien Nov 24, 2023
3c7d6a7
Merge branch 'julio/graphql-blocking' of github.com:DataDog/dd-trace-…
hoolioh Nov 24, 2023
d74cf95
Merge branch 'julio/graphql-blocking' of github.com:DataDog/dd-trace-…
hoolioh Nov 24, 2023
51640f2
Blocking test in apollo-server-fastify
uurien Nov 27, 2023
a2ed730
Refactor graphql blocking.
hoolioh Nov 27, 2023
a8f2186
Add non blocking graphql test
uurien Nov 27, 2023
464ad05
Move abortController constructor to context creation.
hoolioh Nov 27, 2023
b2fb5d6
Add pollo-server-express block tests
uurien Nov 27, 2023
f45d1ed
Add unit tests.
hoolioh Nov 27, 2023
d195082
Merge branch 'julio/graphql-blocking' of github.com:DataDog/dd-trace-…
hoolioh Nov 27, 2023
4ad350c
Add @apollo/server tests
uurien Nov 27, 2023
a059bfd
Update test rules for blocking by `graphql.server.resolver`
uurien Nov 27, 2023
76483a0
Block with graphql templates data
uurien Nov 27, 2023
e982e80
Add tests.
hoolioh Nov 27, 2023
6498d9f
Merge branch 'julio/graphql-blocking' of github.com:DataDog/dd-trace-…
hoolioh Nov 27, 2023
3f5653b
Block with graphql data in graphql endpoint
uurien Nov 27, 2023
5dd121b
Fix tests.
hoolioh Nov 27, 2023
7cc8bba
Execute @apollo/server and apollo-server-express tests
uurien Nov 28, 2023
7542f8a
Unify code in @apollo/server and apollo-server-core
uurien Nov 28, 2023
bbcebcf
update comments
uurien Nov 28, 2023
71a3524
Add appsec.blocked tag in blocked requests
uurien Nov 28, 2023
d9d8050
fix test
uurien Nov 28, 2023
e417adb
Fix lint
uurien Nov 28, 2023
fe6edbb
Add test with non graphql block response
uurien Nov 28, 2023
2410db5
small fix
uurien Nov 28, 2023
df4d73d
Tests for block with redirect
uurien Nov 28, 2023
61b18ac
Fix integration test
uurien Nov 29, 2023
c944acc
Fix graphql plugin tests
uurien Nov 29, 2023
9ce8d98
Prevent creation of resolve span when it is blocked before the execut…
uurien Nov 30, 2023
cf7baa8
Refactor addResolver in order to get directives information.
hoolioh Nov 30, 2023
9cba641
Add tests to block on directives.
hoolioh Nov 30, 2023
12427b3
Add test for directives.
hoolioh Nov 30, 2023
82db1d4
Undo prevent creating resolve span
uurien Dec 1, 2023
1a238bf
Configurable graphql blocking json
uurien Dec 1, 2023
a685a01
Refactor graphql
hoolioh Dec 1, 2023
1d4524f
Merge branch 'julio/graphql-blocking' of github.com:DataDog/dd-trace-…
hoolioh Dec 1, 2023
46039a7
Fix integration tests.
hoolioh Dec 1, 2023
727456e
fix channel name
uurien Dec 1, 2023
35d4aeb
Small fixes
uurien Dec 1, 2023
5c293ee
Small changes in blocking
uurien Dec 1, 2023
d9e6ce3
Small changes
uurien Dec 1, 2023
7cc8561
Move resover information resolution to plugin.
hoolioh Dec 1, 2023
62dc02a
Revert "Move resover information resolution to plugin."
hoolioh Dec 1, 2023
a941309
Remove resolver information from context, pass it in a different fiel…
hoolioh Dec 4, 2023
d94df58
Throw custom exception rather than send an empty array.
hoolioh Dec 4, 2023
80a4e01
Change exception name
hoolioh Dec 4, 2023
08b3b2b
Update packages/datadog-instrumentations/src/graphql.js
hoolioh Dec 4, 2023
e310e5c
Change a bit apollo-server-core instrumentation
uurien Dec 4, 2023
5fbb4c5
Small code changes
uurien Dec 5, 2023
8be2238
Small changes
uurien Dec 5, 2023
204b405
Protect Header map, if in future version it is moved/removed, prevent…
uurien Dec 5, 2023
3416b3f
Remove some duplicated code
uurien Dec 5, 2023
0347483
Fix blocking tests
uurien Dec 5, 2023
1b1d481
Fix blocking tests
uurien Dec 5, 2023
4c5578b
Small fixes
uurien Dec 5, 2023
44187c6
Update packages/datadog-instrumentations/src/apollo-server.js
uurien Dec 11, 2023
c820226
Fix comments in the PR
uurien Dec 12, 2023
a293f51
Fix PR comments.
hoolioh Dec 13, 2023
c45af94
Fix some comments in the PR
uurien Dec 14, 2023
26f64d0
Fix test
uurien Dec 14, 2023
b4a23e6
Fix lint
uurien Dec 14, 2023
80b3c21
Move resolver information formatting to the plugin.
hoolioh Dec 15, 2023
026aaaa
Fix PR comments.
hoolioh Dec 19, 2023
0b83ea1
Fix tests.
hoolioh Dec 19, 2023
8f427dc
Fix proper use of Promise.race.
hoolioh Dec 19, 2023
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
14 changes: 14 additions & 0 deletions .github/workflows/appsec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,20 @@ jobs:
- run: yarn test:appsec:plugins:ci
- uses: codecov/codecov-action@v2

graphql:
runs-on: ubuntu-latest
env:
PLUGINS: apollo-server|apollo-server-express|apollo-server-fastify|apollo-server-core
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/node/setup
- run: yarn install
- uses: ./.github/actions/node/oldest
- run: yarn test:appsec:plugins:ci
- uses: ./.github/actions/node/latest
- run: yarn test:appsec:plugins:ci
- uses: codecov/codecov-action@v2

mongodb-core:
runs-on: ubuntu-latest
services:
Expand Down
1 change: 1 addition & 0 deletions docs/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ tracer.init({
obfuscatorValueRegex: '.*',
blockedTemplateHtml: './blocked.html',
blockedTemplateJson: './blocked.json',
blockedTemplateGraphql: './blockedgraphql.json',
eventTracking: {
mode: 'safe'
},
Expand Down
5 changes: 5 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,11 @@ export declare interface TracerOptions {
*/
blockedTemplateJson?: string,

/**
* Specifies a path to a custom blocking template json file for graphql requests
*/
blockedTemplateGraphql?: string,

/**
* Controls the automated user event tracking configuration
*/
Expand Down
5 changes: 2 additions & 3 deletions integration-tests/graphql.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ describe('graphql', () => {
{
id: 'test-rule-id-1',
name: 'test-rule-name-1',
on_match: ['block'],
tags:
{
category: 'attack_attempt',
Expand All @@ -92,8 +91,8 @@ describe('graphql', () => {
operator_value: '',
parameters: [
{
address: 'graphql.server.all_resolvers',
key_path: ['images', '0', 'category'],
address: 'graphql.server.resolver',
key_path: ['images', 'category'],
value: 'testattack',
highlight: ['testattack']
}
Expand Down
5 changes: 4 additions & 1 deletion integration-tests/graphql/graphql-rules.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
"inputs": [
{
"address": "graphql.server.all_resolvers"
},
{
"address": "graphql.server.resolver"
}
],
"list": [
Expand All @@ -27,7 +30,7 @@
}
],
"transformers": ["lowercase"],
"on_match": ["block"]
"on_match": []
}
]
}
40 changes: 40 additions & 0 deletions packages/datadog-instrumentations/src/apollo-server-core.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict'

const { AbortController } = require('node-abort-controller')
const { addHook } = require('./helpers/instrument')
const shimmer = require('../../datadog-shimmer')
const dc = require('dc-polyfill')

const requestChannel = dc.tracingChannel('datadog:apollo-server-core:request')

addHook({ name: 'apollo-server-core', file: 'dist/runHttpQuery.js', versions: ['>3.0.0'] }, runHttpQueryModule => {
const HttpQueryError = runHttpQueryModule.HttpQueryError

shimmer.wrap(runHttpQueryModule, 'runHttpQuery', function wrapRunHttpQuery (originalRunHttpQuery) {
return async function runHttpQuery () {
if (!requestChannel.start.hasSubscribers) {
return originalRunHttpQuery.apply(this, arguments)
}

const abortController = new AbortController()
const abortData = {}

const runHttpQueryResult = requestChannel.tracePromise(
originalRunHttpQuery,
{ abortController, abortData },
this,
...arguments)

return Promise.race([runHttpQueryResult]).then((value) => {
if (abortController.signal.aborted) {
// runHttpQuery callbacks are writing the response on resolve/reject.
// We should return blocking data in the apollo-server-core HttpQueryError object
return Promise.reject(new HttpQueryError(abortData.statusCode, abortData.message, true, abortData.headers))
}
return value
})
Qard marked this conversation as resolved.
Show resolved Hide resolved
}
})

return runHttpQueryModule
})
83 changes: 83 additions & 0 deletions packages/datadog-instrumentations/src/apollo-server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
'use strict'

const { AbortController } = require('node-abort-controller')
const dc = require('dc-polyfill')

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

const graphqlMiddlewareChannel = dc.tracingChannel('datadog:apollo:middleware')

const requestChannel = dc.tracingChannel('datadog:apollo:request')

let HeaderMap
uurien marked this conversation as resolved.
Show resolved Hide resolved

function wrapExecuteHTTPGraphQLRequest (originalExecuteHTTPGraphQLRequest) {
return async function executeHTTPGraphQLRequest () {
if (!HeaderMap || !requestChannel.start.hasSubscribers) {
return originalExecuteHTTPGraphQLRequest.apply(this, arguments)
}

const abortController = new AbortController()
const abortData = {}

const graphqlResponseData = requestChannel.tracePromise(
originalExecuteHTTPGraphQLRequest,
{ abortController, abortData },
this,
...arguments)

return Promise.race([graphqlResponseData]).then((value) => {
if (abortController.signal.aborted) {
// This method is expected to return response data
// with headers, status and body
const headers = new HeaderMap()
Object.keys(abortData.headers).forEach(key => {
headers.set(key, abortData.headers[key])
})

return {
headers: headers,
status: abortData.statusCode,
body: {
kind: 'complete',
string: abortData.message
}
}
}

return graphqlResponseData
})
}
}

function apolloExpress4Hook (express4) {
shimmer.wrap(express4, 'expressMiddleware', function wrapExpressMiddleware (originalExpressMiddleware) {
return function expressMiddleware (server, options) {
const originalMiddleware = originalExpressMiddleware.apply(this, arguments)

return shimmer.wrap(originalMiddleware, function (req, res, next) {
if (!graphqlMiddlewareChannel.start.hasSubscribers) {
return originalMiddleware.apply(this, arguments)
}

return graphqlMiddlewareChannel.traceSync(originalMiddleware, { req }, this, ...arguments)
})
}
})
return express4
}

function apolloHeaderMapHook (headerMap) {
HeaderMap = headerMap.HeaderMap
return headerMap
}

function apolloServerHook (apolloServer) {
shimmer.wrap(apolloServer.ApolloServer.prototype, 'executeHTTPGraphQLRequest', wrapExecuteHTTPGraphQLRequest)
return apolloServer
}

addHook({ name: '@apollo/server', file: 'dist/cjs/ApolloServer.js', versions: ['>=4.0.0'] }, apolloServerHook)
addHook({ name: '@apollo/server', file: 'dist/cjs/express4/index.js', versions: ['>=4.0.0'] }, apolloExpress4Hook)
addHook({ name: '@apollo/server', file: 'dist/cjs/utils/HeaderMap.js', versions: ['>=4.0.0'] }, apolloHeaderMapHook)
22 changes: 18 additions & 4 deletions packages/datadog-instrumentations/src/graphql.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict'

const { AbortController } = require('node-abort-controller')

const {
addHook,
channel,
Expand Down Expand Up @@ -37,6 +39,13 @@ 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'
}
}

function getOperation (document, operationName) {
if (!document || !Array.isArray(document.definitions)) {
return
Expand Down Expand Up @@ -175,11 +184,11 @@ function wrapExecute (execute) {
docSource: documentSources.get(document)
})

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

contexts.set(contextValue, context)

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

const error = err || (res && res.errors && res.errors[0])
Expand Down Expand Up @@ -207,7 +216,7 @@ function wrapResolve (resolve) {

const field = assertField(context, info, args)

return callInAsyncScope(resolve, field.asyncResource, this, arguments, (err) => {
return callInAsyncScope(resolve, field.asyncResource, this, arguments, context.abortController, (err) => {
updateFieldCh.publish({ field, info, err })
})
}
Expand All @@ -217,10 +226,15 @@ function wrapResolve (resolve) {
return resolveAsync
}

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

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

try {
const result = fn.apply(thisArg, args)
if (result && typeof result.then === 'function') {
Expand Down
2 changes: 2 additions & 0 deletions packages/datadog-instrumentations/src/helpers/hooks.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict'

module.exports = {
'@apollo/server': () => require('../apollo-server'),
'apollo-server-core': () => require('../apollo-server-core'),
'@aws-sdk/smithy-client': () => require('../aws-sdk'),
'@cucumber/cucumber': () => require('../cucumber'),
'@playwright/test': () => require('../playwright'),
Expand Down
44 changes: 26 additions & 18 deletions packages/datadog-plugin-graphql/src/resolve.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'

const TracingPlugin = require('../../dd-trace/src/plugins/tracing')
const dc = require('dc-polyfill')

const collapsedPathSym = Symbol('collapsedPaths')

Expand All @@ -14,8 +15,6 @@ class GraphQLResolvePlugin extends TracingPlugin {
if (!shouldInstrument(this.config, path)) return
const computedPathString = path.join('.')

addResolver(context, info, args)

if (this.config.collapse) {
if (!context[collapsedPathSym]) {
context[collapsedPathSym] = {}
Expand Down Expand Up @@ -55,6 +54,10 @@ class GraphQLResolvePlugin extends TracingPlugin {
span.setTag(`graphql.variables.${name}`, variables[name])
})
}

if (this.resolverStartCh.hasSubscribers) {
this.resolverStartCh.publish({ context, resolverInfo: getResolverInfo(info, args) })
}
}

constructor (...args) {
Expand All @@ -69,6 +72,8 @@ class GraphQLResolvePlugin extends TracingPlugin {
field.finishTime = span._getTime ? span._getTime() : 0
field.error = field.error || err
})

this.resolverStartCh = dc.channel('datadog:graphql:resolver:start')
}

configure (config) {
Expand Down Expand Up @@ -109,28 +114,31 @@ function withCollapse (responsePathAsArray) {
}
}

function addResolver (context, info, args) {
if (info.rootValue && !info.rootValue[info.fieldName]) {
return
}
function getResolverInfo (info, args) {
let resolverInfo = null
const resolverVars = {}

if (!context.resolvers) {
context.resolvers = {}
if (args && Object.keys(args).length) {
Object.assign(resolverVars, args)
}

const resolvers = context.resolvers

if (!resolvers[info.fieldName]) {
if (args && Object.keys(args).length) {
resolvers[info.fieldName] = [args]
} else {
resolvers[info.fieldName] = []
const directives = info.fieldNodes[0].directives
for (const directive of directives) {

Choose a reason for hiding this comment

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

This line is breaking to me.

I am using KeystoneJS and the schema generated is not returning directives sometimes.
For example:

🚀 ~ getResolverInfo ~ info.fieldNodes[0]: {
  kind: 'Field',
  name: { kind: 'Name', value: 'user' },
  arguments: [ { kind: 'Argument', name: [Object], value: [Object] } ],
  selectionSet: {
    kind: 'SelectionSet',
    selections: [ [Object], [Object], [Object] ],
    loc: Location {
      start: 19,
      end: 108,
      startToken: [Token],
      endToken: [Token],
      source: [Source]
    }
  }
}

Of course, the error I am getting is: TypeError: directives is not iterable.


Should we validate if directives is effectively iterable before the for?

Copy link
Member

Choose a reason for hiding this comment

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

We'll investigate and fix this bug as fast as possible. Thank you for reporting the issue!

Choose a reason for hiding this comment

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

Thank you @simon-id! I've just created an issue with the same topic and more details 😄

#4097

Copy link
Member

Choose a reason for hiding this comment

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

Oh wow perfect, I was about to create one too. Thank you for being proactive and let's continue this discussion there.

const argList = {}
for (const argument of directive['arguments']) {
argList[argument.name.value] = argument.value.value
}
} else {
if (args && Object.keys(args).length) {
resolvers[info.fieldName].push(args)

if (Object.keys(argList).length) {
resolverVars[directive.name.value] = argList
}
}

if (Object.keys(resolverVars).length) {
resolverInfo = { [info.fieldName]: resolverVars }
}

return resolverInfo
}

module.exports = GraphQLResolvePlugin
1 change: 1 addition & 0 deletions packages/dd-trace/src/appsec/addresses.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module.exports = {
HTTP_INCOMING_RESPONSE_HEADERS: 'server.response.headers.no_cookies',
// TODO: 'server.response.trailers',
HTTP_INCOMING_GRAPHQL_RESOLVERS: 'graphql.server.all_resolvers',
HTTP_INCOMING_GRAPHQL_RESOLVER: 'graphql.server.resolver',

HTTP_CLIENT_IP: 'http.client_ip',

Expand Down
5 changes: 4 additions & 1 deletion packages/dd-trace/src/appsec/blocked_templates.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading