Merge pull request #4503 from dynatrace-oss-contrib/next-main-2024-02-23

[next] merge changes from `main`

CHANGELOG.md

@@ -17,6 +17,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 * feat(sdk-metrics): allow single bucket histograms [#4456](https://github.com/open-telemetry/opentelemetry-js/pull/4456) @pichlermarc
 * feat(instrumentation): Make `init()` method public [#4418](https://github.com/open-telemetry/opentelemetry-js/pull/4418)
+* feat(context-zone-peer-dep, context-zone): support zone.js 0.13.x, 0.14.x [#4469](https://github.com/open-telemetry/opentelemetry-js/pull/4469) @pichlermarc
 
 ### :bug: (Bug Fix)
 
@@ -25,6 +26,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
   * fixes a bug where recording `NaN` on a histogram would result in the sum of bucket count values not matching the overall count
 * fix(sdk-metrics): allow single bucket histograms [#4456](https://github.com/open-telemetry/opentelemetry-js/pull/4456) @pichlermarc
   * fixes a bug where `Meter.createHistogram()` with the advice `explicitBucketBoundaries: []` would throw
+* fix(context-zone-peer-dep, context-zone):  support zone.js 0.13.x, 0.14.x [#4469](https://github.com/open-telemetry/opentelemetry-js/pull/4469) @pichlermarc
+  * fixes a bug where old versions of `zone.js` affected by <https://github.com/angular/angular/issues/53507> would be pulled in
 
 ### :books: (Refine Doc)
 

experimental/CHANGELOG.md

@@ -6,16 +6,26 @@ All notable changes to experimental packages in this project will be documented
 
 ### :boom: Breaking Change
 
+* fix(otlp-exporter-base)!: remove unload event from OTLPExporterBrowserBase [#4438](https://github.com/open-telemetry/opentelemetry-js/pull/4438) @eldavojohn
+  * Reason: The 'unload' event prevents sites from taking advantage of Google's [backward/forward cache](https://web.dev/articles/bfcache#never_use_the_unload_event) and will be [deprecated](https://developer.chrome.com/articles/deprecating-unload/).  It is now up to the consuming site to implement these shutdown events.
+  * This breaking change affects users under this scenario:
+    1. A user extends the exporter and overrides the shutdown function, and does something which is usually called by the unload listener
+    2. We remove the unload event listener
+    3. That user's overridden shutdown function no longer gets called
+
 ### :rocket: (Enhancement)
 
 * feat(instrumentation): allow LoggerProvider to be specified in Instrumentations [#4314](https://github.com/open-telemetry/opentelemetry-js/pull/4314) @hectorhdzg
-* feat(instrumentation): Make `init()` method public [#4418](https://github.com/open-telemetry/opentelemetry-js/pull/4418)
+* feat(instrumentation): add getModuleDefinitions() to InstrumentationBase [#4475](https://github.com/open-telemetry/opentelemetry-js/pull/4475) @pichlermarc
 * feat(exporter-metrics-otlp-http): add option to set the exporter aggregation preference  [#4409](https://github.com/open-telemetry/opentelemetry-js/pull/4409) @AkselAllas
 * feat(node-sdk): add spanProcessors option [#4454](https://github.com/open-telemetry/opentelemetry-js/pull/4454) @naseemkullah
 
 ### :bug: (Bug Fix)
 
 * fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber
+* fix(instrumentation): normalize paths for internal files in scoped packages [#4467](https://github.com/open-telemetry/opentelemetry-js/pull/4467) @pichlermarc
+  * Fixes a bug where, on Windows, internal files on scoped packages would not be instrumented.
+* fix(otlp-transformer): only use BigInt inside hrTimeToNanos() [#4484](https://github.com/open-telemetry/opentelemetry-js/pull/4484) @pichlermarc
 
 ### :books: (Refine Doc)
 

experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts

@@ -108,6 +108,23 @@ export abstract class InstrumentationAbstract<T = any>
     );
   }
 
+  /**
+   * @experimental
+   *
+   * Get module definitions defined by {@link init}.
+   * This can be used for experimental compile-time instrumentation.
+   *
+   * @returns an array of {@link InstrumentationModuleDefinition}
+   */
+  public getModuleDefinitions(): InstrumentationModuleDefinition<T>[] {
+    const initResult = this.init() ?? [];
+    if (!Array.isArray(initResult)) {
+      return [initResult];
+    }
+
+    return initResult;
+  }
+
   /**
    * Sets the new metric instruments with the current Meter.
    */
@@ -153,11 +170,8 @@ export abstract class InstrumentationAbstract<T = any>
   /**
    * Init method in which plugin should define _modules and patches for
    * methods.
-   * Use `enable()` if you are trying to turn on this plugin. This method
-   * will return objects to patch specific modules with the appropriate
-   * instrumentation (or not return anything).
    */
-  abstract init():
+  protected abstract init():
     | InstrumentationModuleDefinition<T>
     | InstrumentationModuleDefinition<T>[]
     | void;

experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts

@@ -204,8 +204,9 @@ export abstract class InstrumentationBase<T = any>
     }
     // internal file
     const files = module.files ?? [];
+    const normalizedName = path.normalize(name);
     const supportedFileInstrumentations = files
-      .filter(f => f.name === name)
+      .filter(f => f.name === normalizedName)
       .filter(f =>
         isSupported(f.supportedVersions, version, module.includePrerelease)
       );

experimental/packages/opentelemetry-instrumentation/test/common/Instrumentation.test.ts

@@ -19,6 +19,7 @@ import {
   Instrumentation,
   InstrumentationBase,
   InstrumentationConfig,
+  InstrumentationModuleDefinition,
 } from '../../src';
 
 import { MeterProvider } from '@opentelemetry/sdk-metrics';
@@ -132,4 +133,54 @@ describe('BaseInstrumentation', () => {
       assert.strictEqual(configuration.isActive, true);
     });
   });
+
+  describe('getModuleDefinitions', () => {
+    const moduleDefinition: InstrumentationModuleDefinition<unknown> = {
+      name: 'foo',
+      patch: moduleExports => {},
+      unpatch: moduleExports => {},
+      moduleExports: {},
+      files: [],
+      supportedVersions: ['*'],
+    };
+
+    it('should return single module definition from init() as array ', () => {
+      class TestInstrumentation2 extends TestInstrumentation {
+        override init() {
+          return moduleDefinition;
+        }
+      }
+      const instrumentation = new TestInstrumentation2();
+
+      assert.deepStrictEqual(instrumentation.getModuleDefinitions(), [
+        moduleDefinition,
+      ]);
+    });
+
+    it('should return multiple module definitions from init() as array ', () => {
+      class TestInstrumentation2 extends TestInstrumentation {
+        override init() {
+          return [moduleDefinition, moduleDefinition, moduleDefinition];
+        }
+      }
+      const instrumentation = new TestInstrumentation2();
+
+      assert.deepStrictEqual(instrumentation.getModuleDefinitions(), [
+        moduleDefinition,
+        moduleDefinition,
+        moduleDefinition,
+      ]);
+    });
+
+    it('should return void from init() as empty array ', () => {
+      class TestInstrumentation2 extends TestInstrumentation {
+        override init() {
+          return;
+        }
+      }
+      const instrumentation = new TestInstrumentation2();
+
+      assert.deepStrictEqual(instrumentation.getModuleDefinitions(), []);
+    });
+  });
 });

experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts

@@ -0,0 +1,105 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as assert from 'assert';
+
+import {
+  InstrumentationBase,
+  InstrumentationConfig,
+  InstrumentationNodeModuleDefinition,
+  InstrumentationNodeModuleFile,
+} from '../../src';
+
+class TestInstrumentationSimple extends InstrumentationBase {
+  constructor(config: InstrumentationConfig) {
+    super('test-scoped-package-instrumentation', '0.0.1', config);
+  }
+  init() {
+    return new InstrumentationNodeModuleDefinition(
+      '@opentelemetry/scoped-test-module',
+      ['*'],
+      moduleExports => {
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
+        moduleExports.propertyOnMainModule = 'modified string in main module';
+        return moduleExports;
+      },
+      moduleExports => {
+        return moduleExports;
+      },
+      [
+        new InstrumentationNodeModuleFile(
+          '@opentelemetry/scoped-test-module/src/internal.js',
+          ['*'],
+          moduleExports => {
+            // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+            // @ts-ignore no types
+            moduleExports.testString = 'modified internal string';
+            return moduleExports;
+          },
+          moduleExports => {
+            return moduleExports;
+          }
+        ),
+      ]
+    );
+  }
+}
+
+describe('instrumenting a scoped package', function () {
+  /**
+   * On Windows, instrumentation would fail on internal files of scoped packages.
+   * The reason: the path would include a '/' separator in the package name:
+   * - actual:   @opentelemetry/scoped-test-module\src\internal.js
+   * - expected: @opentelemetry\scoped-test-module\src\internal.js
+   * This resulted in internal files of scoped packages not being instrumented.
+   *
+   * See https://github.com/open-telemetry/opentelemetry-js/issues/4436
+   */
+  it('should patch internal file and main module', function () {
+    const instrumentation = new TestInstrumentationSimple({
+      enabled: false,
+    });
+    instrumentation.enable();
+
+    const { getString } = require('@opentelemetry/scoped-test-module');
+
+    assert.strictEqual(getString(), 'from index.js: modified internal string');
+  });
+
+  /**
+   * Normalizing everything passed to onRequire() from RequireInTheMiddleSingleton would cause main modules from a
+   * scoped package not to be instrumented.
+   * The reason: we'd check:
+   * '@opentelemetry\scoped-test-module' === '@opentelemetry/scoped-test-module'
+   *
+   * then determine that since this evaluates to false, this is not the main module, and we'd never instrument it.
+   *
+   * Therefore, the fix to the above test (internal files) is not to normalize everything passed to onRequire()
+   */
+  it('should patch main module', function () {
+    const instrumentation = new TestInstrumentationSimple({
+      enabled: false,
+    });
+    instrumentation.enable();
+
+    const {
+      propertyOnMainModule,
+    } = require('@opentelemetry/scoped-test-module');
+
+    assert.strictEqual(propertyOnMainModule, 'modified string in main module');
+  });
+});

experimental/packages/otlp-exporter-base/src/platform/browser/OTLPExporterBrowserBase.ts

@@ -20,7 +20,7 @@ import * as otlpTypes from '../../types';
 import { parseHeaders } from '../../util';
 import { sendWithBeacon, sendWithXhr } from './util';
 import { diag } from '@opentelemetry/api';
-import { getEnv, baggageUtils, _globalThis } from '@opentelemetry/core';
+import { getEnv, baggageUtils } from '@opentelemetry/core';
 
 /**
  * Collector Metric Exporter abstract base class
@@ -52,13 +52,9 @@ export abstract class OTLPExporterBrowserBase<
     }
   }
 
-  onInit(): void {
-    _globalThis.addEventListener('unload', this.shutdown);
-  }
+  onInit(): void {}
 
-  onShutdown(): void {
-    _globalThis.removeEventListener('unload', this.shutdown);
-  }
+  onShutdown(): void {}
 
   send(
     items: ExportItem[],

experimental/packages/otlp-transformer/src/common/index.ts

@@ -18,9 +18,8 @@ import type { OtlpEncodingOptions, Fixed64, LongBits } from './types';
 import { HrTime } from '@opentelemetry/api';
 import { hexToBinary, hrTimeToNanoseconds } from '@opentelemetry/core';
 
-const NANOSECONDS = BigInt(1_000_000_000);
-
 export function hrTimeToNanos(hrTime: HrTime): bigint {
+  const NANOSECONDS = BigInt(1_000_000_000);
   return BigInt(hrTime[0]) * NANOSECONDS + BigInt(hrTime[1]);
 }