Skip to content

Commit 961f1fb

Browse files
david-lunadavid-luna
authored
chore: refactor tests for duration options (#3764)
1 parent 0e3e002 commit 961f1fb

File tree

2 files changed

+86
-179
lines changed

2 files changed

+86
-179
lines changed

‎test/config.test.js

+1-177
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,9 @@ const Agent = require('../lib/agent');
2020
const { MockAPMServer } = require('./_mock_apm_server');
2121
const { MockLogger } = require('./_mock_logger');
2222
const { NoopApmClient } = require('../lib/apm-client/noop-apm-client');
23-
const { secondsFromDuration } = require('../lib/config/normalizers');
24-
const {
25-
getDefaultOptions,
26-
ENV_TABLE,
27-
DURATION_OPTS,
28-
} = require('../lib/config/schema');
23+
const { ENV_TABLE } = require('../lib/config/schema');
2924
const config = require('../lib/config/config');
3025

31-
const DEFAULTS = getDefaultOptions();
32-
3326
// Options to pass to `agent.start()` to turn off some default agent behavior
3427
// that is unhelpful for these tests.
3528
const agentOpts = {
@@ -100,175 +93,6 @@ function assertEncodedError(t, error, result, trans, parent) {
10093

10194
// ---- tests
10295

103-
DURATION_OPTS.forEach(function (optSpec) {
104-
const key = optSpec.name;
105-
106-
// Skip the deprecated `spanFramesMinDuration` because config normalization
107-
// converts it to `spanStackTraceMinDuration` and then removes it.
108-
if (key === 'spanFramesMinDuration') {
109-
return;
110-
}
111-
112-
let def;
113-
if (key in DEFAULTS) {
114-
def = secondsFromDuration(
115-
DEFAULTS[key],
116-
optSpec.defaultUnit,
117-
optSpec.allowedUnits,
118-
optSpec.allowNegative,
119-
key,
120-
new MockLogger(),
121-
);
122-
} else if (key === 'spanStackTraceMinDuration') {
123-
// Because of special handling in normalizeSpanStackTraceMinDuration()
124-
// `spanStackTraceMinDuration` is not listed in `DEFAULTS`.
125-
def = -1;
126-
} else {
127-
def = undefined;
128-
}
129-
130-
if (!optSpec.allowNegative) {
131-
test(key + ' should guard against a negative time', function (t) {
132-
var agent = new Agent();
133-
var logger = new MockLogger();
134-
agent.start(
135-
Object.assign({}, agentOptsNoopTransport, {
136-
[key]: '-3s',
137-
logger,
138-
}),
139-
);
140-
141-
if (def === undefined) {
142-
t.strictEqual(
143-
agent._conf[key],
144-
undefined,
145-
'config opt was removed from agent._conf',
146-
);
147-
} else {
148-
t.strictEqual(agent._conf[key], def, 'fell back to default value');
149-
}
150-
const warning = logger.calls.find((log) => log.type === 'warn');
151-
t.equal(warning.type, 'warn', 'got a log.warn');
152-
t.ok(
153-
warning.message.indexOf('-3s') !== -1,
154-
'warning message includes the invalid value',
155-
);
156-
t.ok(
157-
warning.message.indexOf(key) !== -1,
158-
'warning message includes the invalid key',
159-
);
160-
161-
agent.destroy();
162-
t.end();
163-
});
164-
}
165-
166-
test(key + ' should guard against a bogus non-time', function (t) {
167-
var agent = new Agent();
168-
var logger = new MockLogger();
169-
agent.start(
170-
Object.assign({}, agentOptsNoopTransport, {
171-
[key]: 'bogusvalue',
172-
logger,
173-
}),
174-
);
175-
176-
if (def === undefined) {
177-
t.strictEqual(
178-
agent._conf[key],
179-
undefined,
180-
'config opt was removed from agent._conf',
181-
);
182-
} else {
183-
t.strictEqual(agent._conf[key], def, 'fell back to default value');
184-
}
185-
const warning = logger.calls.find((log) => log.type === 'warn');
186-
t.equal(warning.type, 'warn', 'got a log.warn');
187-
t.ok(
188-
warning.message.indexOf('bogusvalue') !== -1,
189-
'warning message includes the invalid value',
190-
);
191-
t.ok(
192-
warning.message.indexOf(key) !== -1,
193-
'warning message includes the invalid key',
194-
);
195-
196-
agent.destroy();
197-
t.end();
198-
});
199-
200-
test(key + ' warn about units not beng set', function (t) {
201-
var agent = new Agent();
202-
var logger = new MockLogger();
203-
var opts = { logger };
204-
opts[key] = '1';
205-
agent.start(Object.assign({}, agentOptsNoopTransport, opts));
206-
const warning = logger.calls.find((log) => log.type === 'warn');
207-
t.equal(warning.type, 'warn', 'got a log.warn');
208-
t.ok(
209-
warning.message.indexOf('units missing') !== -1,
210-
'warning message tells about missing units',
211-
);
212-
t.ok(
213-
warning.message.indexOf(key) !== -1,
214-
'warning message contains the key',
215-
);
216-
agent.destroy();
217-
t.end();
218-
});
219-
220-
test(key + ' should convert minutes to seconds', function (t) {
221-
var agent = new Agent();
222-
var opts = {};
223-
opts[key] = '1m';
224-
agent.start(Object.assign({}, agentOptsNoopTransport, opts));
225-
t.strictEqual(agent._conf[key], 60);
226-
agent.destroy();
227-
t.end();
228-
});
229-
230-
test(key + ' should convert milliseconds to seconds', function (t) {
231-
var agent = new Agent();
232-
var opts = {};
233-
opts[key] = '2000ms';
234-
agent.start(Object.assign({}, agentOptsNoopTransport, opts));
235-
t.strictEqual(agent._conf[key], 2);
236-
agent.destroy();
237-
t.end();
238-
});
239-
240-
test(key + ' should parse seconds', function (t) {
241-
var agent = new Agent();
242-
var opts = {};
243-
opts[key] = '5s';
244-
agent.start(Object.assign({}, agentOptsNoopTransport, opts));
245-
t.strictEqual(agent._conf[key], 5);
246-
agent.destroy();
247-
t.end();
248-
});
249-
250-
test(key + ' should support bare numbers', function (t) {
251-
var agent = new Agent();
252-
var opts = {};
253-
opts[key] = 10;
254-
agent.start(Object.assign({}, agentOptsNoopTransport, opts));
255-
var expectedVal;
256-
switch (optSpec.defaultUnit) {
257-
case 's':
258-
expectedVal = 10;
259-
break;
260-
case 'ms':
261-
expectedVal = 10 / 1e3;
262-
break;
263-
default:
264-
throw new Error(`unexpected defaultUnit: ${optSpec.defaultUnit}`);
265-
}
266-
t.strictEqual(agent._conf[key], expectedVal);
267-
agent.destroy();
268-
t.end();
269-
});
270-
});
271-
27296
var keyValuePairValues = ['addPatch', 'globalLabels'];
27397

27498
keyValuePairValues.forEach(function (key) {

‎test/config/config.test.js

+85-2
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,10 @@ const testFixtures = [
342342
'Elastic APM agent disabled (`active` is false)',
343343
'got a debug log about agent disabled',
344344
);
345-
// XXX: normalization turns this value to `undefined` because "bogus"
346-
// is not a boolean. Do we want to keep it that way?
345+
// TODO: normalization turns this value to `undefined` because "bogus"
346+
// is not a boolean. This is not what should happen according to the specs
347+
// so we need to change this behaviour
348+
// https://github.com/elastic/apm/blob/main/specs/agents/configuration.md#invalid-configuration-values
347349
t.equal(
348350
resolvedConfig.active,
349351
undefined,
@@ -421,6 +423,87 @@ const testFixtures = [
421423
);
422424
},
423425
},
426+
{
427+
name: 'use agent - should support duration options',
428+
script: 'fixtures/use-agent.js',
429+
cwd: __dirname,
430+
noConvenienceConfig: true,
431+
env: {
432+
ELASTIC_APM_ABORTED_ERROR_THRESHOLD: '10s',
433+
ELASTIC_APM_API_REQUEST_TIME: '1m',
434+
ELASTIC_APM_EXIT_SPAN_MIN_DURATION: 'bogus',
435+
ELASTIC_APM_METRICS_INTERVAL: '500ms',
436+
TEST_APM_START_OPTIONS: JSON.stringify({
437+
serverTimeout: 30,
438+
spanCompressionExactMatchMaxDuration: '200',
439+
spanCompressionSameKindMaxDuration: '-100ms',
440+
spanStackTraceMinDuration: '-1s',
441+
}),
442+
},
443+
checkScriptResult: (t, err, stdout) => {
444+
t.error(err, `use-agent.js script succeeded: err=${err}`);
445+
const useAgentLogs = getUseAgentLogs(stdout);
446+
const warnLogs = getApmLogs(stdout, 'warn');
447+
const resolvedConfig = JSON.parse(useAgentLogs[2]);
448+
449+
t.equal(
450+
resolvedConfig.abortedErrorThreshold,
451+
10,
452+
'seconds are parsed correcly',
453+
);
454+
t.equal(
455+
resolvedConfig.apiRequestTime,
456+
60,
457+
'minutes are converted to seconds',
458+
);
459+
t.equal(
460+
resolvedConfig.metricsInterval,
461+
0.5,
462+
'miliseconds are converted to seconds',
463+
);
464+
t.equal(
465+
resolvedConfig.serverTimeout,
466+
30,
467+
'number value is accepted with default unit',
468+
);
469+
t.equal(
470+
resolvedConfig.exitSpanMinDuration,
471+
0,
472+
'bogus value is not accepted and used default instead',
473+
);
474+
t.equal(
475+
warnLogs[0].message,
476+
'invalid duration value "bogus" for "exitSpanMinDuration" config option: using default "0ms"',
477+
'agent warns about a bogus value in the configuration',
478+
);
479+
t.equal(
480+
resolvedConfig.spanCompressionExactMatchMaxDuration,
481+
0.2,
482+
'string value with no unit is accepted with default unit',
483+
);
484+
t.equal(
485+
warnLogs[1].message,
486+
'units missing in duration value "200" for "spanCompressionExactMatchMaxDuration" config option: using default units "ms"',
487+
'agent warns about a bogus value in the configuration',
488+
);
489+
490+
t.equal(
491+
resolvedConfig.spanCompressionSameKindMaxDuration,
492+
0,
493+
'not allowed negative values fallback into the default value',
494+
);
495+
t.equal(
496+
warnLogs[2].message,
497+
'invalid duration value "-100ms" for "spanCompressionSameKindMaxDuration" config option: using default "0ms"',
498+
'agent warns about a bogus value in the configuration',
499+
);
500+
t.equal(
501+
resolvedConfig.spanStackTraceMinDuration,
502+
-1,
503+
'allowed negative are parsed correctly',
504+
);
505+
},
506+
},
424507
];
425508

426509
test('agent config fixtures', (suite) => {

0 commit comments

Comments
 (0)