Merge branch 'main' into feature/config-links-amqp-plugin

.github/workflows/unit-test.yml

@@ -138,7 +138,7 @@ jobs:
         run: npm run test:ci:changed
       - name: Report Coverage
         if: ${{ matrix.code-coverage && !cancelled()}}
-        uses: codecov/codecov-action@v3
+        uses: codecov/codecov-action@v4
         with:
           verbose: true
 
@@ -169,6 +169,6 @@ jobs:
       - name: Unit tests
         run: npm run test:browser
       - name: Report Coverage
-        uses: codecov/codecov-action@v3
+        uses: codecov/codecov-action@v4
         with:
           verbose: true

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

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 import { EventLoopUtilization, performance } from 'node:perf_hooks';
-import { clearInterval, setInterval } from 'node:timers';
 const { eventLoopUtilization } = performance;
 
 import { InstrumentationBase } from '@opentelemetry/instrumentation';
@@ -79,6 +78,9 @@ export class RuntimeNodeInstrumentation extends InstrumentationBase {
       (this._config as RuntimeNodeInstrumentationConfig)
         .eventLoopUtilizationMeasurementInterval
     );
+
+    // unref so that it does not keep the process running if disable() is never called
+    this._interval?.unref();
   }
 
   override disable() {

plugins/node/instrumentation-runtime-node/test/event_loop_utilization.test.ts

@@ -14,85 +14,110 @@
  * limitations under the License.
  */
 import {
-  AggregationTemporality,
-  InMemoryMetricExporter,
   MeterProvider,
   DataPointType,
-  PeriodicExportingMetricReader,
+  MetricReader,
 } from '@opentelemetry/sdk-metrics';
 
 import { RuntimeNodeInstrumentation } from '../src';
 import * as assert from 'assert';
 
 const MEASUREMENT_INTERVAL = 10;
 
-const metricExporter = new InMemoryMetricExporter(AggregationTemporality.DELTA);
-const metricReader = new PeriodicExportingMetricReader({
-  exporter: metricExporter,
-  exportIntervalMillis: MEASUREMENT_INTERVAL * 2,
-});
-const meterProvider = new MeterProvider();
-meterProvider.addMetricReader(metricReader);
+class TestMetricReader extends MetricReader {
+  constructor() {
+    super();
+  }
 
-const instrumentation = new RuntimeNodeInstrumentation({
-  eventLoopUtilizationMeasurementInterval: MEASUREMENT_INTERVAL,
-});
+  protected async onForceFlush(): Promise<void> {}
 
-instrumentation.setMeterProvider(meterProvider);
+  protected async onShutdown(): Promise<void> {}
+}
 
-describe('nodejs.event_loop.utilization', () => {
-  beforeEach(async () => {
-    instrumentation.disable(); // Stops future metrics from being collected
-    metricExporter.reset(); // Remove existing collected metrics
-  });
+describe('nodejs.event_loop.utilization', function () {
+  let metricReader: TestMetricReader;
+  let meterProvider: MeterProvider;
 
-  after(() => {
-    instrumentation.disable();
-    meterProvider.shutdown();
+  beforeEach(() => {
+    metricReader = new TestMetricReader();
+    meterProvider = new MeterProvider();
+    meterProvider.addMetricReader(metricReader);
   });
 
-  it('should stop exporting metrics when disabled', async () => {
-    // Wait for the ELU data to be collected and exported
-    // MEASUREMENT_INTERVAL * 2 is the export interval, plus MEASUREMENT_INTERVAL as a buffer
-    await new Promise(resolve => setTimeout(resolve, MEASUREMENT_INTERVAL * 3));
-    // Retrieve exported metrics
-    const resourceMetrics = metricExporter.getMetrics();
-    const scopeMetrics =
-      resourceMetrics[resourceMetrics.length - 1].scopeMetrics;
+  it('should not export before being enabled', async function () {
+    // arrange
+    const instrumentation = new RuntimeNodeInstrumentation({
+      eventLoopUtilizationMeasurementInterval: MEASUREMENT_INTERVAL,
+      enabled: false,
+    });
+    instrumentation.setMeterProvider(meterProvider);
+
+    // act
+    await new Promise(resolve => setTimeout(resolve, MEASUREMENT_INTERVAL * 5));
+    const { resourceMetrics, errors } = await metricReader.collect();
+
+    // assert
+    assert.deepEqual(errors, []);
+    const scopeMetrics = resourceMetrics.scopeMetrics;
     assert.strictEqual(scopeMetrics.length, 0);
   });
 
-  it('should not export immediately after enable', async () => {
-    instrumentation.enable();
-    assert.deepEqual(metricExporter.getMetrics(), []);
+  it('should not record result when collecting immediately with custom config', async function () {
+    const instrumentation = new RuntimeNodeInstrumentation({
+      eventLoopUtilizationMeasurementInterval: MEASUREMENT_INTERVAL,
+    });
+    instrumentation.setMeterProvider(meterProvider);
+
+    assert.deepEqual(
+      (await metricReader.collect()).resourceMetrics.scopeMetrics,
+      []
+    );
   });
 
-  it('can use default eventLoopUtilizationMeasurementInterval', async () => {
-    // Repeat of 'should not export immediately after enable' but with defaults
-    const localInstrumentation = new RuntimeNodeInstrumentation();
-    localInstrumentation.setMeterProvider(meterProvider);
-    localInstrumentation.disable();
-    metricExporter.reset();
-    localInstrumentation.enable();
-    assert.deepEqual(metricExporter.getMetrics(), []);
-    localInstrumentation.disable();
+  it('should not record result when collecting immediately with default config', async function () {
+    const instrumentation = new RuntimeNodeInstrumentation();
+    instrumentation.setMeterProvider(meterProvider);
+
+    assert.deepEqual(
+      (await metricReader.collect()).resourceMetrics.scopeMetrics,
+      []
+    );
   });
 
-  it('should export event loop utilization metrics after eventLoopUtilizationMeasurementInterval', async () => {
-    instrumentation.enable();
-    // Wait for the ELU data to be collected and exported
-    // MEASUREMENT_INTERVAL * 2 is the export interval, plus MEASUREMENT_INTERVAL as a buffer
-    await new Promise(resolve => setTimeout(resolve, MEASUREMENT_INTERVAL * 3));
-    const resourceMetrics = metricExporter.getMetrics();
-    const scopeMetrics =
-      resourceMetrics[resourceMetrics.length - 1].scopeMetrics;
+  it('should write event loop utilization metrics after eventLoopUtilizationMeasurementInterval', async function () {
+    // arrange
+    const instrumentation = new RuntimeNodeInstrumentation({
+      eventLoopUtilizationMeasurementInterval: MEASUREMENT_INTERVAL,
+    });
+    instrumentation.setMeterProvider(meterProvider);
+
+    // act
+    await new Promise(resolve => setTimeout(resolve, MEASUREMENT_INTERVAL * 5));
+    const { resourceMetrics, errors } = await metricReader.collect();
+
+    // assert
+    assert.deepEqual(
+      errors,
+      [],
+      'expected no errors from the callback during collection'
+    );
+    const scopeMetrics = resourceMetrics.scopeMetrics;
+    assert.strictEqual(
+      scopeMetrics.length,
+      1,
+      'expected one scope (one meter created by instrumentation)'
+    );
     const metrics = scopeMetrics[0].metrics;
-    assert.strictEqual(metrics.length, 1, 'one ScopeMetrics');
-    assert.strictEqual(metrics[0].dataPointType, DataPointType.GAUGE, 'gauge');
-    assert.strictEqual(metrics[0].dataPoints.length, 1, 'one data point');
-    const val = metrics[0].dataPoints[0].value;
-    assert.strictEqual(val > 0, true, `val (${val}) > 0`);
-    assert.strictEqual(val < 1, true, `val (${val}) < 1`);
+    assert.strictEqual(
+      metrics.length,
+      1,
+      'expected one metric (one metric created by instrumentation)'
+    );
+    assert.strictEqual(
+      metrics[0].dataPointType,
+      DataPointType.GAUGE,
+      'expected gauge'
+    );
     assert.strictEqual(
       metrics[0].descriptor.name,
       'nodejs.event_loop.utilization',
@@ -102,6 +127,18 @@ describe('nodejs.event_loop.utilization', () => {
       metrics[0].descriptor.description,
       'Event loop utilization'
     );
-    assert.strictEqual(metrics[0].descriptor.unit, '1');
+    assert.strictEqual(
+      metrics[0].descriptor.unit,
+      '1',
+      'expected default unit'
+    );
+    assert.strictEqual(
+      metrics[0].dataPoints.length,
+      1,
+      'expected one data point'
+    );
+    const val = metrics[0].dataPoints[0].value;
+    assert.strictEqual(val > 0, true, `val (${val}) > 0`);
+    assert.strictEqual(val <= 1, true, `val (${val}) <= 1`);
   });
 });

plugins/node/opentelemetry-instrumentation-aws-sdk/src/services/sqs.ts

@@ -110,13 +110,16 @@ export class SqsServiceExtension implements ServiceExtension {
 
       case 'SendMessageBatch':
         {
-          request.commandInput?.Entries?.forEach(
-            (messageParams: SQS.SendMessageBatchRequestEntry) => {
-              messageParams.MessageAttributes = injectPropagationContext(
-                messageParams.MessageAttributes ?? {}
-              );
-            }
-          );
+          const entries = request.commandInput?.Entries;
+          if (Array.isArray(entries)) {
+            entries.forEach(
+              (messageParams: SQS.SendMessageBatchRequestEntry) => {
+                messageParams.MessageAttributes = injectPropagationContext(
+                  messageParams.MessageAttributes ?? {}
+                );
+              }
+            );
+          }
         }
         break;
     }

plugins/node/opentelemetry-instrumentation-aws-sdk/test/sqs.test.ts

@@ -23,6 +23,7 @@ const instrumentation = registerInstrumentationTesting(
 );
 import * as AWS from 'aws-sdk';
 import { AWSError } from 'aws-sdk';
+import type { SQS } from 'aws-sdk';
 
 import {
   MessagingDestinationKindValues,
@@ -494,6 +495,29 @@ describe('SQS', () => {
       expect(processSpans[0].status.code).toStrictEqual(SpanStatusCode.UNSET);
       expect(processSpans[1].status.code).toStrictEqual(SpanStatusCode.UNSET);
     });
+
+    it('bogus sendMessageBatch input should not crash', async () => {
+      const region = 'us-east-1';
+      const sqs = new AWS.SQS();
+      sqs.config.update({ region });
+
+      const QueueName = 'unittest';
+      const params = {
+        QueueUrl: `queue/url/for/${QueueName}`,
+        Entries: { Key1: { MessageBody: 'This is the first message' } },
+      };
+      await sqs
+        .sendMessageBatch(params as unknown as SQS.SendMessageBatchRequest)
+        .promise();
+
+      const spans = getTestSpans();
+      expect(spans.length).toBe(1);
+
+      // Spot check a single attribute as a sanity check.
+      expect(spans[0].attributes[SemanticAttributes.RPC_METHOD]).toEqual(
+        'SendMessageBatch'
+      );
+    });
   });
 
   describe('extract payload', () => {

plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

@@ -197,7 +197,9 @@ export class ExpressInstrumentation extends InstrumentationBase<
         storeLayerPath(req, layerPath);
         const route = (req[_LAYERS_STORE_PROPERTY] as string[])
           .filter(path => path !== '/' && path !== '/*')
-          .join('');
+          .join('')
+          // remove duplicate slashes to normalize route
+          .replace(/\/{2,}/g, '/');
 
         const attributes: Attributes = {
           [SemanticAttributes.HTTP_ROUTE]: route.length > 0 ? route : '/',

plugins/node/opentelemetry-instrumentation-express/test/express.test.ts

@@ -455,6 +455,39 @@ describe('ExpressInstrumentation', () => {
         }
       );
     });
+
+    it('should ignore double slashes in routes', async () => {
+      const rootSpan = tracer.startSpan('rootSpan');
+      let rpcMetadata: RPCMetadata | undefined;
+      const httpServer = await serverWithMiddleware(tracer, rootSpan, app => {
+        app.use(express.json());
+        app.use((req, res, next) => {
+          rpcMetadata = getRPCMetadata(context.active());
+          next();
+        });
+      });
+      server = httpServer.server;
+      port = httpServer.port;
+      assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
+      await context.with(
+        trace.setSpan(context.active(), rootSpan),
+        async () => {
+          const response = await httpRequest.get(
+            `http://localhost:${port}/double-slashes/foo`
+          );
+          assert.strictEqual(response, 'foo');
+          rootSpan.end();
+          const requestHandlerSpan = memoryExporter
+            .getFinishedSpans()
+            .find(span => span.name.includes('request handler'));
+          assert.strictEqual(
+            requestHandlerSpan?.attributes[SemanticAttributes.HTTP_ROUTE],
+            '/double-slashes/:id'
+          );
+          assert.strictEqual(rpcMetadata?.route, '/double-slashes/:id');
+        }
+      );
+    });
   });
 
   describe('Disabling plugin', () => {

plugins/node/opentelemetry-instrumentation-express/test/utils.ts

@@ -66,6 +66,7 @@ export async function serverWithMiddleware(
 
   const router = express.Router();
   app.use('/toto', router);
+  app.use('/double-slashes/', router);
   router.get('/:id', (req, res) => {
     setImmediate(() => {
       res.status(200).end(req.params.id);

plugins/node/opentelemetry-instrumentation-mongodb/.tav.yml

@@ -6,9 +6,6 @@ mongodb:
       commands: npm run test-v4
     - versions: ">=5 <6"
       commands: npm run test-v5-v6
-    - versions: ">=6 <7"
+    - versions: ">=6 <6.4"
       node: '>=15.0.0'
       commands: npm run test-v5-v6
-
-  # Fix missing `contrib-test-utils` package
-  pretest: npm run --prefix ../../../ lerna:link

plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts

@@ -97,31 +97,31 @@ export class MongoDBInstrumentation extends InstrumentationBase {
       ),
       new InstrumentationNodeModuleDefinition<any>(
         'mongodb',
-        ['4.*', '5.*', '6.*'],
+        ['4.*', '5.*', '>=6 <6.4'],
         undefined,
         undefined,
         [
           new InstrumentationNodeModuleFile<V4Connection>(
             'mongodb/lib/cmap/connection.js',
-            ['4.*', '5.*', '6.*'],
+            ['4.*', '5.*', '>=6 <6.4'],
             v4PatchConnection,
             v4UnpatchConnection
           ),
           new InstrumentationNodeModuleFile<V4ConnectionPool>(
             'mongodb/lib/cmap/connection_pool.js',
-            ['4.*', '5.*', '6.*'],
+            ['4.*', '5.*', '>=6 <6.4'],
             v4PatchConnectionPool,
             v4UnpatchConnectionPool
           ),
           new InstrumentationNodeModuleFile<V4Connect>(
             'mongodb/lib/cmap/connect.js',
-            ['4.*', '5.*', '6.*'],
+            ['4.*', '5.*', '>=6 <6.4'],
             v4PatchConnect,
             v4UnpatchConnect
           ),
           new InstrumentationNodeModuleFile<V4Session>(
             'mongodb/lib/sessions.js',
-            ['4.*', '5.*', '6.*'],
+            ['4.*', '5.*', '>=6 <6.4'],
             v4PatchSessions,
             v4UnpatchSessions
           ),