Skip to content

Commit

Permalink
chore: refactor module patch handling in Instrumentation class (#3658)
Browse files Browse the repository at this point in the history
This refactors how module patch handlers are loaded and the
Instrumentation class file. There is now a PatcherRegistry that is
a little smarter than the `_patches` object before it.

The main change is that we now use RITM's internals:true option
so that our `onRequire` hook is called with module sub-paths. This will
allow #3657 to hook both "mongodb" and "mongodb/lib/cmap/connection_pool.js",
and avoid a RITM bug where "mongodb" &
"mongodb/lib/cmap/connection_pool" (no extension, indicating a
"sub-module") don't work together.

Benefits and other changes:
- With the smarter "PatcherRegistry" the module name can be used in
  `disableInstrumentations` to disable any patchers for that module.
  E.g. `disableInstrumentations=next` will disable all four
  `next`-related patchers.
- A custom "lambda" name is now usable to disable the Lambda instr,
  FWIW.
- (Note: we *could* also add custom disableInstrumentations keys for
  other modules, e.g. perhaps "aws-sdk" for all those.)
- Some edge cases with the (rare, and undocumented) `config.addPatch`
  config var have been cleaned up.
- Loading of the lambda-handler has been refactored a bit to no longer
  need to know internal details of the instrumentation module.

Refs: #3657
Closes: #2992
  • Loading branch information
trentm committed Oct 11, 2023
1 parent 3401ece commit 1eae1b5
Show file tree
Hide file tree
Showing 21 changed files with 615 additions and 537 deletions.
1 change: 1 addition & 0 deletions .tav.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ redis:
node: '>=14.0.0'
commands:
- node test/instrumentation/modules/redis.test.js
- node test/instrumentation/modules/redis-disabled.test.js
- node test/instrumentation/modules/redis4-legacy.test.js

# We want these version ranges:
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ See the <<upgrade-to-v4>> guide.
* Add support for `@aws-sdk/client-sqs`, one of the AWS SDK v3 clients.
({issues}2957[#2957])
* Fixes for some values of the <<disable-instrumentations>> config setting.
"redis" will now properly disable instrumentation for redis@4.
"next" will propertly disable all Next.js instrumentation.
({pull}3658[#3658])
[float]
===== Bug fixes
Expand Down
4 changes: 4 additions & 0 deletions docs/agent-api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,10 @@ apm.addPatch('timers', (exports, agent, { version, enabled }) => {
apm.addPatch('timers', './timer-patch')
----

This and the other "Patch"-related API methods should be called *before*
starting the APM agent. Changes after the agent has started and relevant
modules have been `require`d can have surprising caching behavior.

[[apm-remove-patch]]
==== `apm.removePatch(modules, handler)`

Expand Down
2 changes: 1 addition & 1 deletion docs/configuration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ The `sanitizeFieldNames` will redact any matched _field names_. If you wish to
* *Type:* Array of strings
* *Env:* `ELASTIC_APM_DISABLE_INSTRUMENTATIONS`

Array or comma-separated string of modules to disable instrumentation for.
Array or comma-separated string of module names for which to disable instrumentation.
When instrumentation is disabled for a module,
no spans will be collected for that module.

Expand Down
1 change: 1 addition & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ declare namespace apm {
type PatchHandler = (exports: any, agent: Agent, options: PatchOptions) => any;

interface PatchOptions {
name: string;
version: string | undefined;
enabled: boolean;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const connect = require('./middleware/connect');
const constants = require('./constants');
const errors = require('./errors');
const { InflightEventSet } = require('./InflightEventSet');
const Instrumentation = require('./instrumentation');
const { Instrumentation } = require('./instrumentation');
const { elasticApmAwsLambda } = require('./lambda');
const Metrics = require('./metrics');
const parsers = require('./parsers');
Expand Down
Loading

0 comments on commit 1eae1b5

Please sign in to comment.