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

#316: Adds support for http/https APIs for node >= 10 #318

Closed
wants to merge 3 commits into from
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
75 changes: 56 additions & 19 deletions lib/instrumentation/core/http.js
Expand Up @@ -370,8 +370,54 @@ function wrapWriteHead(agent, writeHead) {
}
}

// Taken from the node repo, internal/url.js
function urlToOptions(url) {
const options = {
protocol: url.protocol,
hostname: typeof url.hostname === 'string' && url.hostname.startsWith('[') ?
url.hostname.slice(1, -1) :
url.hostname,
hash: url.hash,
search: url.search,
pathname: url.pathname,
path: `${url.pathname || ''}${url.search || ''}`,
href: url.href
};
if (url.port !== '') {
options.port = Number(url.port);
}
if (url.username || url.password) {
options.auth = `${url.username}:${url.password}`;
}
return options;
}

const searchParamsSymbol = Symbol('query')

function wrapRequest(agent, request) {
return function wrappedRequest(options) {
return function wrappedRequest(input, options, cb) {
// Support new function signatures in Node v10+. If the first argument is a URL, merge
// it into the options object. This code is copied form Node internals.
if (typeof input === 'string') {
const urlStr = input
input = urlToOptions(new URL(urlStr))
} else if (input.constructor.name === 'URL') {
input = urlToOptions(input)
} else {
cb = options
options = input
input = null
}

if (typeof options === 'function') {
cb = options
options = input || {}
} else {
options = Object.assign(input || {}, options)
}

const reqArgs = [options, cb]

// Don't pollute metrics and calls with NR connections
const internalOnly = options && options[NR_CONNECTION_PROP]
if (internalOnly) {
Expand All @@ -389,10 +435,10 @@ function wrapRequest(agent, request) {
logOpts.port
)
}
return request.apply(this, arguments)
return request.apply(this, reqArgs)
}

const args = agent.tracer.slice(arguments)
const args = agent.tracer.slice(reqArgs)
const context = this

// hostname & port logic pulled directly from node's 0.10 lib/http.js
Expand Down Expand Up @@ -481,29 +527,20 @@ module.exports = function initialize(agent, http, moduleName) {
// change originally also appeared in 8.9.0 but was reverted in 8.9.1.
//
// TODO: Remove `SHOULD_WRAP_HTTPS` after deprecating Node <9.
if (SHOULD_WRAP_HTTPS || !IS_HTTPS) {
if (SHOULD_WRAP_HTTPS || (!SHOULD_WRAP_HTTPS && !IS_HTTPS)) {
shimmer.wrapMethod(
http,
'http',
'request',
wrapRequest.bind(null, agent)
)

// Upon updating to https-proxy-agent v2, we are now using
// agent-base v4, which patches https.get to use https.request.
//
// https://github.com/TooTallNate/node-agent-base/commit/7de92df780e147d4618
//
// This means we need to skip instrumenting to avoid double
// instrumenting external requests.
if (!IS_HTTPS && psemver.satisfies('>=8')) {
shimmer.wrapMethod(
http,
'http',
'get',
wrapRequest.bind(null, agent)
)
}
shimmer.wrapMethod(
http,
'http',
'get',
wrapRequest.bind(null, agent)
)
}

shimmer.wrapMethod(
Expand Down
148 changes: 32 additions & 116 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -136,7 +136,7 @@
"concat-stream": "^2.0.0",
"escodegen": "^1.11.1",
"esprima": "^4.0.1",
"https-proxy-agent": "^3.0.0",
"https-proxy-agent": "^4.0.0",
"json-stringify-safe": "^5.0.0",
"readable-stream": "^3.1.1",
"semver": "^5.3.0"
Expand Down Expand Up @@ -165,7 +165,7 @@
"mocha": "^6.2.1",
"mongodb": "^3.3.3",
"mysql": "*",
"nock": "9.1.9",
"nock": "11.8.0",
"proxyquire": "^1.8.0",
"q": "*",
"redis": "^1.0.0",
Expand Down