fix(instrumentation-runtime-node): use absolute results in eventLoopUtilization computation (#3118)

Author: mveroone

packages/instrumentation-runtime-node/src/metrics/eventLoopUtilizationCollector.ts

@@ -22,7 +22,9 @@ import { METRIC_NODEJS_EVENTLOOP_UTILIZATION } from '../semconv';
 const { eventLoopUtilization: eventLoopUtilizationCollector } = performance;
 
 export class EventLoopUtilizationCollector extends BaseCollector {
-  private _lastValue?: EventLoopUtilization;
+  // Value needs to be initialized the first time otherwise the first measurement would always be 1
+  // See https://github.com/open-telemetry/opentelemetry-js-contrib/pull/3118#issuecomment-3429737955
+  private _lastValue: EventLoopUtilization = eventLoopUtilizationCollector();
 
   public updateMetricInstruments(meter: Meter): void {
     meter
@@ -33,9 +35,13 @@ export class EventLoopUtilizationCollector extends BaseCollector {
       .addCallback(async observableResult => {
         if (!this._config.enabled) return;
 
-        const elu = eventLoopUtilizationCollector(this._lastValue);
-        observableResult.observe(elu.utilization);
-        this._lastValue = elu;
+        const currentELU = eventLoopUtilizationCollector();
+        const deltaELU = eventLoopUtilizationCollector(
+          currentELU,
+          this._lastValue
+        );
+        this._lastValue = currentELU;
+        observableResult.observe(deltaELU.utilization);
       });
   }
 

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

@@ -98,4 +98,87 @@ describe('nodejs.eventloop.utilization', function () {
       'expected one data point'
     );
   });
+
+  it('should correctly calculate utilization deltas across multiple measurements', async function () {
+    // This test ensures the bug where delta of deltas was observed instead of deltas of absolute values
+    // does not regress. See https://github.com/open-telemetry/opentelemetry-js-contrib/pull/3118
+    // This bug would surface on the third callback invocation.
+
+    const instrumentation = new RuntimeNodeInstrumentation({});
+    instrumentation.setMeterProvider(meterProvider);
+
+    // Helper function to create blocking work that results in high utilization
+    const createBlockingWork = (durationMs: number) => {
+      const start = Date.now();
+      while (Date.now() - start < durationMs) {
+        // Busy wait to block the event loop
+      }
+    };
+
+    // Helper function to collect metrics and extract utilization value
+    const collectUtilization = async (): Promise<number> => {
+      const { resourceMetrics } = await metricReader.collect();
+      const scopeMetrics = resourceMetrics.scopeMetrics;
+      const utilizationMetric = scopeMetrics[0].metrics.find(
+      x => x.descriptor.name === METRIC_NODEJS_EVENTLOOP_UTILIZATION
+    );
+
+      assert.notEqual(utilizationMetric, undefined, 'metric not found');
+      assert.strictEqual(
+        utilizationMetric!.dataPoints.length,
+        1,
+        'expected one data point'
+      );
+
+      return utilizationMetric!.dataPoints[0].value as number;
+    };
+
+    // Wait for some time to establish baseline utilization
+    await new Promise(resolve => setTimeout(resolve, 200));
+
+    // First collection
+    const firstUtilization = await collectUtilization();
+    assert.notStrictEqual(
+      firstUtilization,
+      1,
+      'Expected utilization in first measurement to be not 1'
+    );
+
+    // Second measurement: Create blocking work and measure
+    createBlockingWork(50);
+    const secondUtilization = await collectUtilization();
+    assert.strictEqual(
+      secondUtilization,
+      1,
+      'Expected utilization in second measurement to be 1'
+    );
+
+    // Third measurement: Create blocking work again and measure
+    // This is where the bug would manifest - if we were observing delta of deltas,
+    // this measurement would not be 1
+    createBlockingWork(50);
+    const thirdUtilization = await collectUtilization();
+    assert.strictEqual(
+      thirdUtilization,
+      1,
+      'Expected utilization in third measurement to be 1'
+    );
+
+    // Fourth measurement (should be the same as the third measurement, just a sanity check)
+    createBlockingWork(50);
+    const fourthUtilization = await collectUtilization();
+    assert.strictEqual(
+      fourthUtilization,
+      1,
+      'Expected utilization in fourth measurement to be 1'
+    );
+
+    // Fifth measurement: Do some NON-blocking work (sanity check, should be low)
+    await new Promise(resolve => setTimeout(resolve, 50));
+    const fifthUtilization = await collectUtilization();
+    assert.ok(
+      fifthUtilization < 1,
+      `Expected utilization in fifth measurement to be less than 1, but got ${fifthUtilization}`
+    );
+  });
 });