feat(server): use native V8 heapdump

[PixnBits]

replaces binary module dependency

[oneamexbot]

📊 Bundle Size Report

file name	size on disk	gzip
app.js	113.2KB	31.6KB
runtime.js	15KB	5.4KB
vendors.js	128.4KB	38KB
app~vendors.js	433.9KB	112.5KB
service-worker-client.js	16.9KB	5.2KB
legacy/app.js	120KB	33.2KB
legacy/runtime.js	15KB	5.4KB
legacy/vendors.js	163.4KB	44.9KB
legacy/app~vendors.js	442.2KB	114.7KB
legacy/service-worker-client.js	17.4KB	5.4KB

Generated by 🚫 dangerJS against fa95e0f

[PixnBits]

Preflight Checklist:

• What is the communication plan for this change?
  This is existing functionality just refactored so not planning on communication as I don't think it's needed? (though now I'm realizing the commit message should have been "refactor" instead of "feat")
• Does any documentation need to be updated or added to account for this change? If so was it done already?
  I don't think so.
• What is the motivation for this change?
  This simplifies installs and reduces the chance of dependency installation failures as the API used is now part of Node.JS core. Issues should be caught before releases and if there are issues, they will likely be faster to be fixed.
• Should these changes also be applied to a maintenance branch or any other long lived branch?
  No, this new API was added in Node.JS v11.13.0 and thus is not applicable for one-app v4 which runs on Node.JS 10.x
• How was this change tested?
  Unit and integration tests, also loaded the heap snapshot into Chrome's DevTools to ensure they are usable (not corrupted)
• Does this change require cross browser checks? Why or why not?
  No, this is not something exposed to browsers (note: "The JSON schema is undocumented and specific to the V8 engine, and may change from one version of V8 to the next." https://nodejs.org/docs/latest-v12.x/api/v8.html#v8_v8_getheapsnapshot)
• Does this change require a performance test prior to merging? Why or why not?
  V8 has changed it's internals where it has multiple threads. Previously heapdumps would pause/stop the world so the existing operational model is to stop routing traffic to a container before taking the heapdump. This new method may minimize that delay but isn't generally part of a performance test.
• Could this be considered a breaking change? Why or why not?
  No, the public interface is the same (still uses the SIGUSR2 signal), the log messages are similar, as is the location the files are written to.
• Does the change impact caching?
  No.
• Does the change impact HTTP headers?
  No.
• Does the change have any new infrastructure requirements?
  No.
• Does the change affect other versions of the app?
  No.
• Does the change require additional environment variables?
  No. There are new runtime CLI options (https://nodejs.org/api/report.html) that may use the SIGUSR2 signal but they are experimental, can use a different signal, and we log a warning when they are in use.
• What is the impact to developers using the app?
  None, they can still get heapdumps the same way if they are investigating memory leaks.
• What is the change to the size of assets?
  None.
• Should integration tests be added to protect against future regressions on this change?
  Integration tests could have been added (and maybe should be for v4 as the input and output are the same) in a different PR. In this case integration tests are included in this PR.

[PixnBits]

• heapdump › writes a heapdump to /tmp on a SIGUSR2 signal
    Failed to match: /wrote heapdump out to .+/ in logs

worked locally, so finishing the heapdump in Travis seems slower than on my local machine, I'll bump the timeout up a bit

[PixnBits]

The integration test passes locally and much faster. Should I increase the timeout further for Travis or remove the test? 😬
Is there a third option? 🙏

[mtomcal]

If we need to increase the timeout in travis, let's do it and see how the tests work out here in the PR.

[JAdshead]

The integration test passes locally and much faster. Should I increase the timeout further for Travis or remove the test? 😬
Is there a third option? 🙏

@PixnBits I think 95000ms should already be a long enough timeout, how long is it taking locally ?
Could there be another issue causing this to fail ?

[PixnBits]

less than the original 20s so seems like a setup issue in Travis? maybe someone could check this out locally to sanity check my findings
[edit]
latest run locally in the report I see

writes a heapdump to /tmp on a SIGUSR2 signal, passed in 3.43s

[infoxicator]

@PixnBits Locally for me passed in 3.8s

✓ writes a heapdump to /tmp on a SIGUSR2 signal (3853ms)

There must be something going on in Travis, I will take a look

[github-actions]

This pull request is stale because it has been open 30 days with no activity.

[PixnBits]

huh, seems like a previous copy+paste error

Suggested change

	describe('safeRequest', () => {
	describe('heapdump', () => {

[PixnBits]

resolving merge conflicts, and reverting the time bumps while at it
[edit] oops, renamed the test but didn't update the snapshot 🤦 pushing again...

[github-actions]

📊 Bundle Size Report

file name	size on disk	gzip
app.js	275.4KB	82.2KB
runtime.js	15KB	5.4KB
vendors.js	117.2KB	36.4KB
app~vendors.js	432.5KB	112.2KB
service-worker-client.js	17KB	5.2KB
legacy/app.js	290.7KB	85.4KB
legacy/runtime.js	15KB	5.4KB
legacy/vendors.js	172.7KB	47.1KB
legacy/app~vendors.js	441.3KB	114.5KB
legacy/service-worker-client.js	17.5KB	5.4KB

Generated by 🚫 dangerJS against aa9afb2