search

chrisa / node-dtrace-provider

Native DTrace probes for node.js apps
Other
319 stars 70 forks source link

Nodejs 12 support #125

Closed chrisdoc closed 5 years ago

chrisdoc commented 5 years ago

This PR aims to make node-dtrace-provider compatible with changes in v8 and allows the usage of the library with Node.JS 12. I ran the tests both for master with node 10.15.2 and on the branch with node 12.5.0 and on both I had 670 passing tests out of 699 total tests. I tested on MacOS 10.14.5 and disable SIP by executing csrutil disable in recovery mode. @chrisa do you have a continuous integration setup where I could test this PR.

melloc commented 5 years ago

Hi @chrisdoc ,

There's no CI setup currently. I have a FreeBSD, SmartOS and OS X box that I currently run the tests on. I'll test these changes on them for you this week, and put the results here.

chrisdoc commented 5 years ago

Thank you @melloc for your help in getting this tested

melloc commented 5 years ago

I tested the following combinations:

dtrace-provider failed to compile with node v0.12.18 on macOS due to nodejs/nan#676, which I can't really do anything about in this library. That's probably fine, since I imagine most people are using newer node versions.

I also had to make some minor tweaks to get the tests running on macOS, due to what seems to be a regression in using -Z to attach to USDT probes that haven't yet been defined. I'll follow up with a separate commit to update the tests to work once I'm convinced that's the problem, and that it's not something that libusdt should be updated to account.

melloc commented 5 years ago

Merged as 3cbb3c8. I'll prepare a 0.8.8 release once I figure out whether anything further needs to be done about the macOS issue.

chrisdoc commented 5 years ago

Thank you @melloc