Skip to content

Commit

Permalink
add support for DD_GRPC_CLIENT_ERROR_STATUSES & DD_GRPC_SERVER_ERROR_…
Browse files Browse the repository at this point in the history
…STATUSES
  • Loading branch information
khanayan123 committed Oct 17, 2024
1 parent 8a874a0 commit f5992f5
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 3 deletions.
4 changes: 3 additions & 1 deletion packages/datadog-plugin-grpc/src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ class GrpcClientPlugin extends ClientPlugin {

error ({ span, error }) {
this.addCode(span, error.code)
this.addError(error, span)
if (error.code && error.code in this._tracerConfig.grpc.client.error.statuses) {
this.addError(error, span)
}
}

finish ({ span, result, peer }) {
Expand Down
4 changes: 3 additions & 1 deletion packages/datadog-plugin-grpc/src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ class GrpcServerPlugin extends ServerPlugin {
if (!span) return

this.addCode(span, error.code)
this.addError(error)
if (error.code && error.code in this._tracerConfig.grpc.server.error.statuses) {
this.addError(error)
}
}

finish ({ span, code, trailer }) {
Expand Down
17 changes: 17 additions & 0 deletions packages/datadog-plugin-grpc/test/client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,23 @@ describe('Plugin', () => {
})
})

it('should ignore errors not set by DD_GRPC_CLIENT_ERROR_STATUSES', async () => {
process.env.DD_GRPC_CLIENT_ERROR_STATUSES = '3-13'
tracer = require('../../dd-trace')
const client = await buildClient({
getUnary: (_, callback) => callback(new Error('foobar'))
})

client.getUnary({ first: 'foobar' }, () => {})

return agent
.use(traces => {
expect(traces[0][0]).to.have.property('error', 0)
expect(traces[0][0].metrics).to.have.property('grpc.status.code', 2)
delete process.env.DD_GRPC_CLIENT_ERROR_STATUSES
})
})

it('should handle protocol errors', async () => {
const definition = loader.loadSync(path.join(__dirname, 'invalid.proto'))
const test = grpc.loadPackageDefinition(definition).test
Expand Down
33 changes: 33 additions & 0 deletions packages/datadog-plugin-grpc/test/server.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,39 @@ describe('Plugin', () => {
})
})

it('should ignore errors not set by DD_GRPC_SERVER_ERROR_STATUSES', async () => {
process.env.DD_GRPC_SERVER_ERROR_STATUSES = '6-13'
tracer = require('../../dd-trace')
const client = await buildClient({
getUnary: (_, callback) => {
const metadata = new grpc.Metadata()

metadata.set('extra', 'information')

const error = new Error('foobar')

error.code = grpc.status.NOT_FOUND

const childOf = tracer.scope().active()
const child = tracer.startSpan('child', { childOf })

// Delay trace to ensure auto-cancellation doesn't override the status code.
setTimeout(() => child.finish())

callback(error, {}, metadata)
}
})

client.getUnary({ first: 'foobar' }, () => {})

return agent
.use(traces => {
expect(traces[0][0]).to.have.property('error', 0)
expect(traces[0][0].metrics).to.have.property('grpc.status.code', 5)
delete process.env.DD_GRPC_SERVER_ERROR_STATUSES
})
})

it('should handle custom errors', async () => {
const client = await buildClient({
getUnary: (_, callback) => {
Expand Down
26 changes: 26 additions & 0 deletions packages/dd-trace/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,8 @@ class Config {
this._setValue(defaults, 'flushInterval', 2000)
this._setValue(defaults, 'flushMinSpans', 1000)
this._setValue(defaults, 'gitMetadataEnabled', true)
this._setValue(defaults, 'grpc.client.error.statuses', [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
this._setValue(defaults, 'grpc.server.error.statuses', [2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
this._setValue(defaults, 'headerTags', [])
this._setValue(defaults, 'hostname', '127.0.0.1')
this._setValue(defaults, 'iast.cookieFilterPattern', '.{32,}')
Expand Down Expand Up @@ -584,6 +586,8 @@ class Config {
DD_EXPERIMENTAL_API_SECURITY_ENABLED,
DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED,
DD_EXPERIMENTAL_PROFILING_ENABLED,
DD_GRPC_CLIENT_ERROR_STATUSES,
DD_GRPC_SERVER_ERROR_STATUSES,
JEST_WORKER_ID,
DD_IAST_COOKIE_FILTER_PATTERN,
DD_IAST_DEDUPLICATION_ENABLED,
Expand Down Expand Up @@ -720,6 +724,8 @@ class Config {
this._setValue(env, 'flushMinSpans', maybeInt(DD_TRACE_PARTIAL_FLUSH_MIN_SPANS))
this._envUnprocessed.flushMinSpans = DD_TRACE_PARTIAL_FLUSH_MIN_SPANS
this._setBoolean(env, 'gitMetadataEnabled', DD_TRACE_GIT_METADATA_ENABLED)
this._setIntegerRangeSet(env, 'grpc.client.error.statuses', DD_GRPC_CLIENT_ERROR_STATUSES)
this._setIntegerRangeSet(env, 'grpc.server.error.statuses', DD_GRPC_SERVER_ERROR_STATUSES)
this._setArray(env, 'headerTags', DD_TRACE_HEADER_TAGS)
this._setString(env, 'hostname', coalesce(DD_AGENT_HOST, DD_TRACE_AGENT_HOSTNAME))
this._setString(env, 'iast.cookieFilterPattern', DD_IAST_COOKIE_FILTER_PATTERN)
Expand Down Expand Up @@ -1163,6 +1169,26 @@ class Config {
}
}

_setIntegerRangeSet (obj, name, value) {
if (value == null) {
return this._setValue(obj, name, null)
}
value = value.split(',')
const result = []

value.forEach(val => {
if (val.includes('-')) {
const [start, end] = val.split('-').map(Number)
for (let i = start; i <= end; i++) {
result.push(i)
}
} else {
result.push(Number(val))
}
})
this._setValue(obj, name, result)
}

_setSamplingRule (obj, name, value) {
if (value == null) {
return this._setValue(obj, name, null)
Expand Down
4 changes: 3 additions & 1 deletion packages/dd-trace/src/telemetry/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,9 @@ function updateConfig (changes, config) {
logInjection: 'DD_LOG_INJECTION',
headerTags: 'DD_TRACE_HEADER_TAGS',
tags: 'DD_TAGS',
'sampler.rules': 'DD_TRACE_SAMPLING_RULES'
'sampler.rules': 'DD_TRACE_SAMPLING_RULES',
'grpc.client.error.statuses': 'DD_GRPC_CLIENT_ERROR_STATUSES',
'grpc.server.error.statuses': 'DD_GRPC_SERVER_ERROR_STATUSES'
}

const namesNeedFormatting = new Set(['DD_TAGS', 'peerServiceMapping', 'serviceMapping'])
Expand Down
32 changes: 32 additions & 0 deletions packages/dd-trace/test/config.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ describe('Config', () => {
expect(config).to.have.property('traceId128BitGenerationEnabled', true)
expect(config).to.have.property('traceId128BitLoggingEnabled', false)
expect(config).to.have.property('spanAttributeSchema', 'v0')
expect(config.grpc.client.error.statuses).to.deep.equal([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
expect(config.grpc.server.error.statuses).to.deep.equal([2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
expect(config).to.have.property('spanComputePeerService', false)
expect(config).to.have.property('spanRemoveIntegrationFromService', false)
expect(config).to.have.property('instrumentation_config_id', undefined)
Expand Down Expand Up @@ -495,6 +497,8 @@ describe('Config', () => {
process.env.DD_INSTRUMENTATION_INSTALL_TYPE = 'k8s_single_step'
process.env.DD_INSTRUMENTATION_INSTALL_TIME = '1703188212'
process.env.DD_INSTRUMENTATION_CONFIG_ID = 'abcdef123'
process.env.DD_GRPC_CLIENT_ERROR_STATUSES = '3,13,400-403'
process.env.DD_GRPC_SERVER_ERROR_STATUSES = '3,13,400-403'

// required if we want to check updates to config.debug and config.logLevel which is fetched from logger
reloadLoggerAndConfig()
Expand All @@ -512,6 +516,8 @@ describe('Config', () => {
expect(config).to.have.property('queryStringObfuscation', '.*')
expect(config).to.have.property('clientIpEnabled', true)
expect(config).to.have.property('clientIpHeader', 'x-true-client-ip')
expect(config.grpc.client.error.statuses).to.deep.equal([3, 13, 400, 401, 402, 403])
expect(config.grpc.server.error.statuses).to.deep.equal([3, 13, 400, 401, 402, 403])
expect(config).to.have.property('runtimeMetrics', true)
expect(config).to.have.property('reportHostname', true)
expect(config).to.have.nested.property('codeOriginForSpans.enabled', true)
Expand Down Expand Up @@ -997,6 +1003,32 @@ describe('Config', () => {
expect(config).to.have.property('spanAttributeSchema', 'v0')
})

it('should parse integer range sets', () => {
process.env.DD_GRPC_CLIENT_ERROR_STATUSES = '3,13,400-403'
process.env.DD_GRPC_SERVER_ERROR_STATUSES = '3,13,400-403'

let config = new Config()

expect(config.grpc.client.error.statuses).to.deep.equal([3, 13, 400, 401, 402, 403])
expect(config.grpc.server.error.statuses).to.deep.equal([3, 13, 400, 401, 402, 403])

process.env.DD_GRPC_CLIENT_ERROR_STATUSES = '1'
process.env.DD_GRPC_SERVER_ERROR_STATUSES = '1'

config = new Config()

expect(config.grpc.client.error.statuses).to.deep.equal([1])
expect(config.grpc.server.error.statuses).to.deep.equal([1])

process.env.DD_GRPC_CLIENT_ERROR_STATUSES = '2,10,13-15'
process.env.DD_GRPC_SERVER_ERROR_STATUSES = '2,10,13-15'

config = new Config()

expect(config.grpc.client.error.statuses).to.deep.equal([2, 10, 13, 14, 15])
expect(config.grpc.server.error.statuses).to.deep.equal([2, 10, 13, 14, 15])
})

context('peer service tagging', () => {
it('should activate peer service only if explicitly true in v0', () => {
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = 'v0'
Expand Down

0 comments on commit f5992f5

Please sign in to comment.