Skip to content

Commit 31b3924

Browse files
authored
tests: fix apm.test.js breakage with [email protected] (#106)
In elastic/apm-agent-nodejs#2294 improve the behaviour of apm.flush() to wait for inflight spans. This improvement broke a faulty assumption in the apm.test.js code for pino and winston: those tests assumed that calling `apm.flush()` as they did would result in a single intake request with both transaction and span. What actually happened with the newer flush implementation is *two* intake requests: one with the span (explicitly flushed by the apm.flush() call), and the second with the transaction (flushed on process exit after the request completed).
1 parent 868d244 commit 31b3924

File tree

6 files changed

+12
-18
lines changed

6 files changed

+12
-18
lines changed

loggers/morgan/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"devDependencies": {
4141
"ajv": "^7.0.3",
4242
"ajv-formats": "^1.5.1",
43-
"elastic-apm-node": "^3.10.0",
43+
"elastic-apm-node": "^3.23.0",
4444
"express": "^4.17.1",
4545
"morgan": "^1.9.1",
4646
"split2": "^3.1.1",

loggers/pino/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
"@types/pino": "^6.3.9",
4343
"ajv": "^7.0.3",
4444
"ajv-formats": "^1.5.1",
45-
"elastic-apm-node": "^3.14.0",
45+
"elastic-apm-node": "^3.23.0",
4646
"express": "^4.17.1",
4747
"pino": "^6.0.0",
4848
"pino-http": "^5.3.0",

loggers/pino/test/apm.test.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ test('tracing integration works', t => {
8585
app.stdout.pipe(split(JSON.parse)).on('data', function (logObj) {
8686
if (!handledFirstLogLine) {
8787
handledFirstLogLine = true
88-
t.equal(logObj.message, 'listening')
88+
t.equal(logObj.message, 'listening', 'got "listening" log message')
8989
t.ok(logObj.address, 'first listening log line has "address"')
9090
cb(null, logObj.address)
9191
} else {
@@ -121,7 +121,7 @@ test('tracing integration works', t => {
121121
}
122122
if (logObj) {
123123
t.ok(validate(logObj), 'logObj is ECS valid')
124-
t.equal(ecsLoggingValidate(logObj), null)
124+
t.equal(ecsLoggingValidate(logObj), null, 'logObj is ecs-logging valid')
125125
logObjs.push(logObj)
126126
}
127127
if (traceObjs.length >= 3 && logObjs.length >= 1) {
@@ -154,23 +154,23 @@ test('tracing integration works', t => {
154154
}
155155

156156
step1StartMockApmServer(function onListening (apmServerErr, apmServerUrl) {
157-
t.ifErr(apmServerErr)
157+
t.error(apmServerErr, 'no error starting mock APM server')
158158
if (apmServerErr) {
159159
finish()
160160
return
161161
}
162162
t.ok(apmServerUrl, 'apmServerUrl: ' + apmServerUrl)
163163

164164
step2StartApp(apmServerUrl, function onReady (appErr, appUrl) {
165-
t.ifErr(appErr)
165+
t.error(appErr, 'no error starting app')
166166
if (appErr) {
167167
finish()
168168
return
169169
}
170170
t.ok(appUrl, 'appUrl: ' + appUrl)
171171

172172
step3CallApp(appUrl, function (clientErr) {
173-
t.ifErr(clientErr)
173+
t.error(clientErr, 'no error calling app')
174174

175175
// The thread of control now is expected to be in
176176
// `collectTracesLogsAndCheck()`.

loggers/pino/test/serve-one-http-req-with-apm.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@
2525
// - log once when it is listening (with its address)
2626
// - handle a single HTTP request
2727
// - log that request
28-
// - flush APM (i.e. ensure it has sent its data to its configured APM server)
29-
// - exit
28+
// - exit (the APM agent should flush trace data on exit)
3029

3130
const serverUrl = process.argv[2]
3231
const disableApmIntegration = process.argv[3] === 'true'
@@ -57,9 +56,7 @@ server.once('request', function handler (req, res) {
5756
span.end()
5857
res.end('ok')
5958
log.info({ req, res }, 'handled request')
60-
apm.flush(function onFlushed () {
61-
server.close()
62-
})
59+
server.close()
6360
})
6461
})
6562

loggers/winston/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
"ajv": "^7.0.3",
4444
"ajv-formats": "^1.5.1",
4545
"autocannon": "^7.0.1",
46-
"elastic-apm-node": "^3.14.0",
46+
"elastic-apm-node": "^3.23.0",
4747
"express": "^4.17.1",
4848
"split2": "^3.2.2",
4949
"standard": "16.x",

loggers/winston/test/serve-one-http-req-with-apm.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@
2525
// - log once when it is listening (with its address)
2626
// - handle a single HTTP request
2727
// - log that request
28-
// - flush APM (i.e. ensure it has sent its data to its configured APM server)
29-
// - exit
28+
// - exit (the APM agent should flush trace data on exit)
3029

3130
const serverUrl = process.argv[2]
3231
const disableApmIntegration = process.argv[3] === 'true'
@@ -63,9 +62,7 @@ server.once('request', function handler (req, res) {
6362
span.end()
6463
res.end('ok')
6564
log.info('handled request', { req, res })
66-
apm.flush(function onFlushed () {
67-
server.close()
68-
})
65+
server.close()
6966
})
7067
})
7168

0 commit comments

Comments
 (0)