Node 21+ support - system test failing

[aabmass]

Specifically seeing this issue with Node v21.6.1. What's happening is the output off pprof is missing function name (it just says "anonymous"):

+ pprof -filefunctions -top -nodecount=2 time.pb.gz                                               
Main binary filename not available.
Type: wall                                                                                        
Time: Jan 31, 2024 at 10:07pm (UTC)
Duration: 10.03s, Total samples = 9274ms (92.48%)                                                 
Showing nodes accounting for 9126ms, 98.40% of 9274ms total                                       
Dropped 43 nodes (cum <= 46.37ms)
Showing top 2 nodes out of 5
      flat  flat%   sum%        cum   cum%                                                        
    8262ms 89.09% 89.09%     8269ms 89.16%  (anonymous) /tmp/tmp.5G3K6fnfvv/busybench/src/busybench.ts
     864ms  9.32% 98.40%      864ms  9.32%  (idle)

compare the the Node 20 version which has the function name busyLoop:

+ pprof -filefunctions -top -nodecount=2 time.pb.gz                                                
Main binary filename not available.
Type: wall                                       
Time: Jan 31, 2024 at 9:24pm (UTC)
Duration: 10.03s, Total samples = 9272ms (92.47%)                                                  
Showing nodes accounting for 8750ms, 94.37% of 9272ms total                                        
Dropped 44 nodes (cum <= 46.36ms)
Showing top 2 nodes out of 6
      flat  flat%   sum%        cum   cum%
    7910ms 85.31% 85.31%     7913ms 85.34%  busyLoop /tmp/tmp.RSf1fHb4u0/busybench/src/busybench.ts 
     840ms  9.06% 94.37%      840ms  9.06%  (idle)

I'm not sure why, but its probably related to the V8 version used in Node 21. Some ideas I think it might be

• Bug in V8's CpuProfiler
• Bug in our code? Could try following https://nodejs.org/en/guides/simple-profiling and see if it has the correct result
• V8 doing some optimization that messes things up

Originally from #281

[aabmass]

OK one way to make the system test pass for CPU profiling is to add a noop in the inner loop: #284

This leads me to think the issue is with V8 CpuProfiler or there is some optimization affecting the profile. In any event, it's probably not our fault. I also tried with various other node v21.* minor versions with the same effect.

[antonio0]

Has there been any update on supporting Node 21+, given that Node 20 just entered maintenance mode and 22 is the current LTS version?

[aabmass]

It mostly likely works (it compiles fine), our tests are just failing. Did you ever try it out and was it working alright for you?

[benminer]

Hey @aabmass, running on Node 22.11.0 and node-pre-gyp build is failing for me.

Versions:

• @google-cloud/profiler: 6.0.2
• pprof: 4.0.0

I am seeing a 404 for node-v127 in the build output, has Nodejs 22 support not been released yet?

npm error node-pre-gyp info check checked for "/Users/ben/scope3/monorepo/third/scope3/node_modules/pprof/build/node-v127-darwin-arm64-unknown/pprof.node" (not found)
npm error node-pre-gyp http GET https://storage.googleapis.com/cloud-profiler/pprof-nodejs/release/v4.0.0/node-v127-darwin-arm64-unknown.tar.gz
npm error node-pre-gyp ERR! install response status 404 Not Found on https://storage.googleapis.com/cloud-profiler/pprof-nodejs/release/v4.0.0/node-v127-darwin-arm64-unknown.tar.gz 
npm error node-pre-gyp WARN Pre-built binaries not installable for [email protected] and [email protected] (node-v127 ABI, unknown) (falling back to source compile with node-gyp) 
npm error node-pre-gyp WARN Hit error response status 404 Not Found on https://storage.googleapis.com/cloud-profiler/pprof-nodejs/release/v4.0.0/node-v127-darwin-arm64-unknown.tar.gz 

[benminer]

Issue was a change in nan, PR up here: #303

[benminer]

@aabmass seeing all tests passing with the nan update merged. Tested in both NodeJS 21 and 22.

[benminer]

@aabmass Is it possible to a minor patch release soon?

What originally got me here was @google-cloud/profiler's dependency on this package. If this is published with the nan bump, I could open a PR on the latter with an update to pprof.

Thanks!

[aabmass]

Will try to get to it tomorrow, sorry. I'd like to update the CI to test Node 21 before releasing

[benminer]

@aabmass Fixed here: #305