Merge branch 'main' into feature/fix-2268

.github/workflows/peer-api.yaml

@@ -10,7 +10,7 @@ jobs:
   peer-api-check:
     runs-on: ubuntu-latest
     container:
-      image: node:18
+      image: node:20
     steps:
       - name: Checkout
         uses: actions/checkout@v4

.github/workflows/release-please-validate.yaml

@@ -10,7 +10,7 @@ jobs:
   rp-validate:
     runs-on: ubuntu-latest
     container:
-      image: node:14
+      image: node:20
     steps:
       - name: Checkout
         uses: actions/checkout@v4

.github/workflows/test-all-versions.pr.yml

@@ -12,7 +12,7 @@ jobs:
   parse-labels:
     runs-on: ubuntu-latest
     container:
-      image: node:16
+      image: node:20
     env:
       PR_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }}
     outputs:

metapackages/auto-instrumentations-web/package.json

@@ -44,7 +44,7 @@
     "@types/webpack-env": "1.16.3",
     "assert": "2.0.0",
     "babel-loader": "8.3.0",
-    "babel-plugin-istanbul": "6.1.1",
+    "babel-plugin-istanbul": "7.0.0",
     "cross-var": "1.1.0",
     "karma": "6.4.4",
     "karma-chrome-launcher": "3.1.0",

packages/opentelemetry-id-generator-aws-xray/package.json

@@ -60,7 +60,7 @@
     "@types/webpack-env": "1.16.3",
     "assert": "2.0.0",
     "babel-loader": "8.3.0",
-    "babel-plugin-istanbul": "6.1.1",
+    "babel-plugin-istanbul": "7.0.0",
     "cross-var": "1.1.0",
     "karma": "6.4.4",
     "karma-chrome-launcher": "3.1.0",

packages/opentelemetry-test-utils/src/test-utils.ts

@@ -24,12 +24,14 @@ import {
 } from '@opentelemetry/api';
 import * as assert from 'assert';
 import { ReadableSpan } from '@opentelemetry/sdk-trace-base';
+import { MetricReader, MeterProvider } from '@opentelemetry/sdk-metrics';
 import {
   hrTimeToMilliseconds,
   hrTimeToMicroseconds,
 } from '@opentelemetry/core';
 import * as path from 'path';
 import * as fs from 'fs';
+import { InstrumentationBase } from '@opentelemetry/instrumentation';
 
 const dockerRunCmds = {
   cassandra:
@@ -179,3 +181,24 @@ export const getPackageVersion = (packageName: string) => {
   );
   return JSON.parse(fs.readFileSync(pjPath, 'utf8')).version;
 };
+
+export class TestMetricReader extends MetricReader {
+  constructor() {
+    super();
+  }
+
+  protected async onForceFlush(): Promise<void> {}
+  protected async onShutdown(): Promise<void> {}
+}
+
+export const initMeterProvider = (
+  instrumentation: InstrumentationBase
+): TestMetricReader => {
+  const metricReader = new TestMetricReader();
+  const meterProvider = new MeterProvider({
+    readers: [metricReader],
+  });
+  instrumentation.setMeterProvider(meterProvider);
+
+  return metricReader;
+};

plugins/node/instrumentation-tedious/.tav.yml

@@ -1,7 +1,26 @@
 tedious:
-  versions:
-    include: ">=1.11.0 <=17"
-    # 4.0.0 is broken: https://github.com/tediousjs/tedious/commit/4eceb48
-    exclude: "4.0.0"
-    mode: latest-majors
-  commands: npm run test
+  - versions:
+      include: ">=1.11.0 <12"
+      # 4.0.0 is broken: https://github.com/tediousjs/tedious/commit/4eceb48
+      exclude: "4.0.0"
+      mode: latest-majors
+    commands: npm run test
+  - versions:
+      include: ">=12 <17"
+      mode: latest-majors
+    node: '>=16'
+    commands: npm run test
+  - versions:
+      include: ">=17 <18"
+      mode: latest-majors
+    node: '>=18'
+    commands: npm run test
+  - versions:
+      include: ">=18 <19"
+      mode: latest-majors
+    node: '>=18'
+    commands: npm run test
+    # tedious@18 started including its own .d.ts files, and they require
+    # TypeScript v5 to use. This peerDependencies can be removed when this
+    # package updates to TypeScript v5.
+    peerDependencies: typescript@5

plugins/node/instrumentation-tedious/README.md

@@ -17,7 +17,7 @@ npm install --save @opentelemetry/instrumentation-tedious
 
 ## Supported Versions
 
-- [tedious](https://www.npmjs.com/package/tedious) `>=1.11.0 <18`
+- [tedious](https://www.npmjs.com/package/tedious) `>=1.11.0 <19`
 
 ## Usage
 

plugins/node/instrumentation-tedious/package.json

@@ -14,7 +14,7 @@
     "prewatch": "npm run precompile",
     "prepublishOnly": "npm run compile",
     "tdd": "npm run test -- --watch-extensions ts --watch",
-    "test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
+    "test": "nyc mocha --require ts-node/register 'test/**/*.test.ts'",
     "test-all-versions": "tav",
     "version:update": "node ../../../scripts/version-update.js"
   },
@@ -59,13 +59,13 @@
     "semver": "7.6.3",
     "tedious": "17.0.0",
     "test-all-versions": "6.1.0",
-    "ts-mocha": "10.0.0",
+    "ts-node": "10.9.2",
     "typescript": "4.4.4"
   },
   "dependencies": {
     "@opentelemetry/instrumentation": "^0.52.0",
     "@opentelemetry/semantic-conventions": "^1.22.0",
-    "@types/tedious": "^4.0.10"
+    "@types/tedious": "^4.0.14"
   },
   "homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/instrumentation-tedious#readme"
 }

plugins/node/instrumentation-tedious/src/instrumentation.ts

@@ -78,7 +78,7 @@ export class TediousInstrumentation extends InstrumentationBase<TediousInstrumen
     return [
       new InstrumentationNodeModuleDefinition(
         TediousInstrumentation.COMPONENT,
-        ['>=1.11.0 <18'],
+        ['>=1.11.0 <19'],
         (moduleExports: typeof tedious) => {
           const ConnectionPrototype: any = moduleExports.Connection.prototype;
           for (const method of PATCHED_METHODS) {

plugins/node/instrumentation-tedious/test/api.ts

@@ -16,14 +16,14 @@
 
 import * as assert from 'assert';
 import { promisify } from 'util';
-import type { Connection, Request, TYPES, ConnectionConfig } from 'tedious';
+import type { Connection, Request, TYPES } from 'tedious';
 
 type Method = keyof Connection & ('execSql' | 'execSqlBatch' | 'prepare');
 export type tedious = {
   Connection: typeof Connection;
   Request: typeof Request;
   TYPES: typeof TYPES;
-  ConnectionConfig: ConnectionConfig;
+  ConnectionConfig: any;
 };
 
 export const makeApi = (tedious: tedious) => {
@@ -32,7 +32,7 @@ export const makeApi = (tedious: tedious) => {
     return `[dbo].[${resource}]`;
   };
 
-  const createConnection = (config: ConnectionConfig): Promise<Connection> => {
+  const createConnection = (config: any): Promise<Connection> => {
     return new Promise((resolve, reject) => {
       const connection = new tedious.Connection(config);
 
@@ -268,11 +268,11 @@ export const makeApi = (tedious: tedious) => {
     },
     execute: (connection: Connection): Promise<number> => {
       return new Promise((resolve, reject) => {
-        const requestDoneCb = (err: any, rowCount: number) => {
+        const requestDoneCb = (err: any, rowCount?: number) => {
           if (err) {
             return reject(err);
           }
-          resolve(rowCount);
+          resolve(rowCount!);
         };
         // <2.2.0 didn't take bulkOptions
         const request =
@@ -294,7 +294,7 @@ export const makeApi = (tedious: tedious) => {
           // required in <=11.5. not supported in 14
           request.addRow({ c1: 1 });
           request.addRow({ c1: 2, c2: 'hello' });
-          return connection.execBulkLoad(request);
+          return (connection.execBulkLoad as any)(request);
         }
 
         (connection.execBulkLoad as any)(request, [

plugins/node/instrumentation-tedious/test/instrumentation.test.ts

@@ -37,7 +37,7 @@ import {
 import * as assert from 'assert';
 import { TediousInstrumentation } from '../src';
 import makeApi from './api';
-import type { Connection, ConnectionConfig } from 'tedious';
+import type { Connection } from 'tedious';
 import * as semver from 'semver';
 
 const port = Number(process.env.MSSQL_PORT) || 1433;
@@ -50,7 +50,7 @@ const instrumentation = new TediousInstrumentation();
 instrumentation.enable();
 instrumentation.disable();
 
-const config: ConnectionConfig & { userName: string; password: string } = {
+const config: any = {
   userName: user,
   password,
   server: host,

plugins/node/instrumentation-tedious/tsconfig.json

@@ -2,7 +2,12 @@
   "extends": "../../../tsconfig.base",
   "compilerOptions": {
     "rootDir": ".",
-    "outDir": "build"
+    "outDir": "build",
+    // When testing with tedious@18, skipLibCheck:true is needed to avoid
+    // checking its types. They require AggregateError that is only available
+    // with `"target": "es2022"`, which, IIUC we don't want with our regular TS
+    // v4 build for release.
+    "skipLibCheck": true
   },
   "include": [
     "src/**/*.ts",

plugins/node/instrumentation-undici/src/undici.ts

@@ -68,7 +68,6 @@ export class UndiciInstrumentation extends InstrumentationBase<UndiciInstrumenta
   private _httpClientDurationHistogram!: Histogram;
   constructor(config: UndiciInstrumentationConfig = {}) {
     super(PACKAGE_NAME, PACKAGE_VERSION, config);
-    this.setConfig(config);
   }
 
   // No need to instrument files/modules
@@ -77,26 +76,32 @@ export class UndiciInstrumentation extends InstrumentationBase<UndiciInstrumenta
   }
 
   override disable(): void {
-    if (!this.getConfig().enabled) {
-      return;
-    }
-
+    super.disable();
     this._channelSubs.forEach(sub => sub.channel.unsubscribe(sub.onMessage));
     this._channelSubs.length = 0;
-    super.disable();
-    this.setConfig({ ...this.getConfig(), enabled: false });
   }
 
   override enable(): void {
-    if (this.getConfig().enabled) {
-      return;
-    }
+    // "enabled" handling is currently a bit messy with InstrumentationBase.
+    // If constructed with `{enabled: false}`, this `.enable()` is still called,
+    // and `this.getConfig().enabled !== this.isEnabled()`, creating confusion.
+    //
+    // For now, this class will setup for instrumenting if `.enable()` is
+    // called, but use `this.getConfig().enabled` to determine if
+    // instrumentation should be generated. This covers the more likely common
+    // case of config being given a construction time, rather than later via
+    // `instance.enable()`, `.disable()`, or `.setConfig()` calls.
     super.enable();
-    this.setConfig({ ...this.getConfig(), enabled: true });
 
-    // This method is called by the `InstrumentationAbstract` constructor before
-    // ours is called. So we need to ensure the property is initalized
+    // This method is called by the super-class constructor before ours is
+    // called. So we need to ensure the property is initalized.
     this._channelSubs = this._channelSubs || [];
+
+    // Avoid to duplicate subscriptions
+    if (this._channelSubs.length > 0) {
+      return;
+    }
+
     this.subscribeToChannel(
       'undici:request:create',
       this.onRequestCreated.bind(this)
@@ -113,16 +118,6 @@ export class UndiciInstrumentation extends InstrumentationBase<UndiciInstrumenta
     this.subscribeToChannel('undici:request:error', this.onError.bind(this));
   }
 
-  override setConfig(config: UndiciInstrumentationConfig = {}): void {
-    super.setConfig(config);
-
-    if (config?.enabled) {
-      this.enable();
-    } else {
-      this.disable();
-    }
-  }
-
   protected override _updateMetricInstruments() {
     this._httpClientDurationHistogram = this.meter.createHistogram(
       'http.client.request.duration',
@@ -162,9 +157,10 @@ export class UndiciInstrumentation extends InstrumentationBase<UndiciInstrumenta
     // - ignored by config
     // - method is 'CONNECT'
     const config = this.getConfig();
+    const enabled = config.enabled !== false;
     const shouldIgnoreReq = safeExecuteInTheMiddle(
       () =>
-        !config.enabled ||
+        !enabled ||
         request.method === 'CONNECT' ||
         config.ignoreRequestHook?.(request),
       e => e && this._diag.error('caught ignoreRequestHook error: ', e),