fix: handle big integers in incoming events

[lance]

An event may have data that contains a BigInt. The builtin JSON parser
for JavaScript does not handle the BigInt types. The introduced
json-bigint dependency (34k) does.

Fixes: #489

Signed-off-by: Lance Ball lball@redhat.com

[cardil]

Thanks a lot @lance for fixing this. It looks good, but I found some doubts about this proposal.

[cardil]

Have you measured the impact of using this library versus using a native V8 implementation?

If the difference is significant (it might be), we should consider gating this behavior with a configuration option (it may be turned on by default IMHO).

[lance]

This is a good point, so I decided to try a simple, probably naive test. But no matter how naive, you can't ignore a 7x slowdown with bigint support.

❯ time node index.js
Unsing native JSON
....................................................................................................
node index.js  199.67s user 1.29s system 98% cpu 3:23.76 total

❯ time node index.js bigint
Using bigint
....................................................................................................
node index.js bigint  1308.75s user 4.49s system 99% cpu 21:55.06 total

❯ cat index.js
   1   │ const fs = require('fs');
   2   │
   3   │ const J = require('json-bigint')({ useNativeBitInt: true });
   4   │ let Parser;
   5   │
   6   │ if (process.argv[2] === 'bigint') {
   7   │   Parser = J;
   8   │   console.log('Using bigint');
   9   │ } else {
  10   │   Parser = JSON;
  11   │   console.log('Unsing native JSON');
  12   │ }
  13   │
  14   │ const json = fs.readFileSync('./generated.json');
  15   │
  16   │ let remain = 10_000_000;
  17   │ while (remain-- > 0) {
  18   │   Parser.parse(json);
  19   │   if (remain%100000 === 0) {
  20   │     process.stdout.write('.');
  21   │   }
  22   │ }
  23   │
  24   │ console.log();

There is no obvious place to set global parser variables. I need to think on this a bit.

[cardil]

@lance Have you thought of where to place this feature flag?

[lance]

Closing and reopening to trigger re-run of CI with updated package.json (to hopefully fix errors)

[github-actions]

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

[lance]

@cardil I have added an environment variable to enable the json-bigint module. I'm not a huge fan of this approach because the check is made for each call to the parser. The library does not have an obvious place to set flags or other static values. An alternative approach would be to add this as an option to the HTTP.toEvent() method, but that changes the method signature resulting in a lot of small changes all over the codebase.

@lholmquist ptal

[lholmquist]

/lgtm.

I do like your suggestion of adding this to the method signature, which as you said is a bigger change, but maybe we can do that later then