From f5683d70e4753abe90e96c3723cbdffdcb24bf23 Mon Sep 17 00:00:00 2001 From: Rob Wells Date: Thu, 5 Jul 2018 15:18:08 +0100 Subject: [PATCH 1/4] Deeper nesting of graphql tracer --- src/plugins/graphql.js | 17 ++++++++++--- test/plugins/graphql.spec.js | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/plugins/graphql.js b/src/plugins/graphql.js index 6fe39d6c22d..ca3eeba7bcd 100644 --- a/src/plugins/graphql.js +++ b/src/plugins/graphql.js @@ -56,20 +56,25 @@ function wrapFields (fields, tracer, config, responsePathAsArray) { Object.keys(fields).forEach(key => { const field = fields[key] - if (typeof field.resolve === 'function') { + if (typeof field.resolve === 'function' && !field.resolve._datadog_patched) { field.resolve = wrapResolve(field.resolve, tracer, config, responsePathAsArray) } - if (field.type && field.type._fields) { - wrapFields(field.type._fields, tracer, config, responsePathAsArray) + if (field.type) { + if (field.type._fields) { + wrapFields(field.type._fields, tracer, config, responsePathAsArray) + } else if (field.type.ofType && field.type.ofType._fields) { + wrapFields(field.type.ofType._fields, tracer, config, responsePathAsArray) + } } }) } function wrapResolve (resolve, tracer, config, responsePathAsArray) { - return function resolveWithTrace (source, args, contextValue, info) { + function resolveWithTrace (source, args, contextValue, info) { const path = responsePathAsArray(info.path) const fieldParent = getFieldParent(contextValue, path) + const childOf = createSpan('graphql.field', tracer, config, fieldParent, path) const deferred = defer(tracer) @@ -88,6 +93,10 @@ function wrapResolve (resolve, tracer, config, responsePathAsArray) { return result } + + resolveWithTrace._datadog_patched = true + + return resolveWithTrace } function wrapFieldResolver (fieldResolver, tracer, config, responsePathAsArray) { diff --git a/test/plugins/graphql.spec.js b/test/plugins/graphql.spec.js index c283cb79e75..57eefe95999 100644 --- a/test/plugins/graphql.spec.js +++ b/test/plugins/graphql.spec.js @@ -38,6 +38,38 @@ describe('Plugin', () => { resolve (obj, args) { return {} } + }, + pets: { + type: new graphql.GraphQLList(new graphql.GraphQLObjectType({ + name: 'Pet', + fields: { + type: { + type: graphql.GraphQLString, + resolve: () => 'dog' + }, + name: { + type: graphql.GraphQLString, + resolve: () => 'foo bar' + }, + colours: { + type: new graphql.GraphQLList(new graphql.GraphQLObjectType({ + name: 'Colour', + fields: { + code: { + type: graphql.GraphQLString, + resolve: () => '#ffffff' + } + } + })), + resolve (obj, args) { + return [{}, {}] + } + } + } + })), + resolve (obj, args) { + return [{}, {}, {}] + } } } }) @@ -288,6 +320,21 @@ describe('Plugin', () => { graphql.graphql(schema, source).catch(done) }) + it('should instrument nested lists', done => { + const source = `{ friends { name pets { name colours { code } } } }` + + agent + .use(traces => { + const spans = sort(traces[0]) + expect(spans.filter(span => span.name === 'graphql.field').map(span => span.resource)).to.have.length(29) + }) + .then(done) + .catch(done) + + graphql.graphql(schema, source) + .catch(done) + }) + it('should ignore the default field resolver', done => { const schema = graphql.buildSchema(` type Query { From ae0d532eb6beaa20a03ccc5e4c6df0781ea66c79 Mon Sep 17 00:00:00 2001 From: Rob Wells Date: Thu, 5 Jul 2018 16:06:54 +0100 Subject: [PATCH 2/4] Handle recursively nested schemas --- src/plugins/graphql.js | 22 +++++++++++++++------- test/plugins/graphql.spec.js | 8 ++++++-- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/plugins/graphql.js b/src/plugins/graphql.js index ca3eeba7bcd..ea6841170e2 100644 --- a/src/plugins/graphql.js +++ b/src/plugins/graphql.js @@ -20,7 +20,7 @@ function createWrapExecute (tracer, config, defaultFieldResolver, responsePathAs args.contextValue = contextValue if (!schema._datadog_patched) { - wrapFields(schema._queryType._fields, tracer, config, responsePathAsArray) + wrapFields(schema._queryType, tracer, config, responsePathAsArray) schema._datadog_patched = true } @@ -52,19 +52,27 @@ function createWrapParse () { } } -function wrapFields (fields, tracer, config, responsePathAsArray) { - Object.keys(fields).forEach(key => { - const field = fields[key] +function wrapFields (type, tracer, config, responsePathAsArray) { + + if (type._datadog_patched) { + return + } + + type._datadog_patched = true + + Object.keys(type._fields).forEach(key => { + const field = type._fields[key] if (typeof field.resolve === 'function' && !field.resolve._datadog_patched) { field.resolve = wrapResolve(field.resolve, tracer, config, responsePathAsArray) } - if (field.type) { + + if (field.type && !field.type._datadog_patched) { if (field.type._fields) { - wrapFields(field.type._fields, tracer, config, responsePathAsArray) + wrapFields(field.type, tracer, config, responsePathAsArray) } else if (field.type.ofType && field.type.ofType._fields) { - wrapFields(field.type.ofType._fields, tracer, config, responsePathAsArray) + wrapFields(field.type.ofType, tracer, config, responsePathAsArray) } } }) diff --git a/test/plugins/graphql.spec.js b/test/plugins/graphql.spec.js index 57eefe95999..ff0220ef7be 100644 --- a/test/plugins/graphql.spec.js +++ b/test/plugins/graphql.spec.js @@ -42,7 +42,7 @@ describe('Plugin', () => { pets: { type: new graphql.GraphQLList(new graphql.GraphQLObjectType({ name: 'Pet', - fields: { + fields: () => ({ type: { type: graphql.GraphQLString, resolve: () => 'dog' @@ -51,6 +51,10 @@ describe('Plugin', () => { type: graphql.GraphQLString, resolve: () => 'foo bar' }, + owner: { + type: Human, + resolve: () => {} + }, colours: { type: new graphql.GraphQLList(new graphql.GraphQLObjectType({ name: 'Colour', @@ -65,7 +69,7 @@ describe('Plugin', () => { return [{}, {}] } } - } + }) })), resolve (obj, args) { return [{}, {}, {}] From 8407c834b1380a3a34e7c33e5831af3193aa476c Mon Sep 17 00:00:00 2001 From: Rob Wells Date: Fri, 6 Jul 2018 08:08:59 +0100 Subject: [PATCH 3/4] Fix lint issues and remove check --- src/plugins/graphql.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/plugins/graphql.js b/src/plugins/graphql.js index ea6841170e2..b86f0788d3c 100644 --- a/src/plugins/graphql.js +++ b/src/plugins/graphql.js @@ -53,7 +53,6 @@ function createWrapParse () { } function wrapFields (type, tracer, config, responsePathAsArray) { - if (type._datadog_patched) { return } @@ -67,8 +66,7 @@ function wrapFields (type, tracer, config, responsePathAsArray) { field.resolve = wrapResolve(field.resolve, tracer, config, responsePathAsArray) } - - if (field.type && !field.type._datadog_patched) { + if (field.type) { if (field.type._fields) { wrapFields(field.type, tracer, config, responsePathAsArray) } else if (field.type.ofType && field.type.ofType._fields) { From 3e0bb0bc398dfd63cc8d2eea18478f861d1e0052 Mon Sep 17 00:00:00 2001 From: Rob Wells Date: Mon, 9 Jul 2018 08:04:57 +0100 Subject: [PATCH 4/4] Added test for a circular schema --- test/plugins/graphql.spec.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/test/plugins/graphql.spec.js b/test/plugins/graphql.spec.js index ff0220ef7be..1a5f21597b7 100644 --- a/test/plugins/graphql.spec.js +++ b/test/plugins/graphql.spec.js @@ -53,7 +53,7 @@ describe('Plugin', () => { }, owner: { type: Human, - resolve: () => {} + resolve: () => ({}) }, colours: { type: new graphql.GraphQLList(new graphql.GraphQLObjectType({ @@ -324,18 +324,15 @@ describe('Plugin', () => { graphql.graphql(schema, source).catch(done) }) - it('should instrument nested lists', done => { - const source = `{ friends { name pets { name colours { code } } } }` - - agent - .use(traces => { - const spans = sort(traces[0]) - expect(spans.filter(span => span.name === 'graphql.field').map(span => span.resource)).to.have.length(29) - }) - .then(done) - .catch(done) + it('should handle a circular schema', done => { + const source = `{ human { pets { owner { name } } } }` graphql.graphql(schema, source) + .then((result) => { + expect(result.data.human.pets[0].owner.name).to.equal('test') + + done() + }) .catch(done) })