DeprecationWarning when running Node 16

[oskarolsson-jimdo]

Hi!

It seems starting with Node 16 there's a deprecating warning stemming from prom-client.

We're using a simple setup of prom.collectDefaultMetrics();

With node@16.0.0, npm@7.5.6 and using --trace-deprecation to run our server, we get the following (replaced irrelevant paths with [...]):

(node:22429) [DEP0152] DeprecationWarning: Custom PerformanceEntry accessors are deprecated. Please use the detail property.
    at PerformanceObserver.<anonymous> (/[...]/node_modules/prom-client/lib/metrics/gc.js:47:38)
    at PerformanceObserver.[kDispatch] (node:internal/perf/observe:261:34)
    at Immediate.<anonymous> (node:internal/perf/observe:94:25)
    at processImmediate (node:internal/timers:464:21)
/[...]/node/16.0.0/bin/node[22429]: ../src/env-inl.h:1052:void node::Environment::AddCleanupHook(node::Environment::CleanupCallback, void*): Assertion `(insertion_info.second) == (true)' failed.
 1: 0xb12b00 node::Abort() [/[...]/node/16.0.0/bin/node]
 2: 0xb12b7e  [/[...]/node/16.0.0/bin/node]
 3: 0xb7a58c  [/[...]/node/16.0.0/bin/node]
 4: 0xd5f70b  [/[...]/node/16.0.0/bin/node]
 5: 0xd60bac  [/[...]/node/16.0.0/bin/node]
 6: 0xd61226 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/[...]/node/16.0.0/bin/node]
 7: 0x160c579  [/[...]/node/16.0.0/bin/node]

Looking at the changelog for Node 16, it seems this is most likely related to this change that overhauled perf_hooks.

[SimenB]

The reported line looks innocent to me:

prom-client/lib/metrics/gc.js

Line 47 in 96f7495

Object.assign({ kind: kinds[entry.kind] }, labels),

Not sure what's up. Maybe the detection in node is thrown off by the merging?

/cc @sam-github if you know

[oskarolsson-jimdo]

The "accessor" that is referred to in the DeprecationWarning most likely refers to entry.kind on that line.
According to the doc update here performanceEntry.kind was deprecated in favor of using the detail property.

[SimenB]

Aha! I didn't think kind was "custom" 🙂 Wanna send a PR fixing it? We need to make sure we still work on node 10, not sure how backported detail is?

[analytik]

The detail should be a new property in 16.0.0, although the documentation confusingly refers to it as details.

The quickest, dirtiest, but probably working "patch" is to change gc.js to something like

	const obs = new perf_hooks.PerformanceObserver(list => {
		const entry = list.getEntries()[0];
		const observations = {kind: kinds[entry.detail && entry.detail.kind || entry.kind]};
		// Convert duration from milliseconds to seconds
		gcHistogram.observe(
			Object.assign(observations, labels),
			entry.duration / 1000,
		);
	});

or if someone is a fan of long lines, then just the Object.assign(observations, labels), to Object.assign({kind: kinds[entry.detail && entry.detail.kind || entry.kind]}, labels),.

Alternatively, some if (parseInt(process.versions.node) >= 16) { ... use entry.detail.kind } else { old code }.
That said, I got it to work, but I didn't get the unit tests to succeed under Node.js v16.0.0 because of what I assume is a Node.js bug.

[SimenB]

Checking the object rather than node version makes sense to me 👍 They might backport it, and it'd be good to use it at that point

[sirreal]

I've proposed a fix in #440 building off the research and discussion provided in this thread.

[nlochschmidt]

We have also started to see these deprecation warnings.

Since #440 looks good, but is over two month old I am guessing it might have just been forgotten about and just needs a friendly nudge to get moving again?