Skip to content

Commit

Permalink
add peer service remapping capability (#3320)
Browse files Browse the repository at this point in the history
Supplying a `DD_TRACE_PEER_SERVICE_MAPPING` allows users to modify the `peer.service` tag
in the same manner that `DD_SERVICE_MAPPING` does for service name.

If the `peer.service` value was matched by the mapping, the original value of the tag before
remapping is reported in `_dd.peer.service.remapped_from`.
  • Loading branch information
jbertran authored and Stephen Belanger committed Jul 13, 2023
1 parent b860cfa commit a897122
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 13 deletions.
7 changes: 7 additions & 0 deletions packages/dd-trace/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,12 @@ class Config {
const DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = validateNamingVersion(
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA
)
const DD_TRACE_PEER_SERVICE_MAPPING = coalesce(
options.peerServiceMapping,
process.env.DD_TRACE_PEER_SERVICE_MAPPING ? fromEntries(
process.env.DD_TRACE_PEER_SERVICE_MAPPING.split(',').map(x => x.trim().split(':'))
) : {}
)
const DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED

const DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED = coalesce(
Expand Down Expand Up @@ -564,6 +570,7 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)
: true
)
this.traceRemoveIntegrationServiceNamesEnabled = DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED
this.peerServiceMapping = DD_TRACE_PEER_SERVICE_MAPPING
this.lookup = options.lookup
this.startupLogs = isTrue(DD_TRACE_STARTUP_LOGS)
// Disabled for CI Visibility's agentless
Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ module.exports = {
CLIENT_PORT_KEY: 'network.destination.port',
PEER_SERVICE_KEY: 'peer.service',
PEER_SERVICE_SOURCE_KEY: '_dd.peer.service.source',
PEER_SERVICE_REMAP_KEY: '_dd.peer.service.remapped_from',
SCI_REPOSITORY_URL: '_dd.git.repository_url',
SCI_COMMIT_SHA: '_dd.git.commit.sha'
}
1 change: 1 addition & 0 deletions packages/dd-trace/src/opentracing/tracer.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class DatadogTracer {
this._env = config.env
this._tags = config.tags
this._computePeerService = config.spanComputePeerService
this._peerServiceMapping = config.peerServiceMapping
this._logInjection = config.logInjection
this._debug = config.debug
this._prioritySampler = new PrioritySampler(config.env, config.sampler)
Expand Down
37 changes: 25 additions & 12 deletions packages/dd-trace/src/plugins/outbound.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
const {
CLIENT_PORT_KEY,
PEER_SERVICE_KEY,
PEER_SERVICE_SOURCE_KEY
PEER_SERVICE_SOURCE_KEY,
PEER_SERVICE_REMAP_KEY
} = require('../constants')
const TracingPlugin = require('./tracing')

Expand Down Expand Up @@ -34,9 +35,11 @@ class OutboundPlugin extends TracingPlugin {
* - If `peer.service` was defined _before_ we compute it (for example in custom instrumentation),
* `_dd.peer.service.source`'s value is `peer.service`
*/

if (tags['peer.service'] !== undefined) {
return { [PEER_SERVICE_SOURCE_KEY]: 'peer.service' }
if (tags[PEER_SERVICE_KEY] !== undefined) {
return {
[PEER_SERVICE_KEY]: tags[PEER_SERVICE_KEY],
[PEER_SERVICE_SOURCE_KEY]: PEER_SERVICE_KEY
}
}

const sourceTags = [
Expand All @@ -52,25 +55,35 @@ class OutboundPlugin extends TracingPlugin {
}
}
}
return {}
return undefined
}

startSpan (name, options) {
const span = super.startSpan(name, options)
return span
getPeerServiceRemap (peerData) {
/**
* If DD_TRACE_PEER_SERVICE_MAPPING is matched, we need to override the existing
* peer service and add the value we overrode.
*/
const peerService = peerData[PEER_SERVICE_KEY]
if (peerService && this.tracer._peerServiceMapping[peerService]) {
return {
...peerData,
[PEER_SERVICE_KEY]: this.tracer._peerServiceMapping[peerService],
[PEER_SERVICE_REMAP_KEY]: peerService
}
}
return peerData
}

finish () {
const span = this.activeSpan
this.tagPeerService(span)
this.tagPeerService(this.activeSpan)
super.finish(...arguments)
}

tagPeerService (span) {
if (this.tracer._computePeerService) {
const peerData = this.getPeerService(span.context()._tags)
if (peerData) {
span.addTags(peerData)
if (peerData !== undefined) {
span.addTags(this.getPeerServiceRemap(peerData))
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions packages/dd-trace/test/config.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ describe('Config', () => {
process.env.DD_TRACE_AGENT_PROTOCOL_VERSION = '0.5'
process.env.DD_SERVICE = 'service'
process.env.DD_SERVICE_MAPPING = 'a:aa, b:bb'
process.env.DD_TRACE_PEER_SERVICE_MAPPING = 'c:cc, d:dd'
process.env.DD_VERSION = '1.0.0'
process.env.DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP = '.*'
process.env.DD_TRACE_CLIENT_IP_ENABLED = 'true'
Expand Down Expand Up @@ -258,6 +259,10 @@ describe('Config', () => {
a: 'aa',
b: 'bb'
})
expect(config).to.have.deep.property('peerServiceMapping', {
c: 'cc',
d: 'dd'
})
expect(config).to.have.nested.deep.property('tracePropagationStyle.inject', ['b3', 'tracecontext'])
expect(config).to.have.nested.deep.property('tracePropagationStyle.extract', ['b3', 'tracecontext'])
expect(config).to.have.nested.property('experimental.runtimeId', true)
Expand Down Expand Up @@ -560,6 +565,7 @@ describe('Config', () => {
process.env.DD_TRACE_PARTIAL_FLUSH_MIN_SPANS = 2000
process.env.DD_SERVICE = 'service'
process.env.DD_SERVICE_MAPPING = 'a:aa'
process.env.DD_TRACE_PEER_SERVICE_MAPPING = 'c:cc'
process.env.DD_VERSION = '0.0.0'
process.env.DD_RUNTIME_METRICS_ENABLED = 'true'
process.env.DD_TRACE_REPORT_HOSTNAME = 'true'
Expand Down Expand Up @@ -614,6 +620,9 @@ describe('Config', () => {
serviceMapping: {
b: 'bb'
},
peerServiceMapping: {
d: 'dd'
},
tracePropagationStyle: {
inject: [],
extract: []
Expand Down Expand Up @@ -668,6 +677,7 @@ describe('Config', () => {
expect(config.tags).to.include({ foo: 'foo', baz: 'qux' })
expect(config.tags).to.include({ service: 'test', version: '1.0.0', env: 'development' })
expect(config).to.have.deep.property('serviceMapping', { b: 'bb' })
expect(config).to.have.deep.property('peerServiceMapping', { d: 'dd' })
expect(config).to.have.nested.deep.property('tracePropagationStyle.inject', [])
expect(config).to.have.nested.deep.property('tracePropagationStyle.extract', [])
expect(config).to.have.nested.property('experimental.runtimeId', false)
Expand Down
101 changes: 100 additions & 1 deletion packages/dd-trace/test/plugins/outbound.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,54 @@

require('../setup/tap')

const { expect } = require('chai')
const OutboundPlugin = require('../../src/plugins/outbound')

describe('OuboundPlugin', () => {
describe('peer service decision', () => {
let instance = null
let computePeerServiceStub = null
let getPeerServiceStub = null
let getRemapStub = null

beforeEach(() => {
instance = new OutboundPlugin()
computePeerServiceStub = sinon.stub(instance, 'tracer')
getPeerServiceStub = sinon.stub(instance, 'getPeerService')
getRemapStub = sinon.stub(instance, 'getPeerServiceRemap')
})

afterEach(() => {
computePeerServiceStub.restore()
getPeerServiceStub.restore()
getRemapStub.restore()
})

it('should attempt to remap when we found peer service', () => {
computePeerServiceStub.value({ _computePeerService: true })
getPeerServiceStub.returns({ foo: 'bar' })
instance.tagPeerService({ context: () => { return { _tags: {} } }, addTags: () => {} })

expect(getPeerServiceStub).to.be.called
expect(getRemapStub).to.be.called
})

it('should not attempt to remap if we found no peer service', () => {
computePeerServiceStub.value({ _computePeerService: true })
getPeerServiceStub.returns(undefined)
instance.tagPeerService({ context: () => { return { _tags: {} } }, addTags: () => {} })

expect(getPeerServiceStub).to.be.called
expect(getRemapStub).to.not.be.called
})

it('should do nothing when disabled', () => {
computePeerServiceStub.value({ _computePeerService: false })
instance.tagPeerService({ context: () => { return { _tags: {} } }, addTags: () => {} })
expect(getPeerServiceStub).to.not.be.called
expect(getRemapStub).to.not.be.called
})
})
describe('peer.service computation', () => {
let instance = null

Expand All @@ -16,7 +61,7 @@ describe('OuboundPlugin', () => {
const res = instance.getPeerService({
fooIsNotAPrecursor: 'bar'
})
expect(res).to.deep.equal({})
expect(res).to.equal(undefined)
})

it('should grab from remote host in datadog format', () => {
Expand Down Expand Up @@ -56,4 +101,58 @@ describe('OuboundPlugin', () => {
})
})
})
describe('remapping computation', () => {
let instance = null
let mappingStub = null
const peerData = {
'peer.service': 'foosvc',
'_dd.peer.service.source': 'out.host'
}

beforeEach(() => {
instance = new OutboundPlugin()
})

afterEach(() => {
mappingStub.restore()
})

it('should return peer data unchanged if there is no peer service', () => {
const mappingData = instance.getPeerServiceRemap({ 'foo': 'bar' })
mappingStub = sinon.stub(instance, 'tracer')
expect(mappingData).to.deep.equal({ 'foo': 'bar' })
})

it('should return peer data unchanged if no mapping is available', () => {
mappingStub = sinon.stub(instance, 'tracer').value({ _peerServiceMapping: {} })
const mappingData = instance.getPeerServiceRemap(peerData)
expect(mappingData).to.deep.equal(peerData)
})

it('should return peer data unchanged if no mapping item matches', () => {
mappingStub = sinon.stub(instance, 'tracer').value({
_peerServiceMapping: {
barsvc: 'bar',
bazsvc: 'baz'
}
})
const mappingData = instance.getPeerServiceRemap(peerData)
expect(mappingData).to.deep.equal(peerData)
})

it('should remap if a mapping item matches', () => {
mappingStub = sinon.stub(instance, 'tracer').value({
_peerServiceMapping: {
foosvc: 'foo',
bazsvc: 'baz'
}
})
const mappingData = instance.getPeerServiceRemap(peerData)
expect(mappingData).to.deep.equal({
'peer.service': 'foo',
'_dd.peer.service.source': 'out.host',
'_dd.peer.service.remapped_from': 'foosvc'
})
})
})
})

0 comments on commit a897122

Please sign in to comment.