Skip to content

Commit

Permalink
Include the extra services found in spans in the remote config client…
Browse files Browse the repository at this point in the history
… request payloads [APPSEC-9476] (#3635)

* Register and send extra services found in spans
  • Loading branch information
iunanua authored Sep 28, 2023
1 parent c56d758 commit 5af860c
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 5 deletions.
14 changes: 11 additions & 3 deletions packages/dd-trace/src/appsec/remote_config/manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
const { URL, format } = require('url')
const uuid = require('crypto-randomuuid')
const { EventEmitter } = require('events')
const Scheduler = require('./scheduler')
const tracerVersion = require('../../../../../package.json').version
const request = require('../../exporters/common/request')
const log = require('../../log')
const { getExtraServices } = require('../../service-naming/extra-services')
const { UNACKNOWLEDGED, ACKNOWLEDGED, ERROR } = require('./apply_states')
const Scheduler = require('./scheduler')

const clientId = uuid()

Expand Down Expand Up @@ -57,7 +58,8 @@ class RemoteConfigManager extends EventEmitter {
tracer_version: tracerVersion,
service: config.service,
env: config.env,
app_version: config.version
app_version: config.version,
extra_services: []
},
capabilities: DEFAULT_CAPABILITY // updated by `updateCapabilities()`
},
Expand Down Expand Up @@ -113,8 +115,14 @@ class RemoteConfigManager extends EventEmitter {
this.state.client.products = this.eventNames().filter(e => typeof e === 'string')
}

getPayload () {
this.state.client.client_tracer.extra_services = getExtraServices()

return JSON.stringify(this.state)
}

poll (cb) {
request(JSON.stringify(this.state), this.requestOptions, (err, data, statusCode) => {
request(this.getPayload(), this.requestOptions, (err, data, statusCode) => {
// 404 means RC is disabled, ignore it
if (statusCode === 404) return cb()

Expand Down
3 changes: 3 additions & 0 deletions packages/dd-trace/src/format.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const constants = require('./constants')
const tags = require('../../../ext/tags')
const id = require('./id')
const { isError } = require('./util')
const { registerExtraService } = require('./service-naming/extra-services')

const SAMPLING_PRIORITY_KEY = constants.SAMPLING_PRIORITY_KEY
const SAMPLING_RULE_DECISION = constants.SAMPLING_RULE_DECISION
Expand Down Expand Up @@ -76,6 +77,8 @@ function extractTags (trace, span) {
const tracerService = span.tracer()._service.toLowerCase()
if (tags['service.name']?.toLowerCase() !== tracerService) {
span.setTag(BASE_SERVICE, tracerService)

registerExtraService(tags['service.name'])
}

for (const tag in tags) {
Expand Down
24 changes: 24 additions & 0 deletions packages/dd-trace/src/service-naming/extra-services.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict'

const maxExtraServices = 64
const extraServices = new Set()

function getExtraServices () {
return [...extraServices]
}

function registerExtraService (serviceName) {
if (serviceName && extraServices.size < maxExtraServices) {
extraServices.add(serviceName)
}
}

function clear () {
extraServices.clear()
}

module.exports = {
registerExtraService,
getExtraServices,
clear
}
26 changes: 24 additions & 2 deletions packages/dd-trace/test/appsec/remote_config/manager.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ describe('RemoteConfigManager', () => {
let Scheduler
let request
let log
let extraServices
let RemoteConfigManager
let config
let rc
Expand All @@ -31,12 +32,17 @@ describe('RemoteConfigManager', () => {
error: sinon.spy()
}

extraServices = []

RemoteConfigManager = proxyquire('../src/appsec/remote_config/manager', {
'crypto-randomuuid': uuid,
'./scheduler': Scheduler,
'../../../../../package.json': { version: '3.0.0' },
'../../exporters/common/request': request,
'../../log': log
'../../log': log,
'../../service-naming/extra-services': {
getExtraServices: () => extraServices
}
})

config = {
Expand Down Expand Up @@ -95,7 +101,8 @@ describe('RemoteConfigManager', () => {
tracer_version: '3.0.0',
service: config.service,
env: config.env,
app_version: config.version
app_version: config.version,
extra_services: []
},
capabilities: 'AA=='
},
Expand Down Expand Up @@ -261,6 +268,21 @@ describe('RemoteConfigManager', () => {
cb()
})
})

it('should include extra_services in the payload', (cb) => {
request.yieldsRight(null, '{}', 200)

extraServices = ['test-service']

// getPayload includes the new extraServices that might be available
const payload = rc.getPayload()
expect(JSON.parse(payload).client.client_tracer.extra_services).to.deep.equal(extraServices)

rc.poll(() => {
expect(request).to.have.been.calledOnceWith(payload, rc.requestOptions)
cb()
})
})
})

describe('parseConfig', () => {
Expand Down
9 changes: 9 additions & 0 deletions packages/dd-trace/test/format.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ require('./setup/tap')
const constants = require('../src/constants')
const tags = require('../../../ext/tags')
const id = require('../src/id')
const { getExtraServices } = require('../src/service-naming/extra-services')

const SAMPLING_PRIORITY_KEY = constants.SAMPLING_PRIORITY_KEY
const MEASURED = tags.MEASURED
Expand Down Expand Up @@ -104,6 +105,14 @@ describe('format', () => {

expect(span.setTag).to.not.have.been.called
})

it('should register extra service name', () => {
span.context()._tags['service.name'] = 'foo'

trace = format(span)

expect(getExtraServices()).to.deep.equal(['foo'])
})
})

it('should extract Datadog specific tags', () => {
Expand Down
44 changes: 44 additions & 0 deletions packages/dd-trace/test/service-naming/extra-services.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict'

require('../setup/tap')

const { expect } = require('chai')
const { registerExtraService, getExtraServices, clear } = require('../../src/service-naming/extra-services')

describe('Extra services', () => {
beforeEach(clear)

describe('registerExtraService', () => {
it('should register defined service names', () => {
registerExtraService('service-test')

expect(getExtraServices()).to.deep.equal(['service-test'])
})

it('should not register invalid service names', () => {
registerExtraService()
registerExtraService(null)
registerExtraService('')

expect(getExtraServices().length).to.equal(0)
})

it('should register the same service name only once', () => {
registerExtraService('service-test')
registerExtraService('service-test')
registerExtraService('service-test')

const extraServices = getExtraServices()
expect(extraServices.length).to.equal(1)
expect(extraServices).to.deep.equal(['service-test'])
})

it('should register a max of 64 service names', () => {
for (let i = 0; i < 100; i++) {
registerExtraService(`service-test-${i}`)
}

expect(getExtraServices().length).to.equal(64)
})
})
})

0 comments on commit 5af860c

Please sign in to comment.