Skip to content

Commit 4836de2

Browse files
jbertrantlhunter
authored andcommitted
Fix service representation config (#3419)
* address and fix edge cases * move to better tracer option name for service name flattening
1 parent f32a349 commit 4836de2

4 files changed

Lines changed: 91 additions & 14 deletions

File tree

packages/dd-trace/src/config.js

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -321,19 +321,38 @@ class Config {
321321
false
322322
)
323323
const DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = validateNamingVersion(
324-
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA
324+
coalesce(
325+
options.spanAttributeSchema,
326+
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA
327+
)
325328
)
326329
const DD_TRACE_PEER_SERVICE_MAPPING = coalesce(
327330
options.peerServiceMapping,
328331
process.env.DD_TRACE_PEER_SERVICE_MAPPING ? fromEntries(
329332
process.env.DD_TRACE_PEER_SERVICE_MAPPING.split(',').map(x => x.trim().split(':'))
330333
) : {}
331334
)
332-
const DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED
335+
336+
const peerServiceSet = (
337+
options.hasOwnProperty('spanComputePeerService') ||
338+
process.env.hasOwnProperty('DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED')
339+
)
340+
const peerServiceValue = coalesce(
341+
options.spanComputePeerService,
342+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED
343+
)
344+
345+
const DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = (
346+
DD_TRACE_SPAN_ATTRIBUTE_SCHEMA === 'v0'
347+
// In v0, peer service is computed only if it is explicitly set to true
348+
? peerServiceSet && isTrue(peerServiceValue)
349+
// In >v0, peer service is false only if it is explicitly set to false
350+
: (peerServiceSet ? !isFalse(peerServiceValue) : true)
351+
)
333352

334353
const DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED = coalesce(
335-
isTrue(process.env.DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED),
336-
false
354+
options.spanRemoveIntegrationFromService,
355+
isTrue(process.env.DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED)
337356
)
338357
const DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH = coalesce(
339358
process.env.DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH,
@@ -565,11 +584,8 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)
565584
exporters: DD_PROFILING_EXPORTERS
566585
}
567586
this.spanAttributeSchema = DD_TRACE_SPAN_ATTRIBUTE_SCHEMA
568-
this.spanComputePeerService = (this.spanAttributeSchema === 'v0'
569-
? isTrue(DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED)
570-
: true
571-
)
572-
this.traceRemoveIntegrationServiceNamesEnabled = DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED
587+
this.spanComputePeerService = DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED
588+
this.spanRemoveIntegrationFromService = DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED
573589
this.peerServiceMapping = DD_TRACE_PEER_SERVICE_MAPPING
574590
this.lookup = options.lookup
575591
this.startupLogs = isTrue(DD_TRACE_STARTUP_LOGS)

packages/dd-trace/src/service-naming/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const { schemaDefinitions } = require('./schemas')
33
class SchemaManager {
44
constructor () {
55
this.schemas = schemaDefinitions
6-
this.config = { spanAttributeSchema: 'v0', traceRemoveIntegrationServiceNamesEnabled: false }
6+
this.config = { spanAttributeSchema: 'v0', spanRemoveIntegrationFromService: false }
77
}
88

99
get schema () {
@@ -15,7 +15,7 @@ class SchemaManager {
1515
}
1616

1717
get shouldUseConsistentServiceNaming () {
18-
return this.config.traceRemoveIntegrationServiceNamesEnabled && this.version === 'v0'
18+
return this.config.spanRemoveIntegrationFromService && this.version === 'v0'
1919
}
2020

2121
opName (type, kind, plugin, ...opNameArgs) {

packages/dd-trace/test/config.spec.js

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe('Config', () => {
9393
expect(config).to.have.property('traceId128BitLoggingEnabled', false)
9494
expect(config).to.have.property('spanAttributeSchema', 'v0')
9595
expect(config).to.have.property('spanComputePeerService', false)
96-
expect(config).to.have.property('traceRemoveIntegrationServiceNamesEnabled', false)
96+
expect(config).to.have.property('spanRemoveIntegrationFromService', false)
9797
expect(config).to.have.deep.property('serviceMapping', {})
9898
expect(config).to.have.nested.deep.property('tracePropagationStyle.inject', ['datadog', 'tracecontext'])
9999
expect(config).to.have.nested.deep.property('tracePropagationStyle.extract', ['datadog', 'tracecontext'])
@@ -195,6 +195,8 @@ describe('Config', () => {
195195
process.env.DD_TRACE_EXPERIMENTAL_GET_RUM_DATA_ENABLED = 'true'
196196
process.env.DD_TRACE_EXPERIMENTAL_INTERNAL_ERRORS_ENABLED = 'true'
197197
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = 'v1'
198+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'true'
199+
process.env.DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED = 'true'
198200
process.env.DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED = true
199201
process.env.DD_APPSEC_ENABLED = 'true'
200202
process.env.DD_APPSEC_RULES = RULES_JSON_PATH
@@ -237,6 +239,8 @@ describe('Config', () => {
237239
expect(config).to.have.property('traceId128BitGenerationEnabled', true)
238240
expect(config).to.have.property('traceId128BitLoggingEnabled', true)
239241
expect(config).to.have.property('spanAttributeSchema', 'v1')
242+
expect(config).to.have.property('spanRemoveIntegrationFromService', true)
243+
expect(config).to.have.property('spanComputePeerService', true)
240244
expect(config.tags).to.include({ foo: 'bar', baz: 'qux' })
241245
expect(config.tags).to.include({ service: 'service', 'version': '1.0.0', 'env': 'test' })
242246
expect(config).to.have.deep.nested.property('sampler', {
@@ -372,6 +376,12 @@ describe('Config', () => {
372376
{ service: 'mysql', sampleRate: 1.0 },
373377
{ sampleRate: 0.1 }
374378
],
379+
spanAttributeSchema: 'v1',
380+
spanComputePeerService: true,
381+
spanRemoveIntegrationFromService: true,
382+
peerServiceMapping: {
383+
d: 'dd'
384+
},
375385
serviceMapping: {
376386
a: 'aa',
377387
b: 'bb'
@@ -437,6 +447,9 @@ describe('Config', () => {
437447
expect(config).to.have.property('logLevel', logLevel)
438448
expect(config).to.have.property('traceId128BitGenerationEnabled', true)
439449
expect(config).to.have.property('traceId128BitLoggingEnabled', true)
450+
expect(config).to.have.property('spanRemoveIntegrationFromService', true)
451+
expect(config).to.have.property('spanComputePeerService', true)
452+
expect(config).to.have.deep.property('peerServiceMapping', { d: 'dd' })
440453
expect(config).to.have.property('tags')
441454
expect(config.tags).to.have.property('foo', 'bar')
442455
expect(config.tags).to.have.property('runtime-id')
@@ -536,13 +549,52 @@ describe('Config', () => {
536549
it('should warn if defaulting to v0 span attribute schema', () => {
537550
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = 'foo'
538551

539-
// eslint-disable-next-line no-new
540552
const config = new Config()
541553

542554
expect(log.warn).to.have.been.calledWith('Unexpected input for config.spanAttributeSchema, picked default v0')
543555
expect(config).to.have.property('spanAttributeSchema', 'v0')
544556
})
545557

558+
context('peer service tagging', () => {
559+
it('should activate peer service only if explicitly true in v0', () => {
560+
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = 'v0'
561+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'true'
562+
let config = new Config()
563+
expect(config).to.have.property('spanComputePeerService', true)
564+
565+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'foo'
566+
config = new Config()
567+
expect(config).to.have.property('spanComputePeerService', false)
568+
569+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'false'
570+
config = new Config()
571+
expect(config).to.have.property('spanComputePeerService', false)
572+
573+
delete process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED
574+
config = new Config()
575+
expect(config).to.have.property('spanComputePeerService', false)
576+
})
577+
578+
it('should activate peer service in v1 unless explicitly false', () => {
579+
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = 'v1'
580+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'false'
581+
let config = new Config()
582+
expect(config).to.have.property('spanComputePeerService', false)
583+
584+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'foo'
585+
config = new Config()
586+
expect(config).to.have.property('spanComputePeerService', true)
587+
588+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'true'
589+
config = new Config()
590+
expect(config).to.have.property('spanComputePeerService', true)
591+
592+
delete process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED
593+
config = new Config()
594+
expect(config).to.have.property('spanComputePeerService', true)
595+
})
596+
})
597+
546598
it('should give priority to the common agent environment variable', () => {
547599
process.env.DD_TRACE_AGENT_HOSTNAME = 'trace-agent'
548600
process.env.DD_AGENT_HOST = 'agent'
@@ -572,6 +624,9 @@ describe('Config', () => {
572624
process.env.DD_ENV = 'test'
573625
process.env.DD_API_KEY = '123'
574626
process.env.DD_APP_KEY = '456'
627+
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = 'v0'
628+
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'false'
629+
process.env.DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED = 'false'
575630
process.env.DD_TRACE_CLIENT_IP_ENABLED = 'false'
576631
process.env.DD_TRACE_CLIENT_IP_HEADER = 'foo-bar-header'
577632
process.env.DD_TRACE_GLOBAL_TAGS = 'foo:bar,baz:qux'
@@ -620,6 +675,9 @@ describe('Config', () => {
620675
serviceMapping: {
621676
b: 'bb'
622677
},
678+
spanAttributeSchema: 'v1',
679+
spanComputePeerService: true,
680+
spanRemoveIntegrationFromService: true,
623681
peerServiceMapping: {
624682
d: 'dd'
625683
},
@@ -677,6 +735,9 @@ describe('Config', () => {
677735
expect(config.tags).to.include({ foo: 'foo', baz: 'qux' })
678736
expect(config.tags).to.include({ service: 'test', version: '1.0.0', env: 'development' })
679737
expect(config).to.have.deep.property('serviceMapping', { b: 'bb' })
738+
expect(config).to.have.property('spanAttributeSchema', 'v1')
739+
expect(config).to.have.property('spanRemoveIntegrationFromService', true)
740+
expect(config).to.have.property('spanComputePeerService', true)
680741
expect(config).to.have.deep.property('peerServiceMapping', { d: 'dd' })
681742
expect(config).to.have.nested.deep.property('tracePropagationStyle.inject', [])
682743
expect(config).to.have.nested.deep.property('tracePropagationStyle.extract', [])

packages/dd-trace/test/setup/mocha.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ function withNamingSchema (spanProducerFn, expectedOpName, expectedServiceName,
8686
Nomenclature.configure({
8787
spanAttributeSchema: 'v0',
8888
service: fullConfig.service,
89-
traceRemoveIntegrationServiceNamesEnabled: true
89+
spanRemoveIntegrationFromService: true
9090
})
9191
})
9292
after(() => {

0 commit comments

Comments
 (0)