Skip to content

Commit

Permalink
fix apollo gateway get signature method (#4174)
Browse files Browse the repository at this point in the history 
* fix apollo gateway get signature method & add checks for config values
  • Loading branch information
@khanayan123
khanayan123 authored Mar 20, 2024
1 parent c9e1082 commit 8cc7705
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 22 deletions.
21 changes: 20 additions & 1 deletion index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,26 @@ declare namespace tracer {
* This module uses graphql operations to service requests & thus generates graphql spans.
* We recommend disabling the graphql plugin if you only want to trace @apollo/gateway
*/
interface apollo extends Instrumentation {}
interface apollo extends Instrumentation {
/**
* Whether to include the source of the operation within the query as a tag
* on every span. This may contain sensitive information and should only be
* enabled if sensitive data is always sent as variables and not in the
* query text.
*
* @default false
*/
source?: boolean;

/**
* Whether to enable signature calculation for the resource name. This can
* be disabled if your apollo/gateway operations always have a name. Note that when
* disabled all queries will need to be named for this to work properly.
*
* @default true
*/
signature?: boolean;
}

/**
* This plugin automatically instruments the
Expand Down
4 changes: 2 additions & 2 deletions packages/datadog-plugin-apollo/src/gateway/request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class ApolloGatewayRequestPlugin extends ApolloBasePlugin {
if (requestContext?.operationName) {
spanData.meta['graphql.operation.name'] = requestContext.operationName
}
if (gateway?.config?.telemetry?.includeDocument !== false && requestContext?.source) {
if ((this.config.source || gateway?.config?.telemetry?.includeDocument) && requestContext?.source) {
spanData.meta['graphql.source'] = requestContext.source
}

Expand Down Expand Up @@ -121,7 +121,7 @@ function getSignature (document, operationName, operationType, calculate) {
if (calculate !== false && tools !== false) {
try {
try {
tools = tools || require('./tools')
tools = tools || require('../../../datadog-plugin-graphql/src/tools')
} catch (e) {
tools = false
throw e
Expand Down
34 changes: 15 additions & 19 deletions packages/datadog-plugin-apollo/test/index.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,18 @@ describe('Plugin', () => {
})
it('should instrument apollo/gateway', done => {
const operationName = 'MyQuery'
const resource = `query ${operationName}`
const source = `${resource} { hello(name: "world") }`
const source = `query ${operationName} { hello(name: "world") }`
const variableValues = { who: 'world' }
agent
.use((traces) => {
// the spans are in order of execution
expect(traces[0][0]).to.have.property('name', expectedSchema.server.opName)
expect(traces[0][0]).to.have.property('service', expectedSchema.server.serviceName)
expect(traces[0][0]).to.have.property('resource', resource)
expect(traces[0][0]).to.have.property('resource', 'query MyQuery{hello(name:"")}')
expect(traces[0][0]).to.have.property('type', 'web')
expect(traces[0][0]).to.have.property('error', 0)
expect(traces[0][0].meta).to.have.property('graphql.operation.name', operationName)
expect(traces[0][0].meta).to.have.property('graphql.source', source)
expect(traces[0][0].meta).to.not.have.property('graphql.source')
expect(traces[0][0].meta).to.have.property('graphql.operation.type', 'query')
expect(traces[0][0].meta).to.have.property('component', 'apollo.gateway')

Expand Down Expand Up @@ -196,10 +195,10 @@ describe('Plugin', () => {
.use((traces) => {
expect(traces[0][0]).to.have.property('name', expectedSchema.server.opName)
expect(traces[0][0]).to.have.property('service', expectedSchema.server.serviceName)
expect(traces[0][0]).to.have.property('resource', 'query')
expect(traces[0][0]).to.have.property('resource', '{hello(name:"")}')
expect(traces[0][0]).to.have.property('type', 'web')
expect(traces[0][0]).to.have.property('error', 0)
expect(traces[0][0].meta).to.have.property('graphql.source', source)
expect(traces[0][0].meta).to.not.have.property('graphql.source')
expect(traces[0][0].meta).to.have.property('graphql.operation.type', 'query')
expect(traces[0][0].meta).to.have.property('component', 'apollo.gateway')
})
Expand Down Expand Up @@ -228,10 +227,10 @@ describe('Plugin', () => {
.use((traces) => {
expect(traces[0][0]).to.have.property('name', expectedSchema.server.opName)
expect(traces[0][0]).to.have.property('service', expectedSchema.server.serviceName)
expect(traces[0][0]).to.have.property('resource', 'query')
expect(traces[0][0]).to.have.property('resource', '{human{address{civicNumber street}name}}')
expect(traces[0][0]).to.have.property('type', 'web')
expect(traces[0][0]).to.have.property('error', 0)
expect(traces[0][0].meta).to.have.property('graphql.source', source)
expect(traces[0][0].meta).to.not.have.property('graphql.source')
expect(traces[0][0].meta).to.have.property('graphql.operation.type', 'query')
expect(traces[0][0].meta).to.have.property('component', 'apollo.gateway')
})
Expand Down Expand Up @@ -315,8 +314,7 @@ describe('Plugin', () => {
it('should instrument plan failure', done => {
let error
const operationName = 'MyQuery'
const resource = `subscription ${operationName}`
const source = `${resource} { hello(name: "world") }`
const source = `subscription ${operationName} { hello(name: "world") }`
const variableValues = { who: 'world' }
agent
.use((traces) => {
Expand Down Expand Up @@ -350,8 +348,7 @@ describe('Plugin', () => {
it('should instrument fetch failure', done => {
let error
const operationName = 'MyQuery'
const resource = `query ${operationName}`
const source = `${resource} { hello(name: "world") }`
const source = `query ${operationName} { hello(name: "world") }`
const variableValues = { who: 'world' }
agent
.use((traces) => {
Expand Down Expand Up @@ -410,8 +407,7 @@ describe('Plugin', () => {

it('should run spans in the correct context', done => {
const operationName = 'MyQuery'
const resource = `query ${operationName}`
const source = `${resource} { hello(name: "world") }`
const source = `query ${operationName} { hello(name: "world") }`
const variableValues = { who: 'world' }

agent
Expand Down Expand Up @@ -446,8 +442,7 @@ describe('Plugin', () => {
withNamingSchema(
() => {
const operationName = 'MyQuery'
const resource = `query ${operationName}`
const source = `${resource} { hello(name: "world") }`
const source = `query ${operationName} { hello(name: "world") }`
const variableValues = { who: 'world' }
gateway()
.then(({ executor }) => {
Expand All @@ -464,18 +459,19 @@ describe('Plugin', () => {

describe('with configuration', () => {
before(() => {
return agent.load('apollo', { service: 'custom' })
return agent.load('apollo', { service: 'custom', source: true, signature: false })
})

it('should be configured with the correct values', done => {
const operationName = 'MyQuery'
const resource = `query ${operationName}`
const source = `${resource} { hello(name: "world") }`
const source = `query ${operationName} { hello(name: "world") }`
const variableValues = { who: 'world' }
agent
.use((traces) => {
expect(traces[0][0]).to.have.property('name', expectedSchema.server.opName)
expect(traces[0][0]).to.have.property('service', 'custom')
expect(traces[0][0]).to.have.property('resource', `query ${operationName}`)
expect(traces[0][0].meta).to.have.property('graphql.source', source)

expect(traces[0][1]).to.have.property('name', 'apollo.gateway.validate')
expect(traces[0][1]).to.have.property('service', 'custom')
Expand Down

0 comments on commit 8cc7705

Please sign in to comment.