From fb5361c79d20824f3ad4285fa6fea7e3980253f4 Mon Sep 17 00:00:00 2001 From: William Conti <58711692+wconti27@users.noreply.github.com> Date: Wed, 29 Jan 2025 15:20:08 -0500 Subject: [PATCH] chore(tracing): graphql error support (#5162) * add graphql error reporting via span links --- .../datadog-plugin-graphql/src/execute.js | 6 +++ packages/datadog-plugin-graphql/src/utils.js | 40 +++++++++++++++++++ .../datadog-plugin-graphql/src/validate.js | 6 +++ .../datadog-plugin-graphql/test/index.spec.js | 25 ++++++++++++ packages/dd-trace/src/config.js | 4 ++ packages/dd-trace/test/plugins/agent.js | 24 ++++++++++- 6 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 packages/datadog-plugin-graphql/src/utils.js diff --git a/packages/datadog-plugin-graphql/src/execute.js b/packages/datadog-plugin-graphql/src/execute.js index 60cede44e14..f0186983c70 100644 --- a/packages/datadog-plugin-graphql/src/execute.js +++ b/packages/datadog-plugin-graphql/src/execute.js @@ -1,6 +1,7 @@ 'use strict' const TracingPlugin = require('../../dd-trace/src/plugins/tracing') +const { extractErrorIntoSpanEvent } = require('./utils') let tools @@ -34,6 +35,11 @@ class GraphQLExecutePlugin extends TracingPlugin { finish ({ res, args }) { const 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() } } diff --git a/packages/datadog-plugin-graphql/src/utils.js b/packages/datadog-plugin-graphql/src/utils.js new file mode 100644 index 00000000000..844ed62442c --- /dev/null +++ b/packages/datadog-plugin-graphql/src/utils.js @@ -0,0 +1,40 @@ +function extractErrorIntoSpanEvent (config, span, exc) { + const attributes = {} + + if (exc.name) { + attributes.type = exc.name + } + + if (exc.stack) { + attributes.stacktrace = exc.stack + } + + if (exc.locations) { + attributes.locations = [] + for (const location of exc.locations) { + attributes.locations.push(`${location.line}:${location.column}`) + } + } + + if (exc.path) { + attributes.path = exc.path.map(String) + } + + if (exc.message) { + attributes.message = exc.message + } + + if (config.graphqlErrorExtensions) { + for (const ext of config.graphqlErrorExtensions) { + if (exc.extensions?.[ext]) { + attributes[`extensions.${ext}`] = exc.extensions[ext].toString() + } + } + } + + span.addEvent('dd.graphql.query.error', attributes, Date.now()) +} + +module.exports = { + extractErrorIntoSpanEvent +} diff --git a/packages/datadog-plugin-graphql/src/validate.js b/packages/datadog-plugin-graphql/src/validate.js index bda4886a6f0..2ed05179b31 100644 --- a/packages/datadog-plugin-graphql/src/validate.js +++ b/packages/datadog-plugin-graphql/src/validate.js @@ -1,6 +1,7 @@ 'use strict' const TracingPlugin = require('../../dd-trace/src/plugins/tracing') +const { extractErrorIntoSpanEvent } = require('./utils') class GraphQLValidatePlugin extends TracingPlugin { static get id () { return 'graphql' } @@ -21,6 +22,11 @@ class GraphQLValidatePlugin extends TracingPlugin { finish ({ document, errors }) { const span = this.activeSpan this.config.hooks.validate(span, document, errors) + if (errors) { + for (const err of errors) { + extractErrorIntoSpanEvent(this._tracerConfig, span, err) + } + } super.finish() } } diff --git a/packages/datadog-plugin-graphql/test/index.spec.js b/packages/datadog-plugin-graphql/test/index.spec.js index aa8c754f28a..609502762d9 100644 --- a/packages/datadog-plugin-graphql/test/index.spec.js +++ b/packages/datadog-plugin-graphql/test/index.spec.js @@ -920,6 +920,18 @@ describe('Plugin', () => { expect(spans[0].meta).to.have.property(ERROR_MESSAGE, errors[0].message) expect(spans[0].meta).to.have.property(ERROR_STACK, errors[0].stack) expect(spans[0].meta).to.have.property('component', 'graphql') + + const spanEvents = agent.unformatSpanEvents(spans[0]) + + expect(spanEvents).to.have.length(1) + expect(spanEvents[0]).to.have.property('startTime') + expect(spanEvents[0]).to.have.property('name', 'dd.graphql.query.error') + expect(spanEvents[0].attributes).to.have.property('type', 'GraphQLError') + expect(spanEvents[0].attributes).to.have.property('stacktrace') + expect(spanEvents[0].attributes).to.have.property('message', 'Field "address" of ' + + 'type "Address" must have a selection of subfields. Did you mean "address { ... }"?') + expect(spanEvents[0].attributes.locations).to.have.length(1) + expect(spanEvents[0].attributes.locations[0]).to.equal('1:11') }) .then(done) .catch(done) @@ -986,6 +998,19 @@ describe('Plugin', () => { expect(spans[0].meta).to.have.property(ERROR_MESSAGE, error.message) expect(spans[0].meta).to.have.property(ERROR_STACK, error.stack) expect(spans[0].meta).to.have.property('component', 'graphql') + + const spanEvents = agent.unformatSpanEvents(spans[0]) + + expect(spanEvents).to.have.length(1) + expect(spanEvents[0]).to.have.property('startTime') + expect(spanEvents[0]).to.have.property('name', 'dd.graphql.query.error') + expect(spanEvents[0].attributes).to.have.property('type', 'GraphQLError') + expect(spanEvents[0].attributes).to.have.property('stacktrace') + expect(spanEvents[0].attributes).to.have.property('message', 'test') + expect(spanEvents[0].attributes.locations).to.have.length(1) + expect(spanEvents[0].attributes.locations[0]).to.equal('1:3') + expect(spanEvents[0].attributes.path).to.have.length(1) + expect(spanEvents[0].attributes.path[0]).to.equal('hello') }) .then(done) .catch(done) diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index c2e8f28e565..221582a7a74 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -482,6 +482,7 @@ class Config { this._setValue(defaults, 'flushInterval', 2000) this._setValue(defaults, 'flushMinSpans', 1000) this._setValue(defaults, 'gitMetadataEnabled', true) + this._setValue(defaults, 'graphqlErrorExtensions', []) this._setValue(defaults, 'grpc.client.error.statuses', GRPC_CLIENT_ERROR_STATUSES) this._setValue(defaults, 'grpc.server.error.statuses', GRPC_SERVER_ERROR_STATUSES) this._setValue(defaults, 'headerTags', []) @@ -669,6 +670,7 @@ class Config { DD_TRACE_EXPERIMENTAL_RUNTIME_ID_ENABLED, DD_TRACE_GIT_METADATA_ENABLED, DD_TRACE_GLOBAL_TAGS, + DD_TRACE_GRAPHQL_ERROR_EXTENSIONS, DD_TRACE_HEADER_TAGS, DD_TRACE_LEGACY_BAGGAGE_ENABLED, DD_TRACE_MEMCACHED_COMMAND_ENABLED, @@ -895,6 +897,7 @@ class Config { this._setString(env, 'version', DD_VERSION || tags.version) this._setBoolean(env, 'inferredProxyServicesEnabled', DD_TRACE_INFERRED_PROXY_SERVICES_ENABLED) this._setString(env, 'aws.dynamoDb.tablePrimaryKeys', DD_AWS_SDK_DYNAMODB_TABLE_PRIMARY_KEYS) + this._setArray(env, 'graphqlErrorExtensions', DD_TRACE_GRAPHQL_ERROR_EXTENSIONS) } _applyOptions (options) { @@ -1020,6 +1023,7 @@ class Config { this._setBoolean(opts, 'traceId128BitLoggingEnabled', options.traceId128BitLoggingEnabled) this._setString(opts, 'version', options.version || tags.version) this._setBoolean(opts, 'inferredProxyServicesEnabled', options.inferredProxyServicesEnabled) + this._setBoolean(opts, 'graphqlErrorExtensions', options.graphqlErrorExtensions) // For LLMObs, we want the environment variable to take precedence over the options. // This is reliant on environment config being set before options. diff --git a/packages/dd-trace/test/plugins/agent.js b/packages/dd-trace/test/plugins/agent.js index 041cbf73967..8328c9e42b2 100644 --- a/packages/dd-trace/test/plugins/agent.js +++ b/packages/dd-trace/test/plugins/agent.js @@ -87,6 +87,27 @@ function dsmStatsExistWithParentHash (agent, expectedParentHash) { return hashFound } +function unformatSpanEvents (span) { + if (span.meta && span.meta.events) { + // Parse the JSON string back into an object + const events = JSON.parse(span.meta.events) + + // Create the _events array + const spanEvents = events.map(event => { + return { + name: event.name, + startTime: event.time_unix_nano / 1e6, // Convert from nanoseconds back to milliseconds + attributes: event.attributes ? event.attributes : undefined + } + }) + + // Return the unformatted _events + return spanEvents + } + + return [] // Return an empty array if no events are found +} + function addEnvironmentVariablesToHeaders (headers) { // get all environment variables that start with "DD_" const ddEnvVars = new Map( @@ -443,5 +464,6 @@ module.exports = { testedPlugins, getDsmStats, dsmStatsExist, - dsmStatsExistWithParentHash + dsmStatsExistWithParentHash, + unformatSpanEvents }