Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(http): add monitorIncomingHTTPRequests config #1298

Merged
merged 5 commits into from
Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions docs/configuration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,18 @@ A boolean specifying if the agent should collect performance metrics for the app

Note that both `active` and `instrument` needs to be `true` for instrumentation to be running.

[[instrument-incoming-http-requests]]
==== `instrumentIncomingHTTPRequests?:`

* *Type:* Boolean
* *Default:* `true`
* *Env:* `ELASTIC_APM_INSTRUMENT_INCOMING_HTTP_REQUESTS`

A boolean specifying if the agent should instrument incoming HTTP requests.

To configure if outgoing http requests should be instrumented,
see <<disable-instrumentations, `disableInstrumentations`>>.

[[central-config]]
==== `centralConfig`

Expand Down Expand Up @@ -660,6 +672,9 @@ see the https://github.com/elastic/apm-agent-nodejs/tree/master/lib/instrumentat
Note that not all modules represented in this directory will generate spans,
and adding those to this array has no effect.

To configure if incoming http requests should be instrumented,
see <<instrument-incoming-http-requests, `instrumentIncomingHTTPRequests`>>.

[[container-id]]
==== `containerId`

Expand Down
1 change: 1 addition & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ interface AgentConfigOptions {
ignoreUrls?: Array<string | RegExp>;
ignoreUserAgents?: Array<string | RegExp>;
instrument?: boolean;
instrumentIncomingHTTPRequests?: boolean;
kubernetesNamespace?: string;
kubernetesNodeName?: string;
kubernetesPodName?: string;
Expand Down
5 changes: 4 additions & 1 deletion lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var DEFAULTS = {
logLevel: 'info',
metricsInterval: '30s',
metricsLimit: 1000,
instrumentIncomingHTTPRequests: true,
centralConfig: true,
serverTimeout: '30s',
sourceLinesErrorAppFrames: 5,
Expand Down Expand Up @@ -98,6 +99,7 @@ var ENV_TABLE = {
globalLabels: 'ELASTIC_APM_GLOBAL_LABELS',
hostname: 'ELASTIC_APM_HOSTNAME',
instrument: 'ELASTIC_APM_INSTRUMENT',
instrumentIncomingHTTPRequests: 'ELASTIC_APM_INSTRUMENT_INCOMING_HTTP_REQUESTS',
kubernetesNamespace: ['ELASTIC_APM_KUBERNETES_NAMESPACE', 'KUBERNETES_NAMESPACE'],
kubernetesNodeName: ['ELASTIC_APM_KUBERNETES_NODE_NAME', 'KUBERNETES_NODE_NAME'],
kubernetesPodName: ['ELASTIC_APM_KUBERNETES_POD_NAME', 'KUBERNETES_POD_NAME'],
Expand Down Expand Up @@ -138,10 +140,11 @@ var BOOL_OPTS = [
'captureExceptions',
'captureHeaders',
'captureSpanStackTraces',
'centralConfig',
'errorOnAbortedRequests',
'filterHttpHeaders',
'instrument',
'centralConfig',
'instrumentIncomingHTTPRequests',
'usePathAsTransactionName',
'verifyServerCert'
]
Expand Down
13 changes: 8 additions & 5 deletions lib/instrumentation/modules/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@ var httpShared = require('../http-shared')
var shimmer = require('../shimmer')

module.exports = function (http, agent, { enabled }) {
if (agent._conf.instrumentIncomingHTTPRequests) {
agent.logger.debug('shimming http.Server.prototype.emit function')
shimmer.wrap(http && http.Server && http.Server.prototype, 'emit', httpShared.instrumentRequest(agent, 'http'))

agent.logger.debug('shimming http.ServerResponse.prototype.writeHead function')
shimmer.wrap(http && http.ServerResponse && http.ServerResponse.prototype, 'writeHead', wrapWriteHead)
}

if (!enabled) return http
agent.logger.debug('shimming http.Server.prototype.emit function')
shimmer.wrap(http && http.Server && http.Server.prototype, 'emit', httpShared.instrumentRequest(agent, 'http'))

agent.logger.debug('shimming http.request function')
shimmer.wrap(http, 'request', httpShared.traceOutgoingRequest(agent, 'http', 'request'))
Expand All @@ -18,9 +24,6 @@ module.exports = function (http, agent, { enabled }) {
shimmer.wrap(http, 'get', httpShared.traceOutgoingRequest(agent, 'http', 'get'))
}

agent.logger.debug('shimming http.ServerResponse.prototype.writeHead function')
shimmer.wrap(http && http.ServerResponse && http.ServerResponse.prototype, 'writeHead', wrapWriteHead)

return http

function wrapWriteHead (original) {
Expand Down
10 changes: 7 additions & 3 deletions lib/instrumentation/modules/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@ var symbols = require('../../symbols')
var parsers = require('../../parsers')

module.exports = function (http2, agent, { enabled }) {
if (agent._conf.instrumentIncomingHTTPRequests) {
agent.logger.debug('shimming http2.createServer function')
shimmer.wrap(http2, 'createServer', wrapCreateServer)
shimmer.wrap(http2, 'createSecureServer', wrapCreateServer)
}

if (!enabled) return http2
var ins = agent._instrumentation
agent.logger.debug('shimming http2.createServer function')
shimmer.wrap(http2, 'createServer', wrapCreateServer)
shimmer.wrap(http2, 'createSecureServer', wrapCreateServer)
agent.logger.debug('shimming http2.connect function')
shimmer.wrap(http2, 'connect', wrapConnect)

return http2
Expand Down
8 changes: 5 additions & 3 deletions lib/instrumentation/modules/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ var httpShared = require('../http-shared')
var shimmer = require('../shimmer')

module.exports = function (https, agent, { version, enabled }) {
if (!enabled) return https
agent.logger.debug('shimming https.Server.prototype.emit function')
shimmer.wrap(https && https.Server && https.Server.prototype, 'emit', httpShared.instrumentRequest(agent, 'https'))
if (agent._conf.instrumentIncomingHTTPRequests) {
agent.logger.debug('shimming https.Server.prototype.emit function')
shimmer.wrap(https && https.Server && https.Server.prototype, 'emit', httpShared.instrumentRequest(agent, 'https'))
}

if (!enabled) return https
// From Node.js v9.0.0 and onwards, https requests no longer just call the
// http.request function. So to correctly instrument outgoing HTTPS requests
// in all supported Node.js versions, we'll only only instrument the
Expand Down
1 change: 1 addition & 0 deletions test/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ var optionFixtures = [
['frameworkVersion', 'FRAMEWORK_VERSION'],
['hostname', 'HOSTNAME'],
['instrument', 'INSTRUMENT', true],
['instrumentIncomingHTTPRequests', 'INSTRUMENT_INCOMING_HTTP_REQUESTS', true],
['kubernetesNamespace', 'KUBERNETES_NAMESPACE'],
['kubernetesNodeName', 'KUBERNETES_NODE_NAME'],
['kubernetesPodName', 'KUBERNETES_POD_NAME'],
Expand Down
26 changes: 6 additions & 20 deletions test/instrumentation/_agent.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

var config = require('../../lib/config')
var Metrics = require('../../lib/metrics')
var Instrumentation = require('../../lib/instrumentation')
var mockClient = require('../_mock_http_client')
Expand All @@ -12,27 +13,12 @@ var sharedInstrumentation

module.exports = function mockAgent (expected, cb) {
var agent = {
_conf: {
serviceName: 'service-name',
active: true,
instrument: true,
captureSpanStackTraces: true,
_conf: config({
Qard marked this conversation as resolved.
Show resolved Hide resolved
abortedErrorThreshold: '250ms',
centralConfig: false,
errorOnAbortedRequests: false,
abortedErrorThreshold: 0.25,
sourceLinesErrorAppFrames: 5,
sourceLinesErrorLibraryFrames: 5,
sourceLinesSpanAppFrames: 5,
sourceLinesSpanLibraryFrames: 0,
ignoreUrlStr: [],
ignoreUrlRegExp: [],
ignoreUserAgentStr: [],
ignoreUserAgentRegExp: [],
transactionSampleRate: 1.0,
disableInstrumentations: [],
captureHeaders: true,
metricsInterval: 0,
centralConfig: false
},
metricsInterval: 0
}),
_errorFilters: new Filters(),
_transactionFilters: new Filters(),
_spanFilters: new Filters(),
Expand Down
1 change: 1 addition & 0 deletions test/instrumentation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

var agent = require('../..').start({
serviceName: 'test',
breakdownMetrics: false,
captureExceptions: false,
metricsInterval: 0
})
Expand Down
161 changes: 161 additions & 0 deletions test/instrumentation/modules/http/disabling.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
'use strict'

const cluster = require('cluster')

if (cluster.isMaster) {
const test = require('tape')

const findObjInArray = require('../../../_utils').findObjInArray

test('http split disabling', t => {
function makeTest (config, handler) {
return t => {
const worker = cluster.fork()
worker.send(config)
worker.on('message', data => {
worker.disconnect()
handler(t, data)
})
}
}

function assertTopTransaction (t, transactions) {
const trans = findObjInArray(transactions, 'type', 'custom')
t.ok(trans, 'found top transaction')
t.equal(trans.name, 'top', 'transaction name')
}

function assertRequestTransaction (t, transactions) {
const trans = findObjInArray(transactions, 'type', 'request')
t.ok(trans, 'found request transaction')
t.equal(trans.name, 'GET /', 'transaction name')
t.equal(trans.result, 'HTTP 2xx', 'transaction result')
t.equal(trans.context.request.method, 'GET', 'transaction method')
}

function assertSpan (t, span) {
t.ok(/GET localhost:\d+\//.test(span.name), 'span name')
t.equal(span.type, 'ext.http.http', 'span type')
}

t.test('incoming enabled + outgoing enabled', makeTest({
disableInstrumentations: '',
instrumentIncomingHTTPRequests: true
}, (t, data) => {
t.equal(data.transactions.length, 2, 'transaction count')
t.equal(data.spans.length, 1, 'span count')

assertRequestTransaction(t, data.transactions)
assertTopTransaction(t, data.transactions)
assertSpan(t, data.spans[0])

t.end()
}))

t.test('incoming enabled + outgoing disabled', makeTest({
disableInstrumentations: 'http',
instrumentIncomingHTTPRequests: true
}, (t, data) => {
t.equal(data.transactions.length, 2, 'transaction count')
t.equal(data.spans.length, 0, 'span count')

assertRequestTransaction(t, data.transactions)
assertTopTransaction(t, data.transactions)

t.end()
}))

t.test('incoming disabled + outgoing enabled', makeTest({
disableInstrumentations: '',
instrumentIncomingHTTPRequests: false
}, (t, data) => {
t.equal(data.transactions.length, 1, 'transaction count')
t.equal(data.spans.length, 1, 'span count')

assertTopTransaction(t, data.transactions)
assertSpan(t, data.spans[0])

t.end()
}))

t.test('incoming disabled + outgoing disabled', makeTest({
disableInstrumentations: 'http',
instrumentIncomingHTTPRequests: false
}, (t, data) => {
t.equal(data.transactions.length, 1, 'transaction count')
t.equal(data.spans.length, 0, 'span count')

assertTopTransaction(t, data.transactions)

t.end()
}))
})
} else {
const http = require('http')

process.on('message', config => {
class MockTransport {
constructor () {
this.transactions = []
this.spans = []
}

config () { }

sendSpan (span) {
this.spans.push(span)
}

sendTransaction (transaction) {
this.transactions.push(transaction)
}
}

const mock = new MockTransport()

const agent = require('../../../..').start(Object.assign({
captureExceptions: false,
metricsInterval: 0,
centralConfig: false,
transport: () => mock
}, config))

const express = require('express')

const app = express()

app.get('/', (req, res) => {
res.end('hello')
})

let trans
sendRequest(app).then(() => {
trans.end()
setTimeout(() => {
process.send(mock)
}, 10)
})

function ping (url, fn = res => res.resume()) {
return new Promise((resolve, reject) => {
const req = http.get(url, res => {
res.on('error', reject)
res.on('end', resolve)
fn(res)
})
req.on('error', reject)
})
}

function sendRequest (app) {
return new Promise((resolve, reject) => {
const server = app.listen(function () {
const port = server.address().port
trans = agent.startTransaction('top')
ping(`http://localhost:${port}`)
.then(resolve, reject)
})
})
}
})
}