Skip to content

Commit c4763d9

Browse files
committed
Improve solution for first iteration and add tests
Co-authored with @d4nyll
1 parent a747c1c commit c4763d9

File tree

2 files changed

+92
-2
lines changed

2 files changed

+92
-2
lines changed

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ import { METRIC_NODEJS_EVENTLOOP_UTILIZATION } from '../semconv';
2222
const { eventLoopUtilization: eventLoopUtilizationCollector } = performance;
2323

2424
export class EventLoopUtilizationCollector extends BaseCollector {
25-
private _lastValue?: EventLoopUtilization;
25+
// Value needs to be initialized the first time otherwise the first measurement would be incorrectly small
26+
// See https://github.com/open-telemetry/opentelemetry-js-contrib/pull/3118#issuecomment-3429737955
27+
private _lastValue: EventLoopUtilization = eventLoopUtilizationCollector();
2628

2729
public updateMetricInstruments(meter: Meter): void {
2830
meter
@@ -34,7 +36,10 @@ export class EventLoopUtilizationCollector extends BaseCollector {
3436
if (!this._config.enabled) return;
3537

3638
const currentELU = eventLoopUtilizationCollector();
37-
const deltaELU = eventLoopUtilizationCollector(currentELU, this._lastValue);
39+
const deltaELU = eventLoopUtilizationCollector(
40+
currentELU,
41+
this._lastValue
42+
);
3843
this._lastValue = currentELU;
3944
observableResult.observe(deltaELU.utilization);
4045
});

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

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,89 @@ describe('nodejs.eventloop.utilization', function () {
9898
'expected one data point'
9999
);
100100
});
101+
102+
it('should correctly calculate utilization deltas across multiple measurements', async function () {
103+
// This test ensures the bug where delta of deltas was observed instead of deltas of absolute values
104+
// does not regress. See https://github.com/open-telemetry/opentelemetry-js-contrib/pull/3118
105+
// This bug would surface on the third callback invocation.
106+
107+
const instrumentation = new RuntimeNodeInstrumentation({});
108+
instrumentation.setMeterProvider(meterProvider);
109+
110+
// Helper function to create blocking work that results in high utilization
111+
const createBlockingWork = (durationMs: number) => {
112+
const start = Date.now();
113+
while (Date.now() - start < durationMs) {
114+
// Busy wait to block the event loop
115+
}
116+
};
117+
118+
// Helper function to collect metrics and extract utilization value
119+
const collectUtilization = async (): Promise<number> => {
120+
const { resourceMetrics } = await metricReader.collect();
121+
const scopeMetrics = resourceMetrics.scopeMetrics;
122+
const utilizationMetric = scopeMetrics[0].metrics.find(
123+
x =>
124+
x.descriptor.name ===
125+
`${ConventionalNamePrefix.NodeJs}.${ATTR_NODEJS_EVENT_LOOP_UTILIZATION}`
126+
);
127+
128+
assert.notEqual(utilizationMetric, undefined, 'metric not found');
129+
assert.strictEqual(
130+
utilizationMetric!.dataPoints.length,
131+
1,
132+
'expected one data point'
133+
);
134+
135+
return utilizationMetric!.dataPoints[0].value as number;
136+
};
137+
138+
// Wait for some time to establish baseline utilization
139+
await new Promise(resolve => setTimeout(resolve, 200));
140+
141+
// First collection
142+
const firstUtilization = await collectUtilization();
143+
assert.notStrictEqual(
144+
firstUtilization,
145+
1,
146+
'Expected utilization in first measurement to be not 1'
147+
);
148+
149+
// Second measurement: Create blocking work and measure
150+
createBlockingWork(50);
151+
const secondUtilization = await collectUtilization();
152+
assert.strictEqual(
153+
secondUtilization,
154+
1,
155+
'Expected utilization in second measurement to be 1'
156+
);
157+
158+
// Third measurement: Create blocking work again and measure
159+
// This is where the bug would manifest - if we were observing delta of deltas,
160+
// this measurement would not be 1
161+
createBlockingWork(50);
162+
const thirdUtilization = await collectUtilization();
163+
assert.strictEqual(
164+
thirdUtilization,
165+
1,
166+
'Expected utilization in third measurement to be 1'
167+
);
168+
169+
// Fourth measurement (should be the same as the third measurement, just a sanity check)
170+
createBlockingWork(50);
171+
const fourthUtilization = await collectUtilization();
172+
assert.strictEqual(
173+
fourthUtilization,
174+
1,
175+
'Expected utilization in fourth measurement to be 1'
176+
);
177+
178+
// Fifth measurement: Do some NON-blocking work (sanity check, should be low)
179+
await new Promise(resolve => setTimeout(resolve, 50));
180+
const fifthUtilization = await collectUtilization();
181+
assert.ok(
182+
fifthUtilization < 0.1,
183+
'Expected utilization in fifth measurement to be less than 0.1'
184+
);
185+
});
101186
});

0 commit comments

Comments
 (0)