Skip to content

Conversation

@trentm
Copy link
Contributor

@trentm trentm commented Aug 15, 2025

Now that the mocha + ts-node + Node automatic type-stripping thing is
(mostly) sorted, we should be able to test with Node.js 24 now.

Some special cases:

  • restify doesn't support Node.js v24, so scripts/skip-test-if.js was improved and used to skip restify tests when using Node.js 24
  • the transitive better-sqlite3 dep needed a bug in minor version to compile against Node.js v24
  • shockingly tedious@1 (from 2017) doesn't support Node.js v24, so we skip that combo in the test-all-versions tests

checklist

  • add all the pkg: labels to do the full TAV test run against Node.js 24.

(It sure would be nice to have a pkg:all label or something to say "yup, please test EVERY THING".)

Now that the mocha + ts-node + Node automatic type-stripping thing is
(mostly) sorted, we should be able to test with Node.js 24 now.
@trentm trentm self-assigned this Aug 15, 2025
@trentm trentm changed the title chore: test against Node.js 24 ci: test against Node.js 24 Aug 15, 2025
@codecov
Copy link

codecov bot commented Aug 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.79%. Comparing base (01e507f) to head (85512a2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2984   +/-   ##
=======================================
  Coverage   89.79%   89.79%           
=======================================
  Files         188      188           
  Lines        9295     9295           
  Branches     1907     1907           
=======================================
  Hits         8346     8346           
  Misses        949      949           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trentm
Copy link
Contributor Author

trentm commented Aug 15, 2025

Sigh. npm ci fails with Node.js 24 (at least with v24.1.0 that I have) on macOS (my laptop) and on the Linux is CI

...
npm error gyp ERR! command "/Users/trentm/.nvm/versions/node/v24.1.0/bin/node" "/Users/trentm/tm/opentelemetry-js-contrib14/node_modules/.bin/node-gyp" "rebuild" "--release"
npm error gyp ERR! cwd /Users/trentm/tm/opentelemetry-js-contrib14/node_modules/better-sqlite3
...

We transitively depend on a slightly old better-sqlite3

[email protected] /Users/trentm/src/opentelemetry-js-contrib
├─┬ @opentelemetry/[email protected] -> ./packages/instrumentation-knex
│ └── [email protected]
└─┬ @opentelemetry/[email protected] -> ./packages/instrumentation-typeorm
  └─┬ [email protected]
    └── [email protected] deduped

It was released in 2024-05:

% npm info better-sqlite3 time | tail -22
  '10.1.0': '2024-05-30T17:42:35.774Z',
  '11.0.0': '2024-05-31T01:20:58.872Z',
  '11.1.1': '2024-06-27T17:43:11.604Z',
  '11.1.2': '2024-07-02T16:44:43.911Z',
  '11.2.0': '2024-08-21T06:30:23.979Z',
  '11.2.1': '2024-08-21T15:17:08.796Z',
  '11.3.0': '2024-09-10T04:47:21.330Z',
  '11.4.0': '2024-10-17T20:44:57.041Z',
  '11.5.0': '2024-10-22T03:13:46.681Z',
  '11.6.0': '2024-11-26T01:51:32.249Z',
  '11.7.0': '2024-12-08T02:43:04.945Z',
  '11.7.2': '2025-01-04T19:55:18.857Z',
  '11.8.0': '2025-01-15T03:00:14.948Z',
  '11.8.1': '2025-01-18T06:38:21.822Z',
  '11.9.0': '2025-03-14T02:53:57.223Z',
  '11.9.1': '2025-03-18T02:13:51.069Z',
  '11.10.0': '2025-05-08T03:58:24.171Z',
  '12.0.0': '2025-06-21T20:34:02.532Z',
  '12.1.0': '2025-06-23T23:08:14.800Z',
  '12.1.1': '2025-06-25T07:30:40.899Z',
  '12.2.0': '2025-06-28T23:05:23.502Z'
}

I believe that was fixed in WiseLibs/better-sqlite3#1371
which is included in later 11.x versions.

…pport

We shall see if this breaks something with Node.js 18, which seemed to be
dropped in the same better-sqlite3 change.
@github-actions github-actions bot added pkg:instrumentation-knex pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. labels Aug 15, 2025
@github-actions
Copy link
Contributor

This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature.
Are you familiar with this package? Consider becoming a component owner.

Restify doesn't support Node.js 24. See restify/node-restify#1876

This adds 'SKIP_TEST_IF_NODE_NEWER_THAN' support to the existing scripts/skip-test-if.js
and uses that to handle the skipping.
@trentm trentm added has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions and removed pkg-status:unmaintained:autoclose-scheduled labels Aug 15, 2025
@trentm
Copy link
Contributor Author

trentm commented Aug 15, 2025

^^ fixed an issue with instr-restify: Restify doesn't support Node.js v24. Using scripts/skip-test-if.js to skip testing the package with v24 and later.

(Eventually we may want to deprecate, archive, remove instr-restify.)

@trentm
Copy link
Contributor Author

trentm commented Aug 15, 2025

TAV test failure with instr-tedious:

cd packages/instrumentation-tedious
npm run compile:with-dependencies
npm run test-services:start
npm run test-all-versions:with-services-env

fails at:

fails with Node v24 and [email protected]
-- installing ["[email protected]"]
-- running test "npm run test" with tedious (env: {})

> @opentelemetry/[email protected] test
> nyc mocha 'test/**/*.test.ts'



  tedious
    1) "before each" hook for "should instrument execSql calls"


  0 passing (47ms)
  1 failing

  1) tedious
       "before each" hook for "should instrument execSql calls":
     Uncaught TypeError: tls.createSecurePair is not a function
      at MessageIO.startTls (node_modules/tedious/lib/message-io.js:130:29)
      at Connection.tls (node_modules/tedious/lib/connection.js:1244:24)
      at Connection.dispatchEvent (node_modules/tedious/lib/connection.js:672:45)
      at Connection.processPreLoginResponse (node_modules/tedious/lib/connection.js:752:21)
      at Connection.message (node_modules/tedious/lib/connection.js:1233:21)
      at Connection.dispatchEvent (node_modules/tedious/lib/connection.js:672:45)
      at MessageIO.<anonymous> (node_modules/tedious/lib/connection.js:587:18)
      at MessageIO.emit (node:events:508:28)
      at MessageIO.emit (node:domain:489:12)
      at ReadablePacketStream.<anonymous> (node_modules/tedious/lib/message-io.js:104:16)
      at ReadablePacketStream.emit (node:events:508:28)
      at ReadablePacketStream.emit (node:domain:489:12)
      at addChunk (node_modules/readable-stream/lib/_stream_readable.js:291:12)
      at readableAddChunk (node_modules/readable-stream/lib/_stream_readable.js:278:11)
      at ReadablePacketStream.Readable.push (node_modules/readable-stream/lib/_stream_readable.js:245:10)
      at ReadablePacketStream.Transform.push (node_modules/readable-stream/lib/_stream_transform.js:148:32)
      at ReadablePacketStream._transform (node_modules/tedious/lib/message-io.js:72:16)
      at ReadablePacketStream.Transform._read (node_modules/readable-stream/lib/_stream_transform.js:184:10)
      at ReadablePacketStream.Transform._write (node_modules/readable-stream/lib/_stream_transform.js:172:83)
      at doWrite (node_modules/readable-stream/lib/_stream_writable.js:428:64)
      at writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:417:5)
      at ReadablePacketStream.Writable.write (node_modules/readable-stream/lib/_stream_writable.js:334:11)
      at Socket.ondata (node:internal/streams/readable:1008:24)
      at Socket.emit (node:events:508:28)
      at Socket.emit (node:domain:489:12)
      at addChunk (node:internal/streams/readable:559:12)
      at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
      at Socket.Readable.push (node:internal/streams/readable:390:5)
      at TCP.onStreamRead (node:internal/stream_base_commons:189:23)



with config: {
  userName: 'sa',
  password: 'mssql_passw0rd',
  server: '127.0.0.1',
  authentication: {
    type: 'default',
    options: { userName: 'sa', password: 'mssql_passw0rd' }
  },
  options: {
    port: 1433,
    database: 'master',
    encrypt: true,
    trustServerCertificate: true,
    rowCollectionOnRequestCompletion: true,
    rowCollectionOnDone: true
  }
}
/Users/trentm/tm/opentelemetry-js-contrib14/node_modules/mocha/lib/runner.js:999
    throw err;
    ^

Error: done() called multiple times in hook <tedious "before each" hook for "should instrument execSql calls"> of file /Users/trentm/tm/opentelemetry-js-contrib14/packages/instrumentation-tedious/test/instrumentation.test.ts; in addition, done() received error: ConnectionError: Failed to connect to 127.0.0.1:1433 in 15000ms
    at ConnectionError (/Users/trentm/tm/opentelemetry-js-contrib14/packages/instrumentation-tedious/node_modules/tedious/lib/errors.js:12:12)
    at Connection.connectTimeout (/Users/trentm/tm/opentelemetry-js-contrib14/packages/instrumentation-tedious/node_modules/tedious/lib/connection.js:619:28)
    at listOnTimeout (node:internal/timers:608:17)
    at processTimers (node:internal/timers:543:7) {
  code: 'ETIMEOUT'
}
    at createMultipleDoneError (/Users/trentm/tm/opentelemetry-js-contrib14/node_modules/mocha/lib/errors.js:428:13)
    at multiple (/Users/trentm/tm/opentelemetry-js-contrib14/node_modules/mocha/lib/runnable.js:290:24)
    at done (/Users/trentm/tm/opentelemetry-js-contrib14/node_modules/mocha/lib/runnable.js:301:14)
    at /Users/trentm/tm/opentelemetry-js-contrib14/node_modules/mocha/lib/runnable.js:377:11
    at processTicksAndRejections (node:internal/process/task_queues:105:5) {
  code: 'ERR_MOCHA_MULTIPLE_DONE',
  valueType: 'object',
  value: ConnectionError: Failed to connect to 127.0.0.1:1433 in 15000ms
      at ConnectionError (/Users/trentm/tm/opentelemetry-js-contrib14/packages/instrumentation-tedious/node_modules/tedious/lib/errors.js:12:12)
      at Connection.connectTimeout (/Users/trentm/tm/opentelemetry-js-contrib14/packages/instrumentation-tedious/node_modules/tedious/lib/connection.js:619:28)
      at listOnTimeout (node:internal/timers:608:17)
      at processTimers (node:internal/timers:543:7) {
    code: 'ETIMEOUT'
  }
}

Node.js v24.6.0

I am shocked SHOCKED that a tedious release from 2017 has some issue with Node v24!

The failure is because tedious@1 is attempting to use tls.createSecurePair() which was deprecated in Node.js v0.11.3 and removed in Node.js v14.

options for instr-tedious

  1. Drop support for tedious@1 (last released: '1.15.0': '2017-03-10T21:00:43.676Z',).
  2. Just don't test against tedious@1 with Node 24 and later. This would be a .tav.yml change.

For now I will do option 2 to avoid having a breaking change be part of this PR. Realistically however, we aren't going to "support" someone using tedious@1.

@trentm trentm marked this pull request as ready for review August 18, 2025 16:59
@trentm trentm requested a review from a team as a code owner August 18, 2025 16:59
@trentm trentm enabled auto-merge (squash) August 19, 2025 20:07
@trentm trentm merged commit e6c29be into open-telemetry:main Aug 19, 2025
24 of 25 checks passed
@trentm trentm deleted the trentm-test-node-24 branch August 19, 2025 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions pkg:auto-configuration-propagators pkg:auto-instrumentations-node pkg:auto-instrumentations-web pkg:browser-extension-autoinjection pkg:contrib-test-utils pkg:host-metrics pkg:id-generator-aws-xray pkg:instrumentation-amqplib pkg:instrumentation-aws-lambda pkg:instrumentation-aws-sdk pkg:instrumentation-bunyan pkg:instrumentation-cassandra-driver pkg:instrumentation-connect pkg:instrumentation-cucumber pkg:instrumentation-dataloader pkg:instrumentation-dns pkg:instrumentation-document-load pkg:instrumentation-express pkg:instrumentation-fastify pkg:instrumentation-fs pkg:instrumentation-generic-pool pkg:instrumentation-graphql pkg:instrumentation-hapi pkg:instrumentation-ioredis pkg:instrumentation-kafkajs pkg:instrumentation-knex pkg:instrumentation-koa pkg:instrumentation-long-task pkg:instrumentation-lru-memoizer pkg:instrumentation-memcached pkg:instrumentation-mongodb pkg:instrumentation-mongoose pkg:instrumentation-mysql pkg:instrumentation-mysql2 pkg:instrumentation-nestjs-core pkg:instrumentation-net pkg:instrumentation-oracledb pkg:instrumentation-pg pkg:instrumentation-pino pkg:instrumentation-redis pkg:instrumentation-restify pkg:instrumentation-router pkg:instrumentation-runtime-node pkg:instrumentation-socket.io pkg:instrumentation-tedious pkg:instrumentation-typeorm pkg:instrumentation-undici pkg:instrumentation-user-interaction pkg:instrumentation-winston pkg:plugin-react-load pkg:propagation-utils pkg:propagator-aws-xray pkg:propagator-aws-xray-lambda pkg:propagator-instana pkg:propagator-ot-trace pkg:redis-common pkg:resource-detector-alibaba-cloud pkg:resource-detector-aws pkg:resource-detector-azure pkg:resource-detector-container pkg:resource-detector-docker pkg:resource-detector-gcp pkg:resource-detector-github pkg:resource-detector-instana pkg:sampler-aws-xray pkg:sql-common pkg:test-utils pkg:winston-transport pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants