feat: added support for express v5 #1654

abhilash-sivan · 2025-03-27T04:56:50Z

This PR sets the new express v5(tagged next major release) as latest version for express in our SDK.
The plan is to adapt v5 for the tests upon release. So till then we will keep the tests to run against express v4.

Express released 5.1.0:

github: https://github.com/expressjs/express/releases/tag/v5.1.0
see npm: https://www.npmjs.com/package/express?activeTab=versions

bin/currency/utils.js

package.json

abhilash-sivan · 2025-03-27T07:34:23Z

packages/opentelemetry-sampler/test/test.js

        ];
-        expect(spanNames).to.have.members(expectedSpanNames);
-        expect(spans.length).to.eql(7);
+        expect(spanNames).to.eql(spans.map(s => s.data.operation));


Addressed here: https://github.com/instana/nodejs/pull/1249/files#r1709329147

The OpenTelemetry sampler runs on Express v5 because our local dependencies include Express v5, and the @opentelemetry/auto-instrumentations-node package indirectly depends on the express package.

@opentelemetry/auto-instrumentations-node is a meta-package that bundles multiple auto-instrumentation libraries, including @opentelemetry/instrumentation-express.

Rather than overriding it with Express v4 and updating to v5 later, we can adapt the tests to v5 now. Changes have been made accordingly.

kirrg001

This is not a feature.

Where is the express v5 test? Could you add a comment please? THanks

abhilash-sivan · 2025-03-27T14:08:19Z

test pointing to v5:

https://github.com/instana/nodejs/pull/1654/files#diff-ceefa79116448c6868e8ea3461511bb6c341754ed3293cd655639ea6f6449801R20

kirrg001 · 2025-03-27T14:34:15Z

test pointing to v5:
https://github.com/instana/nodejs/pull/1654/files#diff-ceefa79116448c6868e8ea3461511bb6c341754ed3293cd655639ea6f6449801R20

The link does not open for me. Could you pls add a comment on the target test? Thanks!

packages/collector/test/tracing/frameworks/express/test.js

packages/core/src/tracing/instrumentation/frameworks/express.js

packages/core/src/util/initializedTooLateHeuristic.js

packages/core/test/util/require_hook/requireHook_test.js

packages/opentelemetry-exporter/test/test.js

packages/opentelemetry-sampler/test/test.js

package.json

packages/core/test/util/require_hook/requireHook_test.js

packages/google-cloud-run/test/metadata_mock/index.js

…na initialization

packages/opentelemetry-exporter/test/test.js

packages/opentelemetry-sampler/test/test.js

abhilash-sivan · 2025-04-01T06:11:48Z

packages/opentelemetry-sampler/test/test.js

        ];
-        expect(spanNames).to.have.members(expectedSpanNames);
-        expect(spans.length).to.eql(7);
+        expect(spanNames).to.eql(spans.map(s => s.data.operation));


The OpenTelemetry sampler runs on Express v5 because our local dependencies include Express v5, and the @opentelemetry/auto-instrumentations-node package indirectly depends on the express package.

@opentelemetry/auto-instrumentations-node is a meta-package that bundles multiple auto-instrumentation libraries, including @opentelemetry/instrumentation-express.

Rather than overriding it with Express v4 and updating to v5 later, we can adapt the tests to v5 now. Changes have been made accordingly.

abhilash-sivan · 2025-04-01T06:12:00Z

packages/opentelemetry-sampler/test/test.js

-      ];
-      expect(spanNames).to.have.members(expectedSpanNames);
-      expect(resp.spans.length).to.be.gte(7);
+      // NOTE: middleware spans are not collected when migrating to express v5 because:


The OpenTelemetry sampler runs on Express v5 because our local dependencies include Express v5, and the @opentelemetry/auto-instrumentations-node package indirectly depends on the express package.

@opentelemetry/auto-instrumentations-node is a meta-package that bundles multiple auto-instrumentation libraries, including @opentelemetry/instrumentation-express.

Rather than overriding it with Express v4 and updating to v5 later, we can adapt the tests to v5 now. Changes have been made accordingly.

abhilash-sivan · 2025-04-01T06:12:38Z

packages/opentelemetry-exporter/test/test.js

  // EXIT www.example.com
-  // 2 x express middleware, 1 x request handler
  // 1 x tlc connect, 1 x tls connect
-  expectExactlyNMatching(spans, 6, [


The OpenTelemetry exporter runs on Express v5 because our local dependencies include Express v5, and the @opentelemetry/auto-instrumentations-node package indirectly depends on the express package.

@opentelemetry/auto-instrumentations-node is a meta-package that bundles multiple auto-instrumentation libraries, including @opentelemetry/instrumentation-express.

Rather than overriding it with Express v4 and updating to v5 later, we can adapt the tests to v5 now. Changes have been made accordingly.

Why did the request handler disappear?

Why don't we use the loadExpressV4 mock for now?

Why did the request handler disappear?

I beleive it is also because of some changes in the v5. I have no resources to point to at this moment. will update.

Why don't we use the loadExpressV4 mock for now?

Already discussed. #1654 (comment)

kirrg001 · 2025-04-01T06:57:57Z

bin/currency/generate-currency-report.js

    }

-    latestVersion = utils.getLatestVersion(currency.name, installedVersion);
+    latestVersion = utils.getLatestVersion({


We no longer need this. Express is latest now

I removed the beta flag for Express but kept this logic for general use cases, even though we don't have any at the moment!

Okay. Could you pls extract these from this PR and create a new PR? Thanks

kirrg001 · 2025-04-01T07:01:30Z

packages/collector/test/tracing/frameworks/express_uncaught_errors/app.js

 });

 require('./mockVersion');
+require('@instana/core/test/test_util/load_express_v4');


This seems wrong

require('./mockVersion');
require('@instana/core/test/test_util/load_express_v4');

Thanks. Corrected!

kirrg001 · 2025-04-01T07:04:06Z

packages/collector/test/tracing/protocols/http/proxy/expressProxy.js

  process.exit(0);
 });

+require('@instana/core/test/test_util/load_express_v4');


TODO: This test needs to run against v4 and v5. You can copy the iteration from the official express test and just just a local mock version file.

kirrg001 · 2025-04-01T07:04:36Z

packages/core/test/test_util/index.js

  sendToParent: require('./sendToParent'),
  stringifyItems: require('./stringifyItems'),
  mockRequire: require('./mockRequire'),
+  load_express_v4: require('./load_express_v4'),


Camelcase vs underscore consistency.

kirrg001 · 2025-04-01T07:06:04Z

packages/core/test/util/require_hook/requireHook_test.js


        requireHook.init({ logger: testUtils.createFakeLogger() });
-        const pattern = requireHook.buildFileNamePattern(['node_modules', 'express', 'lib', 'router', 'route.js']);
+        // Adapt express v5: it has some changes in the folder structure


Could you please update this comment and put v4 and v5 latest version links to the file on github? Thanks

kirrg001 · 2025-04-01T07:11:23Z

packages/opentelemetry-sampler/test/app.js

 }

 require('./tracing');
-


Same here: Why don't we use the loadExpressV4 mock for now?

Even with the mock, this will run on Express v5 because:

@opentelemetry/auto-instrumentations-node is a dependency in our package.

It depends on @opentelemetry/instrumentation-express, which in turn depends on express.

AFAIK, this dependency resolves to our local node_modules since we already have express in our root dependencies.

So, it will run on v5 regardless, which aligns with our need to use v5.

NPM should load express v4, because @opentelemetry/instrumentation-express depends on v4.

:( But it does not

Ah I see. @opentelemetry/instrumentation-express has no peer or prod dependency on express. Thats why

Can't we use overrides for this?

Yes I think so.
But do we need it ? Because we want to anyway update to v5.

If we override this, then we will have to revert it back in the v5 test update

We ship the exporter with a specific pinned version for Otel

"@instana/serverless": "4.9.0",

"@opentelemetry/api": "1.4.1",
"@opentelemetry/core": "1.17.1"

As long as we still test against the right conditions, I am fine with leaving it as it is.
If customers can use Express v5 with these ^ versions, then we an update the test.

Sorry I don't completely understand this.
Are you suggesting that we use the override here to use v4 or just leave it as it is (v5) ?

As long as we still test against the right conditions, I am fine with leaving it as it is.

Conditions: The pinned production versions for the Otel exporter.

kirrg001 · 2025-04-01T08:53:35Z

@abhilash-sivan Please link the currency express ticket to Q2 currency epic. Thanks

abhilash-sivan · 2025-04-01T11:05:03Z

packages/collector/test/tracing/misc/stack_trace/test.js

-
-    beforeEach(async () => {
-      await agentControls.waitUntilAppIsCompletelyInitialized(expressControls.getPid());
+['latest', 'v4'].forEach(version => {


Just wrapped inside the version iterator and added version in application start calls

abhilash-sivan · 2025-04-01T11:06:32Z

packages/collector/test/tracing/protocols/http/proxy/test.js


-mochaSuiteFn('http with proxy', function () {
-  this.timeout(config.getTestTimeout());
+['latest', 'v4'].forEach(version => {


Here also, only wrapped the tests in versions iterator and updated the server start commands with env variable

kirrg001

LGTM

abhilash-sivan commented Mar 27, 2025

View reviewed changes

abhilash-sivan changed the title ~~feat: add express v5 support in currency~~ feat: add express v5 support in currency report Mar 27, 2025

abhilash-sivan added WIP and removed WIP labels Mar 27, 2025

abhilash-sivan marked this pull request as ready for review March 27, 2025 13:57

abhilash-sivan requested a review from a team as a code owner March 27, 2025 13:57

kirrg001 reviewed Mar 27, 2025

View reviewed changes

abhilash-sivan commented Mar 27, 2025

View reviewed changes

packages/collector/test/tracing/frameworks/express/test.js Show resolved Hide resolved

aryamohanan reviewed Mar 27, 2025

View reviewed changes

abhilash-sivan changed the title ~~feat: add express v5 support in currency report~~ feat: added support for express v5 Mar 28, 2025

kirrg001 mentioned this pull request Mar 28, 2025

chore: bump express version in currency report to v5 #1657

Closed

kirrg001 reviewed Mar 28, 2025

View reviewed changes

packages/google-cloud-run/test/metadata_mock/index.js Show resolved Hide resolved

abhilash-sivan force-pushed the feat-add-express-v5 branch 2 times, most recently from c7bbcdf to 7da5e32 Compare April 1, 2025 03:11

abhilash-sivan mentioned this pull request Apr 1, 2025

[Currency Bot] Bumped express from 4.21.2 to 5.1.0 #1660

Closed

abhilash-sivan added 13 commits April 1, 2025 08:48

build: bumper major version to v5

dea7725

test: updated pkg names after express v4 bump to v5

477d6c6

test: fixed hook module load test

ed83037

test: fixed opentelemetry tests

d044c2f

test: fixed shared-metrics test

90d8919

chore: refactor

337a2ef

test: express test refactor

88cc6dc

chore: update express test

e0a0e5e

chore: removed v4 from core

619975d

chore: updated the logic to add beta versions in currency report

b34e07b

test: added comments for middleware changes

41eecdd

chore: updated test apps to use express v4 via global mockrequire

fcb5a7e

chore: cleanup

293bc56

abhilash-sivan added 7 commits April 1, 2025 08:48

refactor: modified test files to use mockrequire exoress before insta…

7a8dc8e

…na initialization

refactor: name change to load-express-v4

6e941ff

refactor: updates

5c47111

test: fixws test and removed unwanted express v4 mockings

1e4d42d

test: update otel tests

84d9b15

chore: cleanup

4ea0eef

build: bumped express to 5.1.0

e921c1b

abhilash-sivan force-pushed the feat-add-express-v5 branch from 7da5e32 to e921c1b Compare April 1, 2025 03:18

abhilash-sivan added 2 commits April 1, 2025 08:52

build: bump dependency overrides

ac7c101

chore: remove beta tag for express in currency

7852482

aryamohanan reviewed Apr 1, 2025

View reviewed changes

packages/opentelemetry-exporter/test/test.js Outdated Show resolved Hide resolved

packages/opentelemetry-sampler/test/test.js Outdated Show resolved Hide resolved

packages/opentelemetry-sampler/test/test.js Outdated Show resolved Hide resolved

abhilash-sivan added 2 commits April 1, 2025 11:34

chore: updated comments

c1f28b1

test: comments

be66566

abhilash-sivan commented Apr 1, 2025

View reviewed changes

kirrg001 reviewed Apr 1, 2025

View reviewed changes

test: resolved pr comments

0b174ab

abhilash-sivan added 2 commits April 1, 2025 16:02

test: fixed misc test

528d26b

chore: revert isBeta changes

3275bc5

abhilash-sivan commented Apr 1, 2025

View reviewed changes

chore: updated the load-express-v4 file to camelCase

f8507a1

kirrg001 approved these changes Apr 1, 2025

View reviewed changes

abhilash-sivan merged commit 8576291 into main Apr 1, 2025
1 check passed

instanacd deleted the feat-add-express-v5 branch June 2, 2025 00:34

feat: added support for express v5 #1654

feat: added support for express v5 #1654

Uh oh!

Conversation

abhilash-sivan commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirrg001 left a comment

Choose a reason for hiding this comment

Uh oh!

abhilash-sivan commented Mar 27, 2025

Uh oh!

kirrg001 commented Mar 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

abhilash-sivan commented Mar 27, 2025 •

edited

Loading

kirrg001 Apr 1, 2025 •

edited

Loading