Skip to content

Commit 7e78aaa

Browse files
watsonwatson
authored
[DI] Support custom upload interval (#5861)
Add a new config option to control the upload interval used by Dynamic Instrumentation (DI). By default, data collected by DI is buffered in memory, and only sent to the agent once every second (if there is data in the buffer). The new config option, allows the timeout to be controlled by the following environment variable: DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS There is also a code config alias: { dynamicInstrumentation: { uploadIntervalSeconds: 1 } } The value of this config option is parsed as a float. The main use-case for this config option is to make tests run faster, as they don't have to wait for the one second timeout. It's not expected that end-users would need to modify the default value.
1 parent 388c916 commit 7e78aaa

File tree

10 files changed

+119
-59
lines changed

10 files changed

+119
-59
lines changed

‎integration-tests/debugger/re-evaluation.spec.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ describe('Dynamic Instrumentation Probe Re-Evaluation', function () {
7373
it(testName, function (done) {
7474
this.timeout(5000)
7575

76+
let doneCalled = false
7677
const probeId = rcConfig.config.id
7778
const expectedPayloads = [{
7879
ddsource: 'dd_debugger',
@@ -105,7 +106,10 @@ describe('Dynamic Instrumentation Probe Re-Evaluation', function () {
105106
}
106107
}))
107108

108-
if (expectedPayloads.length === 0) done()
109+
if (expectedPayloads.length === 0 && doneCalled === false) {
110+
doneCalled = true
111+
done()
112+
}
109113
})
110114

111115
agent.addRemoteConfig(rcConfig)
@@ -115,6 +119,7 @@ describe('Dynamic Instrumentation Probe Re-Evaluation', function () {
115119
env: {
116120
NODE_OPTIONS: '--import dd-trace/initialize.mjs',
117121
DD_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
122+
DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS: '0',
118123
DD_TRACE_AGENT_PORT: agent.port,
119124
DD_TRACE_DEBUG: process.env.DD_TRACE_DEBUG, // inherit to make debugging the sandbox easier
120125
DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS: 0.1

‎integration-tests/debugger/utils.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ function setup ({ env, testApp, testAppSource, dependencies } = {}) {
9696
cwd,
9797
env: {
9898
DD_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
99+
DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS: '0',
99100
DD_TRACE_AGENT_PORT: t.agent.port,
100101
DD_TRACE_DEBUG: process.env.DD_TRACE_DEBUG, // inherit to make debugging the sandbox easier
101102
DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS: pollInterval,

‎packages/dd-trace/src/config.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,7 @@ class Config {
487487
this._setValue(defaults, 'dynamicInstrumentation.enabled', false)
488488
this._setValue(defaults, 'dynamicInstrumentation.redactedIdentifiers', [])
489489
this._setValue(defaults, 'dynamicInstrumentation.redactionExcludedIdentifiers', [])
490+
this._setValue(defaults, 'dynamicInstrumentation.uploadIntervalSeconds', 1)
490491
this._setValue(defaults, 'env')
491492
this._setValue(defaults, 'experimental.enableGetRumData', false)
492493
this._setValue(defaults, 'experimental.exporter')
@@ -670,6 +671,7 @@ class Config {
670671
DD_DYNAMIC_INSTRUMENTATION_ENABLED,
671672
DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS,
672673
DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS,
674+
DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS,
673675
DD_ENV,
674676
DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED,
675677
DD_PROFILING_ENABLED,
@@ -843,6 +845,12 @@ class Config {
843845
'dynamicInstrumentation.redactionExcludedIdentifiers',
844846
DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS
845847
)
848+
this._setValue(
849+
env,
850+
'dynamicInstrumentation.uploadIntervalSeconds',
851+
maybeFloat(DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS)
852+
)
853+
this._envUnprocessed['dynamicInstrumentation.uploadInterval'] = DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS
846854
this._setString(env, 'env', DD_ENV || tags.env)
847855
this._setBoolean(env, 'traceEnabled', DD_TRACE_ENABLED)
848856
this._setBoolean(env, 'experimental.enableGetRumData', DD_TRACE_EXPERIMENTAL_GET_RUM_DATA_ENABLED)
@@ -1072,6 +1080,13 @@ class Config {
10721080
'dynamicInstrumentation.redactionExcludedIdentifiers',
10731081
options.dynamicInstrumentation?.redactionExcludedIdentifiers
10741082
)
1083+
this._setValue(
1084+
opts,
1085+
'dynamicInstrumentation.uploadIntervalSeconds',
1086+
maybeFloat(options.dynamicInstrumentation?.uploadIntervalSeconds)
1087+
)
1088+
this._optsUnprocessed['dynamicInstrumentation.uploadIntervalSeconds'] =
1089+
options.dynamicInstrumentation?.uploadIntervalSeconds
10751090
this._setString(opts, 'env', options.env || tags.env)
10761091
this._setBoolean(opts, 'experimental.enableGetRumData', options.experimental?.enableGetRumData)
10771092
this._setString(opts, 'experimental.exporter', options.experimental?.exporter)

‎packages/dd-trace/src/debugger/devtools_client/send.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ const ddtags = [
3232

3333
const path = `/debugger/v1/input?${stringify({ ddtags })}`
3434

35-
const jsonBuffer = new JSONBuffer({ size: config.maxTotalPayloadSize, timeout: 1000, onFlush })
35+
const jsonBuffer = new JSONBuffer({
36+
size: config.maxTotalPayloadSize,
37+
timeout: config.dynamicInstrumentation.uploadIntervalSeconds * 1000,
38+
onFlush
39+
})
3640

3741
function send (message, logger, dd, snapshot) {
3842
const payload = {

‎packages/dd-trace/src/debugger/devtools_client/status.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ const runtimeId = config.runtimeId
2020

2121
const cache = new TTLSet(60 * 60 * 1000) // 1 hour
2222

23-
const jsonBuffer = new JSONBuffer({ size: config.maxTotalPayloadSize, timeout: 1000, onFlush })
23+
const jsonBuffer = new JSONBuffer({
24+
size: config.maxTotalPayloadSize,
25+
timeout: config.dynamicInstrumentation.uploadIntervalSeconds * 1000,
26+
onFlush
27+
})
2428

2529
const STATUSES = {
2630
RECEIVED: 'RECEIVED',

‎packages/dd-trace/src/supported-configurations.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
"DD_DYNAMIC_INSTRUMENTATION_ENABLED": ["A"],
5757
"DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS": ["A"],
5858
"DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS": ["A"],
59+
"DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS": ["A"],
5960
"DD_ENV": ["A"],
6061
"DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED": ["A"],
6162
"DD_EXTERNAL_ENV": ["A"],

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ describe('Config', () => {
273273
expect(config).to.have.nested.property('dynamicInstrumentation.enabled', false)
274274
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactedIdentifiers', [])
275275
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactionExcludedIdentifiers', [])
276+
expect(config).to.have.nested.property('dynamicInstrumentation.uploadIntervalSeconds', 1)
276277
expect(config).to.have.property('traceId128BitGenerationEnabled', true)
277278
expect(config).to.have.property('traceId128BitLoggingEnabled', true)
278279
expect(config).to.have.property('spanAttributeSchema', 'v0')
@@ -368,6 +369,7 @@ describe('Config', () => {
368369
{ name: 'dynamicInstrumentation.enabled', value: false, origin: 'default' },
369370
{ name: 'dynamicInstrumentation.redactedIdentifiers', value: [], origin: 'default' },
370371
{ name: 'dynamicInstrumentation.redactionExcludedIdentifiers', value: [], origin: 'default' },
372+
{ name: 'dynamicInstrumentation.uploadIntervalSeconds', value: 1, origin: 'default' },
371373
{ name: 'env', value: undefined, origin: 'default' },
372374
{ name: 'experimental.enableGetRumData', value: false, origin: 'default' },
373375
{ name: 'experimental.exporter', value: undefined, origin: 'default' },
@@ -518,6 +520,7 @@ describe('Config', () => {
518520
process.env.DD_DYNAMIC_INSTRUMENTATION_ENABLED = 'true'
519521
process.env.DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS = 'foo,bar'
520522
process.env.DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS = 'a,b,c'
523+
process.env.DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS = '0.1'
521524
process.env.DD_TRACE_GLOBAL_TAGS = 'foo:bar,baz:qux'
522525
process.env.DD_TRACE_SAMPLE_RATE = '0.5'
523526
process.env.DD_TRACE_RATE_LIMIT = '-1'
@@ -625,6 +628,7 @@ describe('Config', () => {
625628
expect(config).to.have.nested.property('dynamicInstrumentation.enabled', true)
626629
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactedIdentifiers', ['foo', 'bar'])
627630
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactionExcludedIdentifiers', ['a', 'b', 'c'])
631+
expect(config).to.have.nested.property('dynamicInstrumentation.uploadIntervalSeconds', 0.1)
628632
expect(config).to.have.property('env', 'test')
629633
expect(config).to.have.property('sampleRate', 0.5)
630634
expect(config).to.have.property('traceEnabled', true)
@@ -743,6 +747,7 @@ describe('Config', () => {
743747
{ name: 'dynamicInstrumentation.enabled', value: true, origin: 'env_var' },
744748
{ name: 'dynamicInstrumentation.redactedIdentifiers', value: ['foo', 'bar'], origin: 'env_var' },
745749
{ name: 'dynamicInstrumentation.redactionExcludedIdentifiers', value: ['a', 'b', 'c'], origin: 'env_var' },
750+
{ name: 'dynamicInstrumentation.uploadIntervalSeconds', value: 0.1, origin: 'env_var' },
746751
{ name: 'env', value: 'test', origin: 'env_var' },
747752
{ name: 'experimental.enableGetRumData', value: true, origin: 'env_var' },
748753
{ name: 'experimental.exporter', value: 'log', origin: 'env_var' },
@@ -997,7 +1002,8 @@ describe('Config', () => {
9971002
dynamicInstrumentation: {
9981003
enabled: true,
9991004
redactedIdentifiers: ['foo', 'bar'],
1000-
redactionExcludedIdentifiers: ['a', 'b', 'c']
1005+
redactionExcludedIdentifiers: ['a', 'b', 'c'],
1006+
uploadIntervalSeconds: 0.1
10011007
},
10021008
experimental: {
10031009
b3: true,
@@ -1046,6 +1052,7 @@ describe('Config', () => {
10461052
expect(config).to.have.nested.property('dynamicInstrumentation.enabled', true)
10471053
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactedIdentifiers', ['foo', 'bar'])
10481054
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactionExcludedIdentifiers', ['a', 'b', 'c'])
1055+
expect(config).to.have.nested.property('dynamicInstrumentation.uploadIntervalSeconds', 0.1)
10491056
expect(config).to.have.property('env', 'test')
10501057
expect(config).to.have.property('sampleRate', 0.5)
10511058
expect(config).to.have.property('logger', logger)
@@ -1129,6 +1136,7 @@ describe('Config', () => {
11291136
{ name: 'dynamicInstrumentation.enabled', value: true, origin: 'code' },
11301137
{ name: 'dynamicInstrumentation.redactedIdentifiers', value: ['foo', 'bar'], origin: 'code' },
11311138
{ name: 'dynamicInstrumentation.redactionExcludedIdentifiers', value: ['a', 'b', 'c'], origin: 'code' },
1139+
{ name: 'dynamicInstrumentation.uploadIntervalSeconds', value: 0.1, origin: 'code' },
11321140
{ name: 'env', value: 'test', origin: 'code' },
11331141
{ name: 'experimental.enableGetRumData', value: true, origin: 'code' },
11341142
{ name: 'experimental.exporter', value: 'log', origin: 'code' },
@@ -1340,6 +1348,7 @@ describe('Config', () => {
13401348
process.env.DD_DYNAMIC_INSTRUMENTATION_ENABLED = 'true'
13411349
process.env.DD_DYNAMIC_INSTRUMENTATION_REDACTED_IDENTIFIERS = 'foo,bar'
13421350
process.env.DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS = 'a,b,c'
1351+
process.env.DD_DYNAMIC_INSTRUMENTATION_UPLOAD_INTERVAL_SECONDS = '0.1'
13431352
process.env.DD_API_KEY = '123'
13441353
process.env.DD_TRACE_SPAN_ATTRIBUTE_SCHEMA = 'v0'
13451354
process.env.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED = 'false'
@@ -1425,7 +1434,8 @@ describe('Config', () => {
14251434
dynamicInstrumentation: {
14261435
enabled: false,
14271436
redactedIdentifiers: ['foo2', 'bar2'],
1428-
redactionExcludedIdentifiers: ['a2', 'b2']
1437+
redactionExcludedIdentifiers: ['a2', 'b2'],
1438+
uploadIntervalSeconds: 0.2
14291439
},
14301440
experimental: {
14311441
b3: false,
@@ -1513,6 +1523,7 @@ describe('Config', () => {
15131523
expect(config).to.have.nested.property('dynamicInstrumentation.enabled', false)
15141524
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactedIdentifiers', ['foo2', 'bar2'])
15151525
expect(config).to.have.nested.deep.property('dynamicInstrumentation.redactionExcludedIdentifiers', ['a2', 'b2'])
1526+
expect(config).to.have.nested.property('dynamicInstrumentation.uploadIntervalSeconds', 0.2)
15161527
expect(config).to.have.property('env', 'development')
15171528
expect(config).to.have.property('clientIpEnabled', true)
15181529
expect(config).to.have.property('clientIpHeader', 'x-true-client-ip')

‎packages/dd-trace/test/debugger/devtools_client/send.spec.js

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
require('../../setup/mocha')
44

55
const { hostname: getHostname } = require('os')
6-
const { expectWithin, getRequestOptions } = require('./utils')
6+
const sinon = require('sinon')
7+
const { getRequestOptions } = require('./utils')
78
const JSONBuffer = require('../../../src/debugger/devtools_client/json-buffer')
89
const { version } = require('../../../../../package.json')
910

@@ -21,9 +22,11 @@ const dd = { dd: true }
2122
const snapshot = { snapshot: true }
2223

2324
describe('input message http requests', function () {
24-
let send, request, jsonBuffer
25+
let clock, send, request, jsonBuffer
2526

2627
beforeEach(function () {
28+
clock = sinon.useFakeTimers()
29+
2730
request = sinon.spy()
2831
request['@noCallThru'] = true
2932

@@ -36,12 +39,26 @@ describe('input message http requests', function () {
3639
}
3740

3841
send = proxyquire('../src/debugger/devtools_client/send', {
39-
'./config': { service, commitSHA, repositoryUrl, url, '@noCallThru': true },
42+
'./config': {
43+
service,
44+
commitSHA,
45+
repositoryUrl,
46+
url,
47+
maxTotalPayloadSize: 5 * 1024 * 1024, // 5MB
48+
dynamicInstrumentation: {
49+
uploadIntervalSeconds: 1
50+
},
51+
'@noCallThru': true
52+
},
4053
'./json-buffer': JSONBufferSpy,
4154
'../../exporters/common/request': request
4255
})
4356
})
4457

58+
afterEach(function () {
59+
clock.restore()
60+
})
61+
4562
it('should buffer instead of calling request directly', function () {
4663
const callback = sinon.spy()
4764

@@ -59,28 +76,28 @@ describe('input message http requests', function () {
5976
send({ message: 3 }, logger, dd, snapshot)
6077
expect(request).to.not.have.been.called
6178

62-
expectWithin(1200, () => {
63-
expect(request).to.have.been.calledOnceWith(JSON.stringify([
64-
getPayload({ message: 1 }),
65-
getPayload({ message: 2 }),
66-
getPayload({ message: 3 })
67-
]))
68-
69-
const opts = getRequestOptions(request)
70-
expect(opts).to.have.property('method', 'POST')
71-
expect(opts).to.have.property(
72-
'path',
73-
'/debugger/v1/input?ddtags=' +
74-
`env%3A${process.env.DD_ENV}%2C` +
75-
`version%3A${process.env.DD_VERSION}%2C` +
76-
`debugger_version%3A${version}%2C` +
77-
`host_name%3A${hostname}%2C` +
78-
`git.commit.sha%3A${commitSHA}%2C` +
79-
`git.repository_url%3A${repositoryUrl}`
80-
)
81-
82-
done()
83-
})
79+
clock.tick(1000)
80+
81+
expect(request).to.have.been.calledOnceWith(JSON.stringify([
82+
getPayload({ message: 1 }),
83+
getPayload({ message: 2 }),
84+
getPayload({ message: 3 })
85+
]))
86+
87+
const opts = getRequestOptions(request)
88+
expect(opts).to.have.property('method', 'POST')
89+
expect(opts).to.have.property(
90+
'path',
91+
'/debugger/v1/input?ddtags=' +
92+
`env%3A${process.env.DD_ENV}%2C` +
93+
`version%3A${process.env.DD_VERSION}%2C` +
94+
`debugger_version%3A${version}%2C` +
95+
`host_name%3A${hostname}%2C` +
96+
`git.commit.sha%3A${commitSHA}%2C` +
97+
`git.repository_url%3A${repositoryUrl}`
98+
)
99+
100+
done()
84101
})
85102
})
86103

‎packages/dd-trace/test/debugger/devtools_client/status.spec.js

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@
22

33
require('../../setup/mocha')
44

5-
const { expectWithin, getRequestOptions } = require('./utils')
5+
const sinon = require('sinon')
6+
const { getRequestOptions } = require('./utils')
67
const JSONBuffer = require('../../../src/debugger/devtools_client/json-buffer')
78

89
const ddsource = 'dd_debugger'
910
const service = 'my-service'
1011
const runtimeId = 'my-runtime-id'
1112

1213
describe('diagnostic message http requests', function () {
13-
let statusproxy, request, jsonBuffer
14+
let clock, statusproxy, request, jsonBuffer
1415

1516
const acks = [
1617
['ackReceived', 'RECEIVED'],
@@ -20,6 +21,8 @@ describe('diagnostic message http requests', function () {
2021
]
2122

2223
beforeEach(function () {
24+
clock = sinon.useFakeTimers()
25+
2326
request = sinon.spy()
2427
request['@noCallThru'] = true
2528

@@ -32,12 +35,24 @@ describe('diagnostic message http requests', function () {
3235
}
3336

3437
statusproxy = proxyquire('../src/debugger/devtools_client/status', {
35-
'./config': { service, runtimeId, '@noCallThru': true },
38+
'./config': {
39+
service,
40+
runtimeId,
41+
maxTotalPayloadSize: 5 * 1024 * 1024, // 5MB
42+
dynamicInstrumentation: {
43+
uploadIntervalSeconds: 1
44+
},
45+
'@noCallThru': true
46+
},
3647
'./json-buffer': JSONBufferSpy,
3748
'../../exporters/common/request': request
3849
})
3950
})
4051

52+
afterEach(function () {
53+
clock.restore()
54+
})
55+
4156
for (const [ackFnName, status, err] of acks) {
4257
describe(ackFnName, function () {
4358
let ackFn, exception
@@ -107,23 +122,23 @@ describe('diagnostic message http requests', function () {
107122
ackFn({ id: 'bar', version: 0 })
108123
expect(request).to.not.have.been.called
109124

110-
expectWithin(1200, () => {
111-
expect(request).to.have.been.calledOnce
125+
clock.tick(1000)
126+
127+
expect(request).to.have.been.calledOnce
112128

113-
const payload = getFormPayload(request)
129+
const payload = getFormPayload(request)
114130

115-
expect(payload).to.deep.equal([
116-
formatAsDiagnosticsEvent({ probeId: 'foo', version: 0, status, exception }),
117-
formatAsDiagnosticsEvent({ probeId: 'foo', version: 1, status, exception }),
118-
formatAsDiagnosticsEvent({ probeId: 'bar', version: 0, status, exception })
119-
])
131+
expect(payload).to.deep.equal([
132+
formatAsDiagnosticsEvent({ probeId: 'foo', version: 0, status, exception }),
133+
formatAsDiagnosticsEvent({ probeId: 'foo', version: 1, status, exception }),
134+
formatAsDiagnosticsEvent({ probeId: 'bar', version: 0, status, exception })
135+
])
120136

121-
const opts = getRequestOptions(request)
122-
expect(opts).to.have.property('method', 'POST')
123-
expect(opts).to.have.property('path', '/debugger/v1/diagnostics')
137+
const opts = getRequestOptions(request)
138+
expect(opts).to.have.property('method', 'POST')
139+
expect(opts).to.have.property('path', '/debugger/v1/diagnostics')
124140

125-
done()
126-
})
141+
done()
127142
})
128143
})
129144
}

0 commit comments

Comments
 (0)