Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 26 commits into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
539c406
feat: update compilation target to ES2022
david-luna Feb 12, 2025
1d785c1
Merge branch 'main' into es2022-no-top-level-await
david-luna Feb 12, 2025
8111dec
test: fix enable tests
david-luna Feb 12, 2025
204d0a0
test: fix disable tests
david-luna Feb 12, 2025
6d83f15
test: fix esm tests
david-luna Feb 12, 2025
515416d
chore: update changelog
david-luna Feb 12, 2025
3679793
Merge branch 'main' into es2022-no-top-level-await
david-luna Feb 12, 2025
8f57ec2
chore: fix typo
david-luna Feb 12, 2025
778a9ea
Merge branch 'es2022-no-top-level-await' of github.com:david-luna/ope…
david-luna Feb 12, 2025
9adc09a
chore: update readme
david-luna Feb 12, 2025
81276fb
chore: pin eslint plugin version
david-luna Feb 12, 2025
0d0e6cc
chore: remove unnecessry libs for api and semconv compilation
david-luna Feb 13, 2025
f0aec32
Merge branch 'main' into es2022-no-top-level-await
david-luna Feb 14, 2025
231bd5a
chore: remove unnecessary eslint plugin
david-luna Feb 14, 2025
fd7581e
Merge branch 'main' into es2022-no-top-level-await
david-luna Feb 18, 2025
27a07a7
Merge branch 'main' into es2022-no-top-level-await
david-luna Feb 18, 2025
f2ec659
Merge branch 'main' into es2022-no-top-level-await
david-luna Feb 18, 2025
b2fdca2
chore: remove explicit compilation step for es2017
david-luna Feb 18, 2025
e80e5ae
Merge branch 'es2022-no-top-level-await' of github.com:david-luna/ope…
david-luna Feb 18, 2025
8c608c7
Merge branch 'main' into es2022-no-top-level-await
david-luna Feb 20, 2025
56ae3ee
chore: use @types/node@8 in api and semconv to have proper type checking
david-luna Feb 20, 2025
5cd8397
api-logs: changes its min-node and tsconfig target/libs to match the …
trentm Feb 20, 2025
795d676
semantic-conventions: min node is 14, so use types/node@14 (which is …
trentm Feb 20, 2025
08af24d
api-events: drop min-node to 8, tsconfig target and libs to es2017 to…
trentm Feb 20, 2025
db71061
instr-http tests: undo David's earlier changes and instead use 'decla…
trentm Feb 20, 2025
bd55845
update the changelog entry
trentm Feb 20, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
* (user-facing): all configuration previously possible via `window.OTEL_*` is now not supported anymore
* If you have been using the `envDetector` in browser environments, please migrate to manually creating a resource.
* Note: Node.js environment variable configuration continues to work as-is.
* feat!: set compilation target to ES2022 for all packages except `@opentelemetry/api` and `@opentelemetry/semantic-conventions` [#5456](https://github.com/open-telemetry/opentelemetry-js/pull/5456) @david-luna
* (user-facing): drops browser runtimes which do not support ES2022 features

### :rocket: (Enhancement)

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ underlying language features used.
The current minumum language feature support is set as [ECMAScript 2020](https://262.ecma-international.org/11.0/) that are available
in all modern browsers / runtimes.

This means that if you are targeting or your end-users are using a browser / runtime that does not support ES2020, you will need
This means that if you are targeting or your end-users are using a browser / runtime that does not support ES2022, you will need
to transpile the code and provide any necessary polyfills for the missing features to ensure compatibility with your target
environments. Any support issues that arise from using a browser or runtime that does not support ES2020 will be closed as "won't fix".
environments. Any support issues that arise from using a browser or runtime that does not support ES2022 will be closed as "won't fix".

This minimum support level is subject to change as the project evolves and as the underlying language features evolve.

Expand Down
5 changes: 5 additions & 0 deletions api/tsconfig.esm.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
{
"extends": "../tsconfig.base.esm.json",
"compilerOptions": {
"lib": [
"es2017",
"dom"
],
"outDir": "build/esm",
"rootDir": "src",
"target": "es2017",
"tsBuildInfoFile": "build/esm/tsconfig.esm.tsbuildinfo"
},
"include": [
Expand Down
5 changes: 5 additions & 0 deletions api/tsconfig.esnext.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
{
"extends": "../tsconfig.base.esnext.json",
"compilerOptions": {
"lib": [
"es2017",
"dom"
],
"outDir": "build/esnext",
"rootDir": "src",
"target": "es2017",
"tsBuildInfoFile": "build/esnext/tsconfig.esnext.tsbuildinfo"
},
"include": [
Expand Down
7 changes: 6 additions & 1 deletion api/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
{
"extends": "../tsconfig.base.json",
"compilerOptions": {
"lib": [
"es2017",
"dom"
],
"outDir": "build",
"rootDir": "."
"rootDir": ".",
"target": "es2017"
},
"files": [],
"include": [
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ import {
Attributes,
DiagConsoleLogger,
} from '@opentelemetry/api';
import {
AggregationTemporality,
InMemoryMetricExporter,
MeterProvider,
} from '@opentelemetry/sdk-metrics';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import {
InMemorySpanExporter,
Expand Down Expand Up @@ -70,6 +75,7 @@ import { HttpInstrumentationConfig } from '../../src/types';
import { assertSpan } from '../utils/assertSpan';
import { DummyPropagation } from '../utils/DummyPropagation';
import { httpRequest } from '../utils/httpRequest';
import { TestMetricReader } from '../utils/TestMetricReader';
import { ContextManager } from '@opentelemetry/api';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import type {
Expand All @@ -84,6 +90,18 @@ import { getRPCMetadata, RPCType } from '@opentelemetry/core';
const instrumentation = new HttpInstrumentation();
instrumentation.enable();
instrumentation.disable();
const memoryExporter = new InMemorySpanExporter();
const tracerProvider = new NodeTracerProvider({
spanProcessors: [new SimpleSpanProcessor(memoryExporter)],
});
const metricsMemoryExporter = new InMemoryMetricExporter(
AggregationTemporality.DELTA
);
const metricReader = new TestMetricReader(metricsMemoryExporter);
const meterProvider = new MeterProvider({ readers: [metricReader] });

instrumentation.setTracerProvider(tracerProvider);
instrumentation.setMeterProvider(meterProvider);

import * as http from 'http';
import { AttributeNames } from '../../src/enums/AttributeNames';
Expand All @@ -99,11 +117,6 @@ const protocol = 'http';
const hostname = 'localhost';
const pathname = '/test';
const serverName = 'my.server.name';
const memoryExporter = new InMemorySpanExporter();
const provider = new NodeTracerProvider({
spanProcessors: [new SimpleSpanProcessor(memoryExporter)],
});
instrumentation.setTracerProvider(provider);

function doNock(
hostname: string,
Expand Down Expand Up @@ -342,7 +355,7 @@ describe('HttpInstrumentation', () => {
return;
}
if (request.url?.includes('/ignored')) {
provider.getTracer('test').startSpan('some-span').end();
tracerProvider.getTracer('test').startSpan('some-span').end();
}
if (request.url?.includes('/setroute')) {
const rpcData = getRPCMetadata(context.active());
Expand Down Expand Up @@ -480,7 +493,7 @@ describe('HttpInstrumentation', () => {
const testPath = '/outgoing/rootSpan/childs/1';
doNock(hostname, testPath, 200, 'Ok');
const name = 'TestRootSpan';
const span = provider.getTracer('default').startSpan(name);
const span = tracerProvider.getTracer('default').startSpan(name);
return context.with(trace.setSpan(context.active(), span), async () => {
const result = await httpRequest.get(
`${protocol}://${hostname}${testPath}`
Expand Down Expand Up @@ -523,7 +536,7 @@ describe('HttpInstrumentation', () => {
httpErrorCodes[i].toString()
);
const name = 'TestRootSpan';
const span = provider.getTracer('default').startSpan(name);
const span = tracerProvider.getTracer('default').startSpan(name);
return context.with(
trace.setSpan(context.active(), span),
async () => {
Expand Down Expand Up @@ -565,7 +578,7 @@ describe('HttpInstrumentation', () => {
const num = 5;
doNock(hostname, testPath, 200, 'Ok', num);
const name = 'TestRootSpan';
const span = provider.getTracer('default').startSpan(name);
const span = tracerProvider.getTracer('default').startSpan(name);
await context.with(trace.setSpan(context.active(), span), async () => {
for (let i = 0; i < num; i++) {
await httpRequest.get(`${protocol}://${hostname}${testPath}`);
Expand Down Expand Up @@ -1074,7 +1087,7 @@ describe('HttpInstrumentation', () => {
return;
}
if (request.url?.includes('/ignored')) {
provider.getTracer('test').startSpan('some-span').end();
tracerProvider.getTracer('test').startSpan('some-span').end();
}
if (request.url?.includes('/setroute')) {
const rpcData = getRPCMetadata(context.active());
Expand Down Expand Up @@ -1415,7 +1428,7 @@ describe('HttpInstrumentation', () => {
});
instrumentation.enable();
const testPath = '/test/test';
const tracer = provider.getTracer('default');
const tracer = tracerProvider.getTracer('default');
const span = tracer.startSpan('parentSpan', {
kind: SpanKind.INTERNAL,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,30 @@ import {
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import {
AggregationTemporality,
InMemoryMetricExporter,
MeterProvider,
} from '@opentelemetry/sdk-metrics';
import * as assert from 'assert';
import * as path from 'path';
import * as url from 'url';
import { HttpInstrumentation } from '../../src/http';
import { assertSpan } from '../utils/assertSpan';
import { DummyPropagation } from '../utils/DummyPropagation';
import { TestMetricReader } from '../utils/TestMetricReader';

const instrumentation = new HttpInstrumentation();
instrumentation.enable();
instrumentation.disable();

const metricsMemoryExporter = new InMemoryMetricExporter(
AggregationTemporality.DELTA
);
const metricReader = new TestMetricReader(metricsMemoryExporter);
const meterProvider = new MeterProvider({ readers: [metricReader] });
instrumentation.setMeterProvider(meterProvider);

import * as http from 'http';
import * as superagent from 'superagent';
import * as nock from 'nock';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ import {
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import {
AggregationTemporality,
InMemoryMetricExporter,
MeterProvider,
} from '@opentelemetry/sdk-metrics';
import {
NETTRANSPORTVALUES_IP_TCP,
SEMATTRS_HTTP_CLIENT_IP,
Expand All @@ -45,11 +50,19 @@ import * as path from 'path';
import { HttpInstrumentation } from '../../src/http';
import { assertSpan } from '../utils/assertSpan';
import { DummyPropagation } from '../utils/DummyPropagation';
import { TestMetricReader } from '../utils/TestMetricReader';
import { isWrapped } from '@opentelemetry/instrumentation';

const instrumentation = new HttpInstrumentation();
instrumentation.enable();
instrumentation.disable();
const metricsMemoryExporter = new InMemoryMetricExporter(
AggregationTemporality.DELTA
);
const metricReader = new TestMetricReader(metricsMemoryExporter);
const meterProvider = new MeterProvider({ readers: [metricReader] });

instrumentation.setMeterProvider(meterProvider);

import * as http from 'http';
import * as https from 'https';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,35 @@ import {
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import {
AggregationTemporality,
InMemoryMetricExporter,
MeterProvider,
} from '@opentelemetry/sdk-metrics';
import * as assert from 'assert';
import * as path from 'path';
import * as url from 'url';
import { HttpInstrumentation } from '../../src/http';
import { assertSpan } from '../utils/assertSpan';
import { DummyPropagation } from '../utils/DummyPropagation';
import { TestMetricReader } from '../utils/TestMetricReader';

const instrumentation = new HttpInstrumentation();
instrumentation.enable();
instrumentation.disable();
const memoryExporter = new InMemorySpanExporter();
const metricsMemoryExporter = new InMemoryMetricExporter(
AggregationTemporality.DELTA
);
const metricReader = new TestMetricReader(metricsMemoryExporter);
const meterProvider = new MeterProvider({ readers: [metricReader] });
instrumentation.setMeterProvider(meterProvider);

import * as http from 'http';
import * as superagent from 'superagent';
import * as nock from 'nock';
import * as axios from 'axios';

const memoryExporter = new InMemorySpanExporter();
const customAttributeFunction = (span: Span): void => {
span.setAttribute('span kind', SpanKind.CLIENT);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,27 @@ import {
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import {
AggregationTemporality,
InMemoryMetricExporter,
MeterProvider,
} from '@opentelemetry/sdk-metrics';

import { assertSpan } from '../../build/test/utils/assertSpan.js';
import { TestMetricReader } from '../../build/test/utils/TestMetricReader.js';
import { HttpInstrumentation } from '../../build/src/index.js';

const memoryExporter = new InMemorySpanExporter();
const provider = new NodeTracerProvider({
spanProcessors: [new SimpleSpanProcessor(memoryExporter)],
});
const metricsMemoryExporter = new InMemoryMetricExporter(
AggregationTemporality.DELTA
);
const metricReader = new TestMetricReader(metricsMemoryExporter);
const meterProvider = new MeterProvider({ readers: [metricReader] });
const instrumentation = new HttpInstrumentation();
instrumentation.setMeterProvider(meterProvider);
instrumentation.setTracerProvider(provider);

const httpImports = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,14 @@ import * as assert from 'assert';
import * as url from 'url';
import { HttpInstrumentation } from '../../src/http';
import { assertSpan } from '../utils/assertSpan';
import { TestMetricReader } from '../utils/TestMetricReader';
import * as utils from '../utils/utils';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import {
AggregationTemporality,
InMemoryMetricExporter,
MeterProvider,
} from '@opentelemetry/sdk-metrics';
import {
InMemorySpanExporter,
SimpleSpanProcessor,
Expand All @@ -37,6 +43,13 @@ import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
const instrumentation = new HttpInstrumentation();
instrumentation.enable();
instrumentation.disable();
const memoryExporter = new InMemorySpanExporter();
const metricsMemoryExporter = new InMemoryMetricExporter(
AggregationTemporality.DELTA
);
const metricReader = new TestMetricReader(metricsMemoryExporter);
const meterProvider = new MeterProvider({ readers: [metricReader] });
instrumentation.setMeterProvider(meterProvider);

import * as http from 'http';
import { httpRequest } from '../utils/httpRequest';
Expand All @@ -46,7 +59,6 @@ import { sendRequestTwice } from '../utils/rawRequest';

const protocol = 'http';
const hostname = 'localhost';
const memoryExporter = new InMemorySpanExporter();

const customAttributeFunction = (span: Span): void => {
span.setAttribute('span kind', SpanKind.CLIENT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,24 @@ import {
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import {
AggregationTemporality,
InMemoryMetricExporter,
MeterProvider,
} from '@opentelemetry/sdk-metrics';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import { HttpInstrumentation } from '../../src';
import { TestMetricReader } from '../utils/TestMetricReader';

const instrumentation = new HttpInstrumentation();
instrumentation.enable();
instrumentation.disable();
const metricsMemoryExporter = new InMemoryMetricExporter(
AggregationTemporality.DELTA
);
const metricReader = new TestMetricReader(metricsMemoryExporter);
const meterProvider = new MeterProvider({ readers: [metricReader] });
instrumentation.setMeterProvider(meterProvider);

import * as https from 'https';
import { httpsRequest } from '../utils/httpsRequest';
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"description": "OpenTelemetry is a distributed tracing and stats collection framework.",
"scripts": {
"precompile": "lerna run version && npm run submodule",
"compile": "lerna run protos:generate && tsc --build tsconfig.json tsconfig.esm.json tsconfig.esnext.json",
"compile:es2017": "lerna run --scope @opentelemetry/api --scope @opentelemetry/semantic-conventions compile",
"compile": "lerna run protos:generate && npm run compile:es2017 && tsc --build tsconfig.json tsconfig.esm.json tsconfig.esnext.json",
"prewatch": "npm run precompile",
"watch": "tsc --build --watch tsconfig.json tsconfig.esm.json tsconfig.esnext.json",
"clean": "tsc --build --clean tsconfig.json tsconfig.esm.json tsconfig.esnext.json",
Expand Down
4 changes: 4 additions & 0 deletions semantic-conventions/tsconfig.esm.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
{
"extends": "../tsconfig.base.esm.json",
"compilerOptions": {
"lib": [
"es2017"
],
"outDir": "build/esm",
"rootDir": "src",
"target": "es2017",
"tsBuildInfoFile": "build/esm/tsconfig.esm.tsbuildinfo"
},
"include": [
Expand Down
4 changes: 4 additions & 0 deletions semantic-conventions/tsconfig.esnext.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
{
"extends": "../tsconfig.base.esnext.json",
"compilerOptions": {
"lib": [
"es2017"
],
"outDir": "build/esnext",
"rootDir": "src",
"target": "es2017",
"tsBuildInfoFile": "build/esnext/tsconfig.esnext.tsbuildinfo"
},
"include": [
Expand Down
Loading