honeycombio / libhoney-js

Javascript library for sending data to Honeycomb
Apache License 2.0
49 stars 29 forks source link

Keep-alive HTTP connections with api.honeycomb.io? #351

Open dko-slapdash opened 1 year ago

dko-slapdash commented 1 year ago

Hi.

I looked at the number of TCP (actually SSL) connections per second our infra opens, and it appeared that libhoney produces the absolute most of them. And SSL connections are CPU expensive (they even block event loop sensibly if done simultaneously many times) - we recently shaved off a lot by e.g. lowering the number of connections we reopen with pgbouncer.

Sounds like libhoney doesn't support persistent (keep-alive) connections?

Meanwhile, superagent supports passing a custom instance of Agent, which can be agentkeepalive. But libhoney doesn't expose an API to actually pass a custom Agent instance. (And even without agentkeepalive module, keep-alive mode can probably be turned on by sending a Keep-Alive http header - which libhoney doesn’t support injection of as well).

dko-slapdash commented 1 year ago

Here is a patch which adds agent option to Libhoney.

diff --git a/node_modules/libhoney/dist/libhoney.cjs.js b/node_modules/libhoney/dist/libhoney.cjs.js
index 9249824..9a4bb0b 100644
--- a/node_modules/libhoney/dist/libhoney.cjs.js
+++ b/node_modules/libhoney/dist/libhoney.cjs.js
@@ -443,6 +443,7 @@ class Transmission {

     this._userAgentAddition = options.userAgentAddition || "";
     this._proxy = options.proxy;
+    this._agent = options.agent;

     // Included for testing; to stub out randomness and verify that an event
     // was dropped.
@@ -565,6 +566,11 @@ class Transmission {
               userAgent = `${LIBHONEY_VERSION} ${trimmedAddition} ${NODE_VERSION}`;
             }

+            let agent = typeof this._agent === "function" ? this._agent() : this._agent;
+            if (agent) {
+              req.agent(agent);
+            }
+
             let start = Date.now();
             req
               .set("X-Honeycomb-Team", batch.writeKey)

Usage:

new Libhoney({ ..., agent: new https.Agent({ keepAlive: true, maxSockets: 10 })});

Works very well, dropped the rate of new HTTPS connections dramatically for us.

MikeGoldsmith commented 1 year ago

Hi @dko-slapdash - thanks for the creating the issue and proposing a solution.

I generally like what you've suggested. I'm a little wary of allowing a user to provide their own custom agent as it they could accidentally use the wrong implementation (http vs https) for their destination URL. However, I think good documentation here would help.

An alternative solution would be to expose discrete options for keepalive and maxsockets and we create the agent, but then we'd need to pick a default maxsockets if one isn't set which could also be tricky.

Please could you create a PR with your suggestion, and we can iterate from there. Thanks 👍