consider bumping the min version of express *tested*

[trentm]

          `4.16.2` is not supported anymore, ideally bump this to `4.21.2`.

Originally posted by @wesleytodd in #2801 (comment)

---

I'm moving the discussion started on the above PR review comment thread to here.

[trentm]

tl;dr: This got long, sorry. IMO, the cost of testing some of those older versions of express is low, so I'd keep testing more than just the latest 4.x version. However, (a) 4.16.4 is from 2018 (7 years old) and (b) this is a decision to be shared with the owners/maintainers of instrumentation-express.

/cc @wesleytodd @JamieDanielson @pkanal

discussion

The discussion from that comment thread was specifically about bumping up the lowest version of Express tested in the "test-all-versions" (TAV) tests. I.e. changing "plugins/node/opentelemetry-instrumentation-express/.tav.yml" from:

express:
  - versions:
      include: ">=4.16.2 <6"
      mode: latest-minors
    commands: npm test

which tests instrumentation-express with these versions:

% npx tav --verbose --dry-run | rg matching
-- 8 package versions matching filter: 4.16.4, 4.17.3, 4.18.3, 4.19.2, 4.20.0, 4.21.2, 5.0.1, 5.1.0

to:

express:
  - versions:
      include: ">=4.21.2 <6"
      mode: latest-minors
    commands: npm test

which would test with these express versions:

% npx tav --verbose --dry-run | rg matching
-- 3 package versions matching filter: 4.21.2, 5.0.1, 5.1.0

---

OTel JS does not have a well-defined common practice or strict policy for exactly what versions of the instrumented package should be tested in the TAV tests.

@wesleytodd said (emphasis mine):

I doubt any impact from CVEs here [from the old versions of express]. I just like to point it out in case there are parts of the systems I am unaware of that if it is not done for a real reason it is best to just avoid the versions with published CVEs.

This is my opinion: The reason for testing with the older versions of instrumented package (express, in this case) is to ensure that the instrumentation works when a user happens to use it with those (possibly obsolete, out-dated, deprecated) versions of the package. Instrumentation-express will be active for those older versions of express (according to the semver range in "src/instrumentation.ts", so it is good for us to test against those older versions to make sure we don't break a user's application.

Anecdotally, in my experience, this testing of old versions has been helpful in catching regressions introduced in instrumentations when new features are added. A recent example is #2787 where a new feature added to instrumentation-kafkajs broke the instrumentation for some older kafkajs versions. A more common example is in instrumentation-aws-sdk which is an instrumentation with lots of parts that instruments a number of @aws-sdk/client-* packages that have had significant internal architectural changes in minor releases.

When to drop support for versions of a package?

We don't have a policy here, I don't believe. I'm not sure any section of https://opentelemetry.io/docs/specs/otel/versioning-and-stability/ really applies.

We have, in the past, stopped either (a) testing or (b) even instrumenting old versions of a particular package when:

1. the cost of maintenance of those old versions hit a qualitative PITA threshold, or
2. the old version clearly was out of support and the owner/maintainer of the particular instrumentation decided to drop it.

Some examples: kafkajs min bumped from 0.1.0 to 0.3.0 (release date 2017-11-19), drop aws-sdk v2 (last release 2024-11-06) was sooner than I might have done :), slightly bump the min restify version supported).

I gather I'm pretty conservative here. OTel JS could probably stand to be less conservative and drop support for ancient versions of packages more aggressively. Having a policy on when that is done would help in messaging to users. Writing that policy is a chunk of work.