Skip to content

Commit 918c1bd

Browse files
nsavoireszegedi
andauthored
Add support for node 22 (#169)
* Test on node 22 * Test improvements * Update nan to 2.19.0 for Node 22 * Fix wall profiler test for node 22 on MacOS With node 22, many deopt events are generated by `setContext` call in `fn0`. On MacOS, `v8::TimeTicks::Now` has a resolution of ~42us because `mach_absolute_time` ticks (a tick is ~42ns) conversion to microseconds is done in such a way that drops the 3 least significant digits. This two facts lead to samples having identical timestamps, and incorrectly matched contexts. Workaround here just ensures that after deopt event caused by `setContext`, no sample in `fn1` is immediately taken. * Fix eslint warning --------- Co-authored-by: Attila Szegedi <attila.szegedi@datadoghq.com>
1 parent 5fc51cb commit 918c1bd

File tree

5 files changed

+43
-18
lines changed

5 files changed

+43
-18
lines changed

.github/workflows/build.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ jobs:
1010
asan:
1111
strategy:
1212
matrix:
13-
version: [14, 16, 18, 20, 21]
13+
version: [14, 16, 18, 20, 22]
1414
runs-on: ubuntu-latest
1515
steps:
1616
- uses: actions/checkout@v3
@@ -23,7 +23,7 @@ jobs:
2323
valgrind:
2424
strategy:
2525
matrix:
26-
version: [14, 16, 18, 20, 21]
26+
version: [14, 16, 18, 20, 22]
2727
runs-on: ubuntu-latest
2828
steps:
2929
- uses: actions/checkout@v3

package-lock.json

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
"gts": "^4.0.1",
5656
"js-green-licenses": "^4.0.0",
5757
"mocha": "^10.2.0",
58-
"nan": "^2.18.0",
58+
"nan": "^2.19.0",
5959
"nyc": "^15.1.0",
6060
"sinon": "^15.2.0",
6161
"source-map-support": "^0.5.21",

ts/test/profiles-for-tests.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1229,8 +1229,8 @@ export const labelEncodingProfile = {
12291229

12301230
const {hasOwnProperty} = Object.prototype;
12311231

1232-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
12331232
export function getAndVerifyPresence(
1233+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
12341234
list: any[],
12351235
id: number,
12361236
zeroIndex = false

ts/test/test-time-profiler.ts

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ describe('Time Profiler', () => {
141141
labels[key] = value;
142142
if (
143143
enableEndPoint &&
144-
key === 'local root span id' &&
144+
key === rootSpanIdLabel &&
145145
value === rootSpanId
146146
) {
147147
labels[endPointLabel] = endPoint;
@@ -164,6 +164,17 @@ describe('Time Profiler', () => {
164164
const start = hrtime.bigint();
165165
while (hrtime.bigint() - start < intervalNanos);
166166
time.setContext(undefined);
167+
// With node 22, many deopt events are generated by `setContext` call above.
168+
// On MacOS, `v8::TimeTicks::Now` has a resolution of ~42us because
169+
// `mach_absolute_time` ticks (a tick is ~42ns) conversion to microseconds
170+
// is done in such a way that drops the 3 least significant digits
171+
// (https://github.com/nodejs/node/blob/v22.x/deps/v8/src/base/platform/time.cc#L745-L746).
172+
// This two facts lead to samples having identical timestamps, and
173+
// incorrectly matched contexts.
174+
// Workaround here just ensures that after deopt event caused by `setContext`,
175+
// no sample in `fn1` is immediately taken.
176+
const start2 = hrtime.bigint();
177+
while (hrtime.bigint() - start2 < intervalNanos);
167178
}
168179

169180
function fn1() {
@@ -192,11 +203,12 @@ describe('Time Profiler', () => {
192203
function validateProfile(profile: Profile) {
193204
// Get string table indices for strings we're interested in
194205
const stringTable = profile.stringTable;
195-
const [loopIdx, fn0Idx, fn1Idx, fn2Idx] = [
206+
const [loopIdx, fn0Idx, fn1Idx, fn2Idx, hrtimeBigIntIdx] = [
196207
'loop',
197208
'fn0',
198209
'fn1',
199210
'fn2',
211+
'hrtimeBigInt',
200212
].map(x => stringTable.dedup(x));
201213

202214
function getString(n: number | bigint): string {
@@ -219,7 +231,9 @@ describe('Time Profiler', () => {
219231
}
220232

221233
function labelStr(label: Label) {
222-
return label ? stringTable.strings[idx(label.str) + 1] : 'undefined';
234+
return label
235+
? `${getString(label.key)}=${getString(label.str)}`
236+
: 'undefined';
223237
}
224238

225239
function getLabels(labels: Label[]) {
@@ -234,19 +248,25 @@ describe('Time Profiler', () => {
234248
let fn1ObservedWithLabel1 = false;
235249
let fn2ObservedWithoutLabels = false;
236250
profile.sample.forEach(sample => {
237-
const locIdx = idx(sample.locationId[0]);
238-
const loc = profile.location[locIdx];
239-
const fnIdx = idx(loc.line[0].functionId);
240-
const fn = profile.function[fnIdx];
241-
const fnName = fn.name;
251+
let fnName;
252+
for (const locationId of sample.locationId) {
253+
const locIdx = idx(locationId);
254+
const loc = profile.location[locIdx];
255+
const fnIdx = idx(loc.line[0].functionId);
256+
const fn = profile.function[fnIdx];
257+
fnName = fn.name;
258+
if (fnName !== hrtimeBigIntIdx) {
259+
break;
260+
}
261+
}
242262
const labels = sample.label;
243263

244264
switch (fnName) {
245265
case loopIdx:
246266
if (enableEndPoint) {
247267
assert(
248268
labels.length < 4,
249-
'loop can have at most one label and one endpoint'
269+
'loop can have at most two labels and one endpoint'
250270
);
251271
labels.forEach(label => {
252272
assert(
@@ -271,7 +291,12 @@ describe('Time Profiler', () => {
271291

272292
break;
273293
case fn0Idx:
274-
assert(labels.length < 2, 'fn0 can have at most one label');
294+
assert(
295+
labels.length < 2,
296+
`fn0 can have at most one label, instead got: ${labels.map(
297+
labelStr
298+
)}`
299+
);
275300
labels.forEach(label => {
276301
if (labelIs(label, 'label', 'value0')) {
277302
fn0ObservedWithLabel0 = true;

0 commit comments

Comments
 (0)