diff --git a/lib/instrumentation/core/http-outbound.js b/lib/instrumentation/core/http-outbound.js index fc829c7143..8ef0139fe3 100644 --- a/lib/instrumentation/core/http-outbound.js +++ b/lib/instrumentation/core/http-outbound.js @@ -29,19 +29,21 @@ const NEWRELIC_SYNTHETICS_HEADER = 'x-newrelic-synthetics' * * @return {http.ClientRequest} The instrumented outbound request. */ -module.exports = function instrumentOutbound(agent, opts, makeRequest) { - if (typeof opts === 'string') { - opts = url.parse(opts) - } else { - opts = copy.shallow(opts) +module.exports = function instrumentOutbound(agent, _url, opts, makeRequest) { + opts = copy.shallow(opts) + + if (!_url) { + _url = opts + } else if (typeof _url === 'string') { + _url = url.parse(_url) } - let hostname = opts.hostname || opts.host || DEFAULT_HOST - let port = opts.port || opts.defaultPort + let port = _url.port || opts.defaultPort if (!port) { - port = (!opts.protocol || opts.protocol === 'http:') ? DEFAULT_PORT : DEFAULT_SSL_PORT + port = (!_url.protocol || _url.protocol === 'http:') ? DEFAULT_PORT : DEFAULT_SSL_PORT } + let hostname = _url.hostname || _url.host || DEFAULT_HOST if (!hostname || port < 1) { logger.warn( 'Invalid host name (%s) or port (%s) for outbound request.', @@ -124,7 +126,7 @@ module.exports = function instrumentOutbound(agent, opts, makeRequest) { segment.start() const request = makeRequest(opts) const parsed = urltils.scrubAndParseParameters(request.path) - const proto = parsed.protocol || opts.protocol || 'http:' + const proto = parsed.protocol || _url.protocol || 'http:' segment.name += parsed.path Object.defineProperty(request, '__NR_segment', { value: segment diff --git a/lib/instrumentation/core/http.js b/lib/instrumentation/core/http.js index fb67c87ae5..28ed206947 100644 --- a/lib/instrumentation/core/http.js +++ b/lib/instrumentation/core/http.js @@ -371,13 +371,36 @@ function wrapWriteHead(agent, writeHead) { } function wrapRequest(agent, request) { - return function wrappedRequest(options) { + return function wrappedRequest() { + let _url + let options + let cb + + if ( + typeof arguments[0] === "string" || + (global.URL && arguments[0] instanceof global.URL) + ) { + _url = arguments[0] + + if (arguments[2]) { + options = arguments[1] + cb = arguments[2] + } else { + cb = arguments[1] + } + } else { + options = arguments[0] + cb = arguments[1] + } + // Don't pollute metrics and calls with NR connections const internalOnly = options && options[NR_CONNECTION_PROP] if (internalOnly) { delete options[NR_CONNECTION_PROP] } + const reqArgs = _url ? [_url, options, cb] : [options, cb] + // If this is not a request we're recording, exit early. const transaction = agent.tracer.getTransaction() if (!transaction || internalOnly) { @@ -389,15 +412,19 @@ 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 - return instrumentOutbound(agent, options, function makeRequest(opts) { - args[0] = opts + return instrumentOutbound(agent, _url, options, function makeRequest(opts) { + if (_url) { + args[1] = opts + } else { + args[0] = opts + } return request.apply(context, args) }) } @@ -408,7 +435,7 @@ function wrapLegacyRequest(agent, request) { var makeRequest = request.bind(this, method, path, headers) if (agent.tracer.getTransaction()) { - return instrumentOutbound(agent, this, makeRequest) + return instrumentOutbound(agent, undefined, this, makeRequest) } logger.trace('No transaction, not recording external to %s:%s', this.host, this.port) @@ -481,7 +508,7 @@ 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) { shimmer.wrapMethod( http, 'http', diff --git a/package-lock.json b/package-lock.json index 4c9fdc1a77..afc786cc0d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1243,20 +1243,6 @@ "type-detect": "^4.0.0" } }, - "deep-equal": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/deep-equal/-/deep-equal-1.1.1.tgz", - "integrity": "sha512-yd9c5AdiqVcR+JjcwUQb9DkhJc8ngNr0MahEBGvDiJw8puWab2yZlh+nkasOnZP+EGTAP6rRp2JzJhJZzvNF8g==", - "dev": true, - "requires": { - "is-arguments": "^1.0.4", - "is-date-object": "^1.0.1", - "is-regex": "^1.0.4", - "object-is": "^1.0.1", - "object-keys": "^1.1.1", - "regexp.prototype.flags": "^1.2.0" - } - }, "deep-is": { "version": "0.1.3", "resolved": "https://registry.npmjs.org/deep-is/-/deep-is-0.1.3.tgz", @@ -2573,12 +2559,6 @@ "integrity": "sha512-M4Sjn6N/+O6/IXSJseKqHoFc+5FdGJ22sXqnjTpdZweHK64MzEPAyQZyEU3R/KRv2GLoa7nNtg/C2Ev6m7z+eA==", "dev": true }, - "is-arguments": { - "version": "1.0.4", - "resolved": "https://registry.npmjs.org/is-arguments/-/is-arguments-1.0.4.tgz", - "integrity": "sha512-xPh0Rmt8NE65sNzvyUmWgI1tz3mKq74lGA0mL8LYZcoIzKOzDh6HmrYm3d18k60nHerC8A9Km8kYu87zfSFnLA==", - "dev": true - }, "is-arrayish": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/is-arrayish/-/is-arrayish-0.2.1.tgz", @@ -3672,70 +3652,27 @@ } }, "nock": { - "version": "9.1.9", - "resolved": "https://registry.npmjs.org/nock/-/nock-9.1.9.tgz", - "integrity": "sha512-dQqtJ8EuQ5D5rMHRed9EqhhDbMlmI6Xd9ovMGE0Ff9UEOna9eSFxBWiXWQOTmvrOstq6UG88mku1wvE6624unA==", + "version": "11.7.0", + "resolved": "https://registry.npmjs.org/nock/-/nock-11.7.0.tgz", + "integrity": "sha512-7c1jhHew74C33OBeRYyQENT+YXQiejpwIrEjinh6dRurBae+Ei4QjeUaPlkptIF0ZacEiVCnw8dWaxqepkiihg==", "dev": true, "requires": { - "chai": ">=1.9.2 <4.0.0", - "debug": "^2.2.0", - "deep-equal": "^1.0.0", + "chai": "^4.1.2", + "debug": "^4.1.0", "json-stringify-safe": "^5.0.1", - "lodash": "~4.17.2", + "lodash": "^4.17.13", "mkdirp": "^0.5.0", - "propagate": "0.4.0", - "qs": "^6.5.1", - "semver": "^5.3.0" + "propagate": "^2.0.0" }, "dependencies": { - "chai": { - "version": "3.5.0", - "resolved": "https://registry.npmjs.org/chai/-/chai-3.5.0.tgz", - "integrity": "sha1-TQJjewZ/6Vi9v906QOxW/vc3Mkc=", - "dev": true, - "requires": { - "assertion-error": "^1.0.1", - "deep-eql": "^0.1.3", - "type-detect": "^1.0.0" - } - }, "debug": { - "version": "2.6.9", - "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", - "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", - "dev": true, - "requires": { - "ms": "2.0.0" - } - }, - "deep-eql": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/deep-eql/-/deep-eql-0.1.3.tgz", - "integrity": "sha1-71WKyrjeJSBs1xOQbXTlaTDrafI=", + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", + "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", "dev": true, "requires": { - "type-detect": "0.1.1" - }, - "dependencies": { - "type-detect": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-0.1.1.tgz", - "integrity": "sha1-C6XsKohWQORw6k6FBZcZANrFiCI=", - "dev": true - } + "ms": "^2.1.1" } - }, - "ms": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", - "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=", - "dev": true - }, - "type-detect": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-1.0.0.tgz", - "integrity": "sha1-diIXzAbbJY7EiQihKY6LlRIejqI=", - "dev": true } } }, @@ -3852,12 +3789,6 @@ "integrity": "sha512-a7pEHdh1xKIAgTySUGgLMx/xwDZskN1Ud6egYYN3EdRW4ZMPNEDUTF+hwy2LUC+Bl+SyLXANnwz/jyh/qutKUw==", "dev": true }, - "object-is": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/object-is/-/object-is-1.0.1.tgz", - "integrity": "sha1-CqYOyZiaCz7Xlc9NBvYs8a1lObY=", - "dev": true - }, "object-keys": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/object-keys/-/object-keys-1.1.1.tgz", @@ -4209,9 +4140,9 @@ "dev": true }, "propagate": { - "version": "0.4.0", - "resolved": "https://registry.npmjs.org/propagate/-/propagate-0.4.0.tgz", - "integrity": "sha1-8/zKCm/gZzanulcpZgaWF8EwtIE=", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", + "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", "dev": true }, "proxy-addr": { @@ -4346,15 +4277,6 @@ "integrity": "sha512-naKIZz2GQ8JWh///G7L3X6LaQUAMp2lvb1rvwwsURe/VXwD6VMfr+/1NuNw3ag8v2kY1aQ/go5SNn79O9JU7yw==", "dev": true }, - "regexp.prototype.flags": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/regexp.prototype.flags/-/regexp.prototype.flags-1.2.0.tgz", - "integrity": "sha512-ztaw4M1VqgMwl9HlPpOuiYgItcHlunW0He2fE6eNfT6E/CF2FtYi9ofOYe4mKntstYk0Fyh/rDRBdS3AnxjlrA==", - "dev": true, - "requires": { - "define-properties": "^1.1.2" - } - }, "regexpp": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/regexpp/-/regexpp-2.0.1.tgz", diff --git a/package.json b/package.json index 224acf40d5..1c29a39368 100644 --- a/package.json +++ b/package.json @@ -165,7 +165,7 @@ "mocha": "^6.2.1", "mongodb": "^3.3.3", "mysql": "*", - "nock": "9.1.9", + "nock": "11.7.0", "proxyquire": "^1.8.0", "q": "*", "redis": "^1.0.0", diff --git a/test/unit/instrumentation/http/outbound.test.js b/test/unit/instrumentation/http/outbound.test.js index 07a3a2840e..ec637e50a1 100644 --- a/test/unit/instrumentation/http/outbound.test.js +++ b/test/unit/instrumentation/http/outbound.test.js @@ -1,6 +1,7 @@ 'use strict' var http = require('http') +var https = require('https') var url = require('url') var events = require('events') var expect = require('chai').expect @@ -73,7 +74,7 @@ describe('instrumentOutbound', function() { }) var req = new events.EventEmitter() helper.runInTransaction(agent, function(transaction) { - instrumentOutbound(agent, {host: HOSTNAME, port: PORT}, makeFakeRequest) + instrumentOutbound(agent, undefined, {host: HOSTNAME, port: PORT}, makeFakeRequest) expect(transaction.trace.root.children[0].getAttributes()).to.deep.equal({}) function makeFakeRequest() { @@ -92,7 +93,7 @@ describe('instrumentOutbound', function() { }) var req = new events.EventEmitter() helper.runInTransaction(agent, function(transaction) { - instrumentOutbound(agent, {host: HOSTNAME, port: PORT}, makeFakeRequest) + instrumentOutbound(agent, undefined, {host: HOSTNAME, port: PORT}, makeFakeRequest) expect(transaction.trace.root.children[0].getAttributes()).to.deep.equal({ 'procedure': 'GET', 'url': `http://${HOSTNAME}:${PORT}/asdf`, @@ -113,7 +114,7 @@ describe('instrumentOutbound', function() { var path = '/asdf' var name = NAMES.EXTERNAL.PREFIX + HOSTNAME + ':' + PORT + path - instrumentOutbound(agent, {host: HOSTNAME, port: PORT}, makeFakeRequest) + instrumentOutbound(agent, undefined, {host: HOSTNAME, port: PORT}, makeFakeRequest) expect(transaction.trace.root.children[0].name).equal(name) function makeFakeRequest() { @@ -127,7 +128,7 @@ describe('instrumentOutbound', function() { var req = new events.EventEmitter() helper.runInTransaction(agent, function(transaction) { agent.config.attributes.enabled = true - instrumentOutbound(agent, {host: HOSTNAME, port: PORT}, makeFakeRequest) + instrumentOutbound(agent, undefined, {host: HOSTNAME, port: PORT}, makeFakeRequest) expect(transaction.trace.root.children[0].getAttributes()).to.deep.equal({ 'url': `http://${HOSTNAME}:${PORT}/asdf`, 'procedure': 'GET', @@ -148,7 +149,12 @@ describe('instrumentOutbound', function() { var req = new events.EventEmitter() helper.runInTransaction(agent, function() { expect(function() { - instrumentOutbound(agent, {host: HOSTNAME, port: PORT}, makeFakeRequest) + instrumentOutbound( + agent, + undefined, + {host: HOSTNAME, port: PORT}, + makeFakeRequest + ) }).to.throw(Error) }) @@ -163,7 +169,7 @@ describe('instrumentOutbound', function() { helper.runInTransaction(agent, function(transaction) { var name = NAMES.EXTERNAL.PREFIX + HOSTNAME + ':' + PORT + path req.path = path - instrumentOutbound(agent, {host: HOSTNAME, port: PORT}, makeFakeRequest) + instrumentOutbound(agent, undefined, {host: HOSTNAME, port: PORT}, makeFakeRequest) expect(transaction.trace.root.children[0].name).equal(name) }) @@ -179,7 +185,7 @@ describe('instrumentOutbound', function() { helper.runInTransaction(agent, function(transaction) { var name = NAMES.EXTERNAL.PREFIX + HOSTNAME + ':' + PORT + '/newrelic' req.path = path - instrumentOutbound(agent, {host: HOSTNAME, port: PORT}, makeFakeRequest) + instrumentOutbound(agent, undefined, {host: HOSTNAME, port: PORT}, makeFakeRequest) expect(transaction.trace.root.children[0].name).equal(name) }) @@ -195,7 +201,7 @@ describe('instrumentOutbound', function() { helper.runInTransaction(agent, function() { let req2 = null expect(() => { - req2 = instrumentOutbound(agent, {port: PORT}, makeFakeRequest) + req2 = instrumentOutbound(agent, undefined, {port: PORT}, makeFakeRequest) }).to.not.throw() expect(req2).to.equal(req).and.not.have.property('__NR_transactionInfo') @@ -213,7 +219,12 @@ describe('instrumentOutbound', function() { helper.runInTransaction(agent, function() { let req2 = null expect(() => { - req2 = instrumentOutbound(agent, {host: null, port: PORT}, makeFakeRequest) + req2 = instrumentOutbound( + agent, + undefined, + {host: null, port: PORT}, + makeFakeRequest + ) }).to.not.throw() expect(req2).to.equal(req).and.not.have.property('__NR_transactionInfo') @@ -230,7 +241,12 @@ describe('instrumentOutbound', function() { helper.runInTransaction(agent, function() { let req2 = null expect(() => { - req2 = instrumentOutbound(agent, {host: '', port: PORT}, makeFakeRequest) + req2 = instrumentOutbound( + agent, + undefined, + {host: '', port: PORT}, + makeFakeRequest + ) }).to.not.throw() expect(req2).to.equal(req).and.not.have.property('__NR_transactionInfo') @@ -248,7 +264,7 @@ describe('instrumentOutbound', function() { helper.runInTransaction(agent, function() { let req2 = null expect(() => { - req2 = instrumentOutbound(agent, {host: 'hostname'}, makeFakeRequest) + req2 = instrumentOutbound(agent, undefined, {host: 'hostname'}, makeFakeRequest) }).to.not.throw() expect(req2).to.equal(req).and.not.have.property('__NR_transactionInfo') @@ -572,3 +588,81 @@ describe('when working with http.request', function() { }) }) }) + +describe("node >= v10 api", () => { + it('http.get', function(done) { + const host = 'http://www.google.com' + const path = `/index.html` + const _url = `${host}${path}` + + nock(host).get(path).reply(200, 'Hello from Google') + + http.get( + global.URL ? new global.URL(_url) : _url, + { + headers: { test: 'test' } + }, + (res) => { + expect(res.statusCode).to.equal(200) + expect(res.req.headers.test).to.equal('test') + done() + }) + }) + + it('http.request', function(done) { + const host = 'http://www.google.com' + const path = `/index.html` + const _url = `${host}${path}` + + nock(host).get(path).reply(200, 'Hello from Google') + + http.request( + global.URL ? new global.URL(_url) : _url, + { + headers: { test: 'test' } + }, + (res) => { + expect(res.statusCode).to.equal(200) + expect(res.req.headers.test).to.equal('test') + done() + }).end() + }) + + it('https.get', function(done) { + const host = 'https://www.google.com' + const path = `/index.html` + const _url = `${host}${path}` + + nock(host).get(path).reply(200, 'Hello from Google') + + https.get( + global.URL ? new global.URL(_url) : _url, + { + headers: { test: 'test' } + }, + (res) => { + expect(res.statusCode).to.equal(200) + expect(res.req.headers.test).to.equal('test') + done() + }) + }) + + it('https.request', function(done) { + const host = 'https://www.google.com' + const path = `/index.html` + const _url = `${host}${path}` + + nock(host).get(path).reply(200, 'Hello from Google') + + https.request( + global.URL ? new global.URL(_url) : _url, + { + headers: { test: 'test' } + }, + (res) => { + expect(res.statusCode).to.equal(200) + expect(res.req.headers.test).to.equal('test') + done() + }).end() + }) +})