elastic / elastic/apm-agent-nodejs

options to fail fast on module instrumentation and to override module version guards

Open
#1,915 4 comments 1 reaction 0 assignees View on GitHub
agent-nodejs
Dominant language
JavaScript
Stars
594
Forks
244
Avg merge
1d 8h
Merged PRs (30d)
16

Description

It might be useful to support some config options for control over instrumented module version guards. Specifically:

1. a config option to have the APM agent fail fast/hard if a particular module cannot be instrumented
2. a config option to allow the user to override a version guard

### background

Typically a particular package instrumentation (i.e. each file under https://github.com/elastic/apm-agent-nodejs/tree/master/lib/instrumentation/modules/) will begin something like this:

```
module.exports = function (cassandra, agent, { version, enabled }) {
if (!enabled) return cassandra
if (!semver.satisfies(version, '>=3 <5')) {
agent.logger.debug('cassandra-driver version %s not supported - aborting...', version)
return cassandra
}
// ...
```

The `if (!enabled) ...` is how the [`disableInstrumentations` config var](https://www.elastic.co/guide/en/apm/agent/nodejs/current/configuration.html#disable-instrumentations) works.

The `if (!semver.satisfies(version, '...')) ...` I'm calling the "module version guard". The current considered best practice for this Node.js APM agent's module instrumentations is to be explicit about the supported version range. This means that typically the module version guard will explicitly have a "< $next_major_version" condition. (This "best practice" isn't currently written down, nor universal to all currently instrumented modules, and is still debatable. See #745 and #514.)

Using the cassandra example above, this practice means that we explicitly will instrument "cassandra-driver" versions 3.x and 4.x, but **not 5.x or later**. Should a cassandra-driver@5.0.0 be released, then the current Node.js APM won't instrument it.

Pros:
- Theoretically if the APM agent says it will instrument a module, then it will do so properly.

Cons:
- When a user upgrades to a new major release of a particular module, then APM instrumentation will likely stop.
- Perhaps worse, this change will happen *silently*, because the only noticeable changes are a log message at *debug*-level, a level that is likely not enabled by most users (the default level is "info").

### proposal

1. A config option that would allow a user of the Node.js APM agent to say "I know I want my package FOO to be instrumented, please fail if APM thinks it cannot instrument it." Something like `requireInstrumentations`:

```
require('elastic-apm-node').start({
disableInstrumentations: ['bluebird'],
requireInstrumentations: ['redis', 'cassandra-driver']
})
```

2. A config option that would allow a user of the Node.js APM agent to say "I know package FOO version $new_major isn't explicitly supported by this version, I'd like you to try anyway because it seems to work for me." Something like:

```
require('elastic-apm-node').start({
disableInstrumentations: ['bluebird'],
requireInstrumentations: ['redis', 'cassandra-driver'],
experimentalOverrideInstrumentationVersionGuard: ['cassandra-driver']
})
```

I'm not married to the particular names. I prefixed "experimental" here because I think it is a feature that shouldn't persist in user code for very long. It is a bridge to be able to use an existing version of the Node.js APM agent until it updates to provide support for a $new_major release of a particular package. Whenever an override is used, the APM agent should always emit a log message at the "warning" level.

Note: I think my "2." might be mostly a dupe of #745

### todos

- [ ] Discuss
- [ ] What, if anything, do the other Elastic APM agents provide around this?
- [ ] Ditto for other APM vendors?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.