support @koa/router@11

[trentm]

@koa/router (aka koa-router) v11.0.0 was released a few days ago. We should update our instrumentation to support it.

Some notes:

• koa-router v11 dropped support for node <v12.
• However, just after the v11 release testing of node v12 was dropped from their CI: https://github.com/koajs/router/commit/fdf7117ad01e248c4726ae2730a5f70a32e1d547 I've asked on that commit what node version is actually supported. That doesn't break us... just might later in our TAV tests. -> v12 is still supported, just not tested in CI.
• Current instrumentation tests fail with the new koa-router:

% npm install @koa/router@11
...

% node --version
v16.15.1

% node test/instrumentation/modules/koa-router/new-name.test.js
TAP version 13
# route naming
ok 1 should be strictly equal
ok 2 should be strictly equal
not ok 3 should be strictly equal
  ---
    operator: equal
    expected: 'GET /hello'
    actual:   'GET unknown route'
    at: assert (/Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:150:7)
    stack: |-
      Error: should be strictly equal
          at Test.assert [as _assert] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:314:54)
          at Test.bound [as _assert] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:99:32)
          at Test.strictEqual (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:478:10)
          at Test.bound [as strictEqual] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:99:32)
          at assert (/Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:150:7)
          at /Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:34:7
          at Object._write (/Users/trentm/el/apm-agent-nodejs8/test/_mock_http_client.js:46:50)
          at Object.sendTransaction (/Users/trentm/el/apm-agent-nodejs8/test/_mock_http_client.js:53:12)
          at Instrumentation.addEndedTransaction (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/index.js:339:20)
          at Transaction.end (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/transaction.js:356:32)
          at ServerResponse.<anonymous> (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/http-shared.js:49:36)
          at ServerResponse.f (/Users/trentm/el/apm-agent-nodejs8/node_modules/once/once.js:25:25)
          at ServerResponse.onfinish (/Users/trentm/el/apm-agent-nodejs8/node_modules/end-of-stream/index.js:31:27)
          at /Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/AbstractRunContextManager.js:82:49
          at AsyncHooksRunContextManager.with (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/BasicRunContextManager.js:65:17)
          at ServerResponse.wrapper (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/AbstractRunContextManager.js:82:23)
          at ServerResponse.emit (node:events:539:35)
          at onFinish (node:_http_outgoing:830:10)
          at callback (node:internal/streams/writable:552:21)
          at afterWrite (node:internal/streams/writable:497:5)
          at afterWriteTick (node:internal/streams/writable:484:10)
          at processTicksAndRejections (node:internal/process/task_queues:82:21)
  ...
ok 4 should be strictly equal
ok 5 should be strictly equal
ok 6 should be strictly equal
ok 7 should be strictly equal
ok 8 should be strictly equal
# route naming with params
ok 9 should be strictly equal
ok 10 should be strictly equal
not ok 11 should be strictly equal
  ---
    operator: equal
    expected: |-
      'GET /hello/:name'
    actual: |-
      'GET unknown route'
    at: assert (/Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:150:7)
    stack: |-
      Error: should be strictly equal
          at Test.assert [as _assert] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:314:54)
          at Test.bound [as _assert] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:99:32)
          at Test.strictEqual (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:478:10)
          at Test.bound [as strictEqual] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:99:32)
          at assert (/Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:150:7)
          at /Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:52:7
          at Object._write (/Users/trentm/el/apm-agent-nodejs8/test/_mock_http_client.js:46:50)
          at Object.sendTransaction (/Users/trentm/el/apm-agent-nodejs8/test/_mock_http_client.js:53:12)
          at Instrumentation.addEndedTransaction (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/index.js:339:20)
          at Transaction.end (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/transaction.js:356:32)
          at ServerResponse.<anonymous> (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/http-shared.js:49:36)
          at ServerResponse.f (/Users/trentm/el/apm-agent-nodejs8/node_modules/once/once.js:25:25)
          at ServerResponse.onfinish (/Users/trentm/el/apm-agent-nodejs8/node_modules/end-of-stream/index.js:31:27)
          at /Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/AbstractRunContextManager.js:82:49
          at AsyncHooksRunContextManager.with (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/BasicRunContextManager.js:65:17)
          at ServerResponse.wrapper (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/AbstractRunContextManager.js:82:23)
          at ServerResponse.emit (node:events:539:35)
          at onFinish (node:_http_outgoing:830:10)
          at callback (node:internal/streams/writable:552:21)
          at afterWrite (node:internal/streams/writable:497:5)
          at afterWriteTick (node:internal/streams/writable:484:10)
          at processTicksAndRejections (node:internal/process/task_queues:82:21)
  ...
ok 12 should be strictly equal
ok 13 should be strictly equal
ok 14 should be strictly equal
ok 15 should be strictly equal
ok 16 should be strictly equal
# nested routes
ok 17 should be strictly equal
ok 18 should be strictly equal
not ok 19 should be strictly equal
  ---
    operator: equal
    expected: 'GET /prefix1/prefix2/hello'
    actual:   'GET unknown route'
    at: assert (/Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:150:7)
    stack: |-
      Error: should be strictly equal
          at Test.assert [as _assert] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:314:54)
          at Test.bound [as _assert] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:99:32)
          at Test.strictEqual (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:478:10)
          at Test.bound [as strictEqual] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:99:32)
          at assert (/Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:150:7)
          at /Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:70:7
          at Object._write (/Users/trentm/el/apm-agent-nodejs8/test/_mock_http_client.js:46:50)
          at Object.sendTransaction (/Users/trentm/el/apm-agent-nodejs8/test/_mock_http_client.js:53:12)
          at Instrumentation.addEndedTransaction (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/index.js:339:20)
          at Transaction.end (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/transaction.js:356:32)
          at ServerResponse.<anonymous> (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/http-shared.js:49:36)
          at ServerResponse.f (/Users/trentm/el/apm-agent-nodejs8/node_modules/once/once.js:25:25)
          at ServerResponse.onfinish (/Users/trentm/el/apm-agent-nodejs8/node_modules/end-of-stream/index.js:31:27)
          at /Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/AbstractRunContextManager.js:82:49
          at AsyncHooksRunContextManager.with (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/BasicRunContextManager.js:65:17)
          at ServerResponse.wrapper (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/AbstractRunContextManager.js:82:23)
          at ServerResponse.emit (node:events:539:35)
          at onFinish (node:_http_outgoing:830:10)
          at callback (node:internal/streams/writable:552:21)
          at afterWrite (node:internal/streams/writable:497:5)
          at afterWriteTick (node:internal/streams/writable:484:10)
          at processTicksAndRejections (node:internal/process/task_queues:82:21)
  ...
ok 20 should be strictly equal
ok 21 should be strictly equal
ok 22 should be strictly equal
ok 23 should be strictly equal
ok 24 should be strictly equal
# nested routes with params
ok 25 should be strictly equal
ok 26 should be strictly equal
not ok 27 should be strictly equal
  ---
    operator: equal
    expected: |-
      'GET /prefix1/prefix2/hello/:name'
    actual: |-
      'GET unknown route'
    at: assert (/Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:150:7)
    stack: |-
      Error: should be strictly equal
          at Test.assert [as _assert] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:314:54)
          at Test.bound [as _assert] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:99:32)
          at Test.strictEqual (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:478:10)
          at Test.bound [as strictEqual] (/Users/trentm/el/apm-agent-nodejs8/node_modules/tape/lib/test.js:99:32)
          at assert (/Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:150:7)
          at /Users/trentm/el/apm-agent-nodejs8/test/instrumentation/modules/koa-router/shared.js:88:7
          at Object._write (/Users/trentm/el/apm-agent-nodejs8/test/_mock_http_client.js:46:50)
          at Object.sendTransaction (/Users/trentm/el/apm-agent-nodejs8/test/_mock_http_client.js:53:12)
          at Instrumentation.addEndedTransaction (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/index.js:339:20)
          at Transaction.end (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/transaction.js:356:32)
          at ServerResponse.<anonymous> (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/http-shared.js:49:36)
          at ServerResponse.f (/Users/trentm/el/apm-agent-nodejs8/node_modules/once/once.js:25:25)
          at ServerResponse.onfinish (/Users/trentm/el/apm-agent-nodejs8/node_modules/end-of-stream/index.js:31:27)
          at /Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/AbstractRunContextManager.js:82:49
          at AsyncHooksRunContextManager.with (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/BasicRunContextManager.js:65:17)
          at ServerResponse.wrapper (/Users/trentm/el/apm-agent-nodejs8/lib/instrumentation/run-context/AbstractRunContextManager.js:82:23)
          at ServerResponse.emit (node:events:539:35)
          at onFinish (node:_http_outgoing:830:10)
          at callback (node:internal/streams/writable:552:21)
          at afterWrite (node:internal/streams/writable:497:5)
          at afterWriteTick (node:internal/streams/writable:484:10)
          at processTicksAndRejections (node:internal/process/task_queues:82:21)
  ...
ok 28 should be strictly equal
ok 29 should be strictly equal
ok 30 should be strictly equal
ok 31 should be strictly equal
ok 32 should be strictly equal

1..32
# tests 32
# pass  28
# fail  4

[trentm]

Note that koa-router@12 was recently released as well: '12.0.0': '2022-07-19T00:44:50.443Z' The main change in v12 is: https://github.com/koajs/router/pull/116

[sibelius]

hey @titanism do you have any idea why this broke?

[sibelius]

I'm trying to run all tests using docker

I'm getting this error on mysql

5.7: Pulling from library/mysql ERROR: no matching manifest for linux/arm64/v8 in the manifest list entries

I'm using mac m1

is there a way to run only koa/router tests?

[titanism]

@sibelius is there a specific error you're getting? have you tried running the tests in docker? we're not familiar with this project.

[sibelius]

@titanism it was a question about @koa/router instrumentation https://github.com/elastic/apm-agent-nodejs/blob/main/lib/instrumentation/modules/koa-router.js#L21-L48

Router.match is not working well anymore with koa 11 or koa 12, do you know if anything changed?

[titanism]

Not that I recall - but you can check the changelog or history git diff at https://github.com/koajs/router/releases.

[trentm]

5.7: Pulling from library/mysql ERROR: no matching manifest for linux/arm64/v8 in the manifest list entries

I'm using mac m1

is there a way to run only koa/router tests?

@sibelius If you like, it would be helpful to open a separate issue for this. This is a general issue with this repo's tests. I don't have an M1 mac, so I'm not sure whether I'll be able to make much progress myself.

[trentm]

is there a way to run only koa/router tests?

I don't believe the koa-router tests require any of the services (MySQL, Postgres, etc.) running, so you should be able to run its test files directly:

node test/instrumentation/modules/koa-router/old-name.test.js
node test/instrumentation/modules/koa-router/new-name.test.js

Also, if you want to run the "test-all-versions" (aka TAV) tests to run the koa-router tests against all support koa-router module versions you can run:

TAV=koa-router,@koa/router ./node_modules/.bin/tav

[sibelius]

I'm getting this error when running using node

apm-agent-nodejs/test/instrumentation/modules/koa-router/shared.js:159
  if (agent._transport.destroy) agent._transport.destroy()
  ^

  TypeError: Cannot read properties of null (reading 'destroy')
  at resetAgent (apm-agent-nodejs/test/instrumentation/modules/koa-router/shared.js:159:26)
  at Test.<anonymous> (apm-agent-nodejs/test/instrumentation/modules/koa-router/shared.js:33:5)
    at Test.bound [as _cb] (apm-agent-nodejs/node_modules/tape/lib/test.js:99:32)
    at Test.run (apm-agent-nodejs/node_modules/tape/lib/test.js:117:31)
    at Test.bound [as run] (apm-agent-nodejs/node_modules/tape/lib/test.js:99:32)
    at Immediate.next (apm-agent-nodejs/node_modules/tape/lib/results.js:88:19)
    at processImmediate (node:internal/timers:466:21)

am I doing something wrong?

I install packages using yarn I use nvm to manage node versions, and I'm using node 16

[trentm]

I'm getting this error when running using node

Hrm. I cannot reproduce that locally. Can you try to follow my steps to see if if still fails for you?

% node --version
v16.16.0

% npm ci    # to install the exact versions in package-lock.json
...

% npm ls koa-router
[email protected] /Users/trentm/el/apm-agent-nodejs
└── [email protected]

% node test/instrumentation/modules/koa-router/old-name.test.js
TAP version 13
# route naming
ok 1 should be strictly equal
ok 2 should be strictly equal
ok 3 should be strictly equal
ok 4 should be strictly equal
ok 5 should be strictly equal
ok 6 should be strictly equal
ok 7 should be strictly equal
ok 8 should be strictly equal
# route naming with params
ok 9 should be strictly equal
ok 10 should be strictly equal
ok 11 should be strictly equal
ok 12 should be strictly equal
ok 13 should be strictly equal
ok 14 should be strictly equal
ok 15 should be strictly equal
ok 16 should be strictly equal
# nested routes
ok 17 should be strictly equal
ok 18 should be strictly equal
ok 19 should be strictly equal
ok 20 should be strictly equal
ok 21 should be strictly equal
ok 22 should be strictly equal
ok 23 should be strictly equal
ok 24 should be strictly equal
# nested routes with params
ok 25 should be strictly equal
ok 26 should be strictly equal
ok 27 should be strictly equal
ok 28 should be strictly equal
ok 29 should be strictly equal
ok 30 should be strictly equal
ok 31 should be strictly equal
ok 32 should be strictly equal

1..32
# tests 32
# pass  32

# ok

[trentm]

Ah, do you have ELASTIC_APM_ACTIVE=false in your env? I repro when I set that:

% ELASTIC_APM_ACTIVE=false node test/instrumentation/modules/koa-router/old-name.test.js
TAP version 13
# route naming
/Users/trentm/el/apm-agent-nodejs/test/instrumentation/modules/koa-router/shared.js:159
    if (agent._transport.destroy) agent._transport.destroy()
                         ^

TypeError: Cannot read properties of null (reading 'destroy')
    at resetAgent (/Users/trentm/el/apm-agent-nodejs/test/instrumentation/modules/koa-router/shared.js:159:26)
    at Test.<anonymous> (/Users/trentm/el/apm-agent-nodejs/test/instrumentation/modules/koa-router/shared.js:33:5)
    at Test.bound [as _cb] (/Users/trentm/el/apm-agent-nodejs/node_modules/tape/lib/test.js:99:32)
    at Test.run (/Users/trentm/el/apm-agent-nodejs/node_modules/tape/lib/test.js:117:31)
    at Test.bound [as run] (/Users/trentm/el/apm-agent-nodejs/node_modules/tape/lib/test.js:99:32)
    at Immediate.next (/Users/trentm/el/apm-agent-nodejs/node_modules/tape/lib/results.js:88:19)
    at processImmediate (node:internal/timers:466:21)

[sibelius]

the env was the cause of the bug, thanks

I will try to solve the koa 11/12 now

to test against koa 12 I need to install koa 12 and run the tests?

[trentm]

to test against koa 12 I need to install koa 12 and run the tests?

That's right:

npm install koa-router@12 @koa/router@12
node test/instrumentation/modules/koa-router/old-name.test.js
node test/instrumentation/modules/koa-router/new-name.test.js

And we'll eventually want to replace the koa-router block in ".tav.yml" with something like this:

koa-router-v5-v11:
  name: koa-router
  versions: '>=5.2.0 <11'
  node: '>=6.0.0'
  peerDependencies: koa@2
  commands: node test/instrumentation/modules/koa-router/old-name.test.js
koa-router:
  name: koa-router
  versions: '>=11 <13'
  node: '>=12.0.0'
  peerDependencies: koa@2
  commands: node test/instrumentation/modules/koa-router/old-name.test.js

'@koa/router-v8-v11':
  name: '@koa-router'
  versions: '>=8 <11'
  node: '>=8.0.0'
  peerDependencies: koa@2
  commands: node test/instrumentation/modules/koa-router/new-name.test.js
'@koa/router':
  name: '@koa-router'
  versions: '>=11 <13'
  node: '>=12.0.0'
  peerDependencies: koa@2
  commands: node test/instrumentation/modules/koa-router/new-name.test.js

and then get the following running with the supported versions of node:

TAV=koa-router,@koa/router ./node_modules/.bin/tav

However, start with the former.