Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions .tav.yml
Original file line number Diff line number Diff line change
Expand Up @@ -210,16 +210,21 @@ hapi-no-async-await:
name: hapi
versions: '>=9.0.1 <17.0.0'
commands:
- node test/instrumentation/modules/hapi/basic.js
- node test/instrumentation/modules/hapi/basic-legacy-path.js
- node test/instrumentation/modules/hapi/set-framework.js
hapi-async-await:
name: hapi
node: '>=8.2'
versions: '>=17.0.0'
commands:
- node test/instrumentation/modules/hapi/basic.js
- node test/instrumentation/modules/hapi/basic-legacy-path.js
- node test/instrumentation/modules/hapi/set-framework.js

'@hapi/hapi':
node: '>=8.2'
versions: '>=17.0.0'
commands:
- node test/instrumentation/modules/hapi/basic.js
- node test/instrumentation/modules/hapi/set-framework-2.js
tedious:
name: tedious
versions: '>=1.9 <2.7 || 3.x || >4.0.0'
Expand Down
10 changes: 5 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ jobs:
-
node_js: '12'
if: type IN (cron, pull_request) AND NOT branch =~ ^greenkeeper/.*
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,express,express-queue
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,@hapi/hapi,express,express-queue
script: tav --quiet
-
node_js: '12'
Expand All @@ -127,7 +127,7 @@ jobs:
-
node_js: '11'
if: type IN (cron, pull_request) AND NOT branch =~ ^greenkeeper/.*
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,express,express-queue
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,@hapi/hapi,express,express-queue
script: tav --quiet
-
node_js: '11'
Expand All @@ -154,7 +154,7 @@ jobs:
-
node_js: '10'
if: type IN (cron, pull_request) AND NOT branch =~ ^greenkeeper/.*
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,express,express-queue
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,@hapi/hapi,express,express-queue
script: tav --quiet
-
node_js: '10'
Expand All @@ -181,7 +181,7 @@ jobs:
-
node_js: '8'
if: type IN (cron, pull_request) AND NOT branch =~ ^greenkeeper/.*
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,express,express-queue
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,@hapi/hapi,express,express-queue
script: tav --quiet
-
node_js: '8'
Expand All @@ -208,7 +208,7 @@ jobs:
-
node_js: '6'
if: type IN (cron, pull_request) AND NOT branch =~ ^greenkeeper/.*
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,express,express-queue
env: TAV=ws,graphql,express-graphql,elasticsearch,hapi,@hapi/hapi,express,express-queue
script: tav --quiet
-
node_js: '6'
Expand Down
27 changes: 23 additions & 4 deletions docs/agent-api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -690,12 +690,12 @@ Optionally, a type may also be provided to group lambdas together. By default,
Read more lambda support in the <<lambda,Lambda>> article.

[[apm-add-patch]]
==== `apm.addPatch(name, handler)`
==== `apm.addPatch(modules, handler)`

[small]#Added in: v2.7.0#

* `name` +{type-string}+
Name of module to apply the patch to, when required.
* `modules` +{type-string or list of strings}+
Name of module(s) to apply the patch to, when required.
* `handler` +{type-function}+ | +{type-string}+
Must be a patch function or a path to a module exporting a patch function
** `exports` +{type-object}+ The original export object of the module
Expand Down Expand Up @@ -726,11 +726,26 @@ apm.addPatch('timers', (exports, agent, { version, enabled }) => {

// or ...

apm.addPatch(['hapi', '@hapi/hapi'], (exports, agent, { version, enabled }) => {
const setTimeout = exports.setTimeout
exports.setTimeout = (fn, ms) => {
const span = agent.createSpan('set-timeout')
return setTimeout(() => {
span.end()
fn()
}, ms)
}

return exports
})

// or ...

apm.addPatch('timers', './timer-patch')
----

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

[small]#Added in: v2.7.0#

Expand All @@ -744,6 +759,10 @@ apm.removePatch('timers', './timers-patch')

// or ...

apm.removePatch(['timers'], './timers-patch')

// or ...

apm.removePatch('timers', timerPatchFunction)
----

Expand Down
1 change: 1 addition & 0 deletions docs/supported-technologies.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ These are the frameworks that we officially support:
|Framework |Version |Note
|<<express,Express>> |^4.0.0 |
|<<hapi,hapi>> |>=9.0.0 <19.0.0 |
|<<hapi,@hapi/hapi>> |>=17.9.0 <19.0.0 |
|<<koa,Koa>> via koa-router |>=5.2.0 <8.0.0 |Koa doesn't have a built in router,
so we can't support Koa directly since we rely on router information for full support.
We currently support the most popular Koa router called https://github.com/alexmingoia/koa-router[koa-router]
Expand Down
37 changes: 24 additions & 13 deletions lib/instrumentation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var MODULES = [
'generic-pool',
'graphql',
'handlebars',
'hapi',
['hapi', '@hapi/hapi'],
'http',
'https',
'http2',
Expand Down Expand Up @@ -70,30 +70,41 @@ function Instrumentation (agent) {
this._patches = new NamedArray()

for (let mod of MODULES) {
if (!Array.isArray(mod)) mod = [mod]
const pathName = mod[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe const pathName = Array.isArray(mod) ? mod[0] : mod, so you don't need to build an array you don't actually need.


this.addPatch(mod, (exports, name, version, enabled) => {
// Lazy require so that we don't have to use `require.resolve` which
// would fail in combination with Webpack. For more info see:
// https://github.com/elastic/apm-agent-nodejs/pull/957
const patch = require(`./modules/${mod}`)
const patch = require(`./modules/${pathName}`)
return patch(exports, name, version, enabled)
})
}
}

Instrumentation.prototype.addPatch = function (name, handler) {
const type = typeof handler
if (type !== 'function' && type !== 'string') {
this._agent.logger.error('Invalid patch handler type:', type)
return
}
Instrumentation.prototype.addPatch = function (modules, handler) {
if (!Array.isArray(modules)) modules = [modules]

this._patches.add(name, handler)
this._startHook()
modules.forEach(mod => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be for (let mod of modules) {.

const type = typeof handler
if (type !== 'function' && type !== 'string') {
this._agent.logger.error('Invalid patch handler type:', type)
return
}

this._patches.add(mod, handler)
this._startHook()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call to this._startHook() could be moved out of the loop as it doesn't need to be called between patches being added.

})
}

Instrumentation.prototype.removePatch = function (name, handler) {
this._patches.delete(name, handler)
this._startHook()
Instrumentation.prototype.removePatch = function (modules, handler) {
if (!Array.isArray(modules)) modules = [modules]

modules.forEach(mod => {
this._patches.delete(mod, handler)
this._startHook()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, the call to this._startHook() doesn't need to be made between each change, you could just put it after the loop. Also, for/let loop too, please. :)

})
}

Instrumentation.prototype.clearPatches = function (name) {
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
"graphql": "^14.3.1",
"handlebars": "^4.0.12",
"hapi": "^18.1.0",
"@hapi/hapi": "^18.2.0",
"https-pem": "^2.0.0",
"inquirer": "^0.12.0",
"ioredis": "^4.10.0",
Expand Down Expand Up @@ -182,7 +183,7 @@
]
},
"coordinates": [
55.77826,
12.593255
55.778264,
12.59313
]
}
8 changes: 7 additions & 1 deletion test/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,13 @@ test('disableInstrumentations', function (t) {
}

for (const mod of modules) {
require(mod)
if (Array.isArray(mod)) {
for (const subMod of mod) {
require(subMod)
}
} else {
require(mod)
}
}

t.deepEqual(selectionSet, found, 'disabled all selected modules')
Expand Down
3 changes: 3 additions & 0 deletions test/instrumentation/modules/hapi/basic-legacy-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'use strict'

require('./shared')('hapi')
Loading