instrumental/instrumental_agent-node

Node.js Agent for Instrumental Application Monitoring

instrumental_agent-node's Issues

Race Condition?

Perhaps it is not possible with the node threading model, but https://github.com/expectedbehavior/instrumental-node/blob/master/lib/instrumental.js#L51 seems like it should be an atomic array option that nukes any elements added between the socket write and the setting of the array to []. Possibly more correct and with its own (probably harder to hit) race condition is something along the lines of:

var callsToWrite = this.queuedCalls.splice(0, this.queuedCalls.length);
this.socket.write(callsToWrite.join());

Lingering socket connection prevents app from exiting

Just switched from the old client to instrumental_agent-node and I've noticed that my app no longer completely exits. I believe this is due to a ref'd socket in the Instrumental module.

In my case the underlying Instrumental object will never be GC'd since it's rooted to an object that only goes out of scope on exit. I've added a call in that code's shutdown routine (this is a long-running process that has graceful shutdown behavior) to invoke Instrumental.socket.end(), solving the problem.

However, this gives me hives since I feel like I'm accessing a private class member, even though this is JS and nobody seems to care much about that. Still... ew.

I'm happy to take a stab at solving this in the module and make a PR but would want your input before starting:

1. Setting a socket timeout and setting this.socket = undefined after destroying the socket. The API already has robust reconnection logic so this would be transparent to the user. There is potential impact to the Instrumental service getting connection flooded by clients with ridiculously low timeouts, but enforcing a reasonable range should get around this.

2. Hooking SIGINT and closing the connection.

3. Exposing something like Instrumental.disconnect() and leave it up to the caller when to use it.

...or maybe something else. I'm happy to keep using my workaround, but I imagine others will hit this.

Support Notices

They are a thing our collector API supports and we should support them in this agent.

Agent not compatible for node > 8

I'm currently using NodeJS 9.x and the package is not compatible with version greater than 8.

module version mismatch

https://www.npmjs.com/package/instrumental-node says the module is at version 1.0.0, but the code in this repo is only at version 0.2.0 (https://github.com/Instrumental/instrumental_agent-node/blob/master/package.json#L3). What is correct? Doesn't look like the package is at v1.0.0 in any tags or branches either.

The Instrumental class constructor should accept config options

We're using the Instrumental class directly in a project (we need 2 instances of the agent side by side), and it's annoying to have to call configure separately. We can't chain the call to configure (i.e.,I = new Instrumental().configure({...})) since it doesn't return the instance. That means we've got to call configure separately for both instances.

It seems like the best option is to allow the constructor to optionally take config options.

update engine restriction

Hi, I get very annoying error messages when installing this package with yarn due the engines flag in the package.json. Is this package compatible with node 7.x.x or not?

node.js 10.x support?

In package.json it seems that node.js 10.x support is explicitly removed: "node": ">=5.1 <10".

Is there a reason for this? The 10.x branch is a LTS branch and will be supported until 2021.

Update package name to instrumental_agent

We released this before we discussed how we wanted to name our packages. We should update the package unless there's a good reason not to.

Here's the current: https://www.npmjs.com/package/instrumental-node

Some helpful info on package renaming:
http://stackoverflow.com/questions/28371669/renaming-a-published-npm-module
https://docs.npmjs.com/cli/deprecate
https://www.gregjs.com/javascript/2016/how-to-rename-a-published-npm-module/

Read Agent Version from file/constant

Particularly here.