feat!: set compilation target to es2022 for all packages but api and semantic-conventions

[david-luna]

Which problem is this PR solving?

Sets the compilation target to es2022 to comply with the guidelines the related issue. A cpuple of packages have been kept to target es2017 for compatibility reasons #5393 (comment)

Fixes #5393
Closes: #5462

Short description of the changes

• set the base compilation target to es2022
• remove es5 target for compilation
• keep @opentelemetry/api and @opentelemtry/semantic-conventions to es2017 for backwards compatibility
• update HTTP instrumentation tests which were failing due to a missing meter provider
• update root npm scripts to have a separated compile step for es2017 and es2022

Type of change

Please delete options that are not relevant.

• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

• compiled and tested at root level
• check that compilation fails for @opentelemetry/api and @opentelemtry/semantic-conventions if using a feature of es2018 o higher target.

Checklist:

• Followed the style guidelines of this project
• Unit tests have been modified
• Documentation has been updated

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.95%. Comparing base (492ed35) to head (bd55845).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5456      +/-   ##
==========================================
- Coverage   94.97%   94.95%   -0.03%     
==========================================
  Files         309      309              
  Lines        7958     7924      -34     
  Branches     1692     1606      -86     
==========================================
- Hits         7558     7524      -34     
  Misses        400      400              

see 23 files with indirect coverage changes

[trentm]

I'd expect this to break compilation of these two packages:

experimental/backwards-compatibility/node14/tsconfig.json
2:  "extends": "../../../tsconfig.base.es5.json",

experimental/backwards-compatibility/node16/tsconfig.json
2:  "extends": "../../../tsconfig.base.es5.json",

These are run via npm run test:backcompat ... which I don't be we run automatically anywhere.\

I opened #5460 for this.
These tests are broken and haven't run in ~2y. I don't think this PR needs to worry about this.

[trentm]

Two Qs.

[trentm]

update HTTP instrumentation tests which were failing due to a missing meter provider

I don't see any test failure on "main". I'm not sure what the issue was here.

[pichlermarc]

Hmm, something seems to be wrong here. 🤔

For some reason, the Instrumentation's metrics instruments seem to be undefined without these changes, which should never be the case. Seems like a bug that surfaced from changing the compile settings.

[pichlermarc]

this._updateMetricInstruments() is called but does not define the properties on the HttpInstrumentation instance.

[pichlermarc]

once the base constructor returns, the properties are all undefined. 🤔

[legendecas]

I believe this is due to https://www.typescriptlang.org/tsconfig/#useDefineForClassFields and a simplest fix could be adding the declare modifier to derived instrumentation properties:

export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentationConfig> {
  // add a `declare` modifier.
  declare private _oldHttpServerDurationHistogram!: Histogram;
}

[trentm]

Confirmed that this diff:

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
index 8f0263251..678e579e8 100644
--- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
+++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -95,10 +95,10 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
   /** keep track on spans not ended */
   private readonly _spanNotEnded: WeakSet<Span> = new WeakSet<Span>();
   private _headerCapture;
-  private _oldHttpServerDurationHistogram!: Histogram;
-  private _stableHttpServerDurationHistogram!: Histogram;
-  private _oldHttpClientDurationHistogram!: Histogram;
-  private _stableHttpClientDurationHistogram!: Histogram;
+  declare private _oldHttpServerDurationHistogram: Histogram;
+  declare private _stableHttpServerDurationHistogram: Histogram;
+  declare private _oldHttpClientDurationHistogram: Histogram;
+  declare private _stableHttpClientDurationHistogram: Histogram;

and the instr-http tests pass again.

The diff in the generated JS is:

% diff -u build.predeclare/src/http.js build/src/http.js
--- build.predeclare/src/http.js	2025-02-20 13:57:51
+++ build/src/http.js	2025-02-20 13:59:33
@@ -31,10 +31,6 @@
     /** keep track on spans not ended */
     _spanNotEnded = new WeakSet();
     _headerCapture;
-    _oldHttpServerDurationHistogram;
-    _stableHttpServerDurationHistogram;
-    _oldHttpClientDurationHistogram;
-    _stableHttpClientDurationHistogram;
     _semconvStability = 2 /* SemconvStability.OLD */;
     constructor(config = {}) {
         super('@opentelemetry/instrumentation-http', version_1.VERSION, config);

Pretty subtle :)
The best full write-up of what is going on is: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#the-usedefineforclassfields-flag-and-the-declare-property-modifier
(linked to from @legendecas' link above). Thanks for the pointer.

commit db71061

[trentm]

For my feeble memory, the issue is basically this:

class Base {
  constructor() {
    this._doSomething();
  }
  _doSomething() {
  }
}

class Derived extends Base {
  aVar;
  constructor() {
    super()
  }
  _doSomething() {
    this.aVar = 42;
  }
}

var d = new Derived();
console.log('d.aVar:', d.aVar); // get undefined; Using TS class fields before JS had them, this would get 42.

[trentm]

It occurred to me that we should also be limiting experimental/packages/api-logs to es2017. Its intent is to be merged into the api package.

[david-luna]

It occurred to me that we should also be limiting experimental/packages/api-logs to es2017. Its intent is to be merged into the api package.

I'll update the tsconfig.json there as well. What about api-events? is it intended to be merged into api as well?

[pichlermarc]

It occurred to me that we should also be limiting experimental/packages/api-logs to es2017. Its intent is to be merged into the api package.

I'll update the tsconfig.json there as well. What about api-events? is it intended to be merged into api as well?

api-events will completely go away at some point - but if it's easy to do then I'd still recommend to put it to es2017 for consistency.