diff --git a/packages/collector/test/integration/currencies/protocols/http/client/test_base.js b/packages/collector/test/integration/currencies/protocols/http/client/test_base.js index 2fe0dfc0b1..b84ff432b3 100644 --- a/packages/collector/test/integration/currencies/protocols/http/client/test_base.js +++ b/packages/collector/test/integration/currencies/protocols/http/client/test_base.js @@ -1226,7 +1226,9 @@ module.exports = function (name, version, isLatest) { 'x-my-exit-options-request-multi-header': 'x-my-exit-options-request-multi-header-value-1, x-my-exit-options-request-multi-header-value-2' }), - span => expect(span.data.http.url).to.match(/\/request-only-opts/) + span => expect(span.data.http.url).to.match(/\/request-only-opts/), + span => expect(span.data.http.requestHeader).to.not.exist, + span => expect(span.data.http.responseHeader).to.not.exist ]); }) ) @@ -1250,7 +1252,9 @@ module.exports = function (name, version, isLatest) { 'x-my-exit-set-on-request-multi-header': 'x-my-exit-set-on-request-multi-header-value-1, x-my-exit-set-on-request-multi-header-value-2' }), - span => expect(span.data.http.url).to.match(/\/request-only-opts/) + span => expect(span.data.http.url).to.match(/\/request-only-opts/), + span => expect(span.data.http.requestHeader).to.not.exist, + span => expect(span.data.http.responseHeader).to.not.exist ]); }) ) @@ -1273,7 +1277,9 @@ module.exports = function (name, version, isLatest) { expect(span.data.http.header['x-my-exit-response-header']).to.equal( 'x-my-exit-response-header-value' ), - span => expect(span.data.http.url).to.match(/\/request-only-opts/) + span => expect(span.data.http.url).to.match(/\/request-only-opts/), + span => expect(span.data.http.requestHeader).to.not.exist, + span => expect(span.data.http.responseHeader).to.not.exist ]); }) ) diff --git a/packages/collector/test/integration/currencies/protocols/http/native_fetch/test_base.js b/packages/collector/test/integration/currencies/protocols/http/native_fetch/test_base.js index 477d7ba32d..9b2b7f6b17 100644 --- a/packages/collector/test/integration/currencies/protocols/http/native_fetch/test_base.js +++ b/packages/collector/test/integration/currencies/protocols/http/native_fetch/test_base.js @@ -1137,6 +1137,8 @@ module.exports = function (name, version, isLatest) { } else { expect(span.data.http.header).to.not.exist; } + expect(span.data.http.requestHeader).to.not.exist; + expect(span.data.http.responseHeader).to.not.exist; } function verifyHttpExitEc({ spans, expectedEc, expectedStatus }) { diff --git a/packages/collector/test/integration/currencies/protocols/http/server/test_base.js b/packages/collector/test/integration/currencies/protocols/http/server/test_base.js index bf073f6d25..51cf831603 100644 --- a/packages/collector/test/integration/currencies/protocols/http/server/test_base.js +++ b/packages/collector/test/integration/currencies/protocols/http/server/test_base.js @@ -293,6 +293,8 @@ module.exports = function (name, version, isLatest) { 'x-my-entry-request-multi-header': 'value1,value2' }); } + expect(span.data.http.requestHeader).to.not.exist; + expect(span.data.http.responseHeader).to.not.exist; }) ) ); @@ -314,6 +316,8 @@ module.exports = function (name, version, isLatest) { 'x-my-entry-response-header': expectedResponeHeaderValue, 'x-my-entry-response-multi-header': 'value1, value2' }); + expect(span.data.http.requestHeader).to.not.exist; + expect(span.data.http.responseHeader).to.not.exist; }) ) ); @@ -336,6 +340,8 @@ module.exports = function (name, version, isLatest) { 'x-write-head-response-header': expectedResponeHeaderValue, 'x-write-head-response-multi-header': 'value1, value2' }); + expect(span.data.http.requestHeader).to.not.exist; + expect(span.data.http.responseHeader).to.not.exist; }) ) ); @@ -374,6 +380,8 @@ module.exports = function (name, version, isLatest) { 'x-my-entry-response-multi-header': 'value1, value2' }); } + expect(span.data.http.requestHeader).to.not.exist; + expect(span.data.http.responseHeader).to.not.exist; }) ) ); @@ -420,6 +428,8 @@ module.exports = function (name, version, isLatest) { 'x-write-head-response-multi-header': 'value1, value2' }); } + expect(span.data.http.requestHeader).to.not.exist; + expect(span.data.http.responseHeader).to.not.exist; }) ) ); @@ -437,6 +447,8 @@ module.exports = function (name, version, isLatest) { agentControls.getSpans().then(spans => { const span = verifyThereIsExactlyOneHttpEntry(spans, controls, '/', 'GET', 200, false, false); expect(span.data.http.header).to.not.exist; + expect(span.data.http.requestHeader).to.not.exist; + expect(span.data.http.responseHeader).to.not.exist; }) ) )); diff --git a/packages/core/src/tracing/backend_mappers/mapper.js b/packages/core/src/tracing/backend_mappers/mapper.js index 3e5ede3272..3482acc2d5 100644 --- a/packages/core/src/tracing/backend_mappers/mapper.js +++ b/packages/core/src/tracing/backend_mappers/mapper.js @@ -36,7 +36,10 @@ const fieldMappings = { http: { operation: 'method', endpoints: 'url', - connection: 'host' + connection: 'host', + // The null mapping indicates the field is internal-only and will be dropped + requestHeader: null, + responseHeader: null } }; diff --git a/packages/core/src/tracing/instrumentation/protocols/http2Client.js b/packages/core/src/tracing/instrumentation/protocols/http2Client.js index 4fe996028c..0339e8fe70 100644 --- a/packages/core/src/tracing/instrumentation/protocols/http2Client.js +++ b/packages/core/src/tracing/instrumentation/protocols/http2Client.js @@ -101,7 +101,10 @@ function instrumentClientHttp2Session(clientHttp2Session) { const origin = readSymbolProperty(stream, originS); const reqHeaders = readSymbolProperty(stream, sentHeadersS); - let capturedHeaders = getExtraHeadersCaseInsensitive(reqHeaders, extraHttpHeadersToCapture); + const requestHeader = getExtraHeadersCaseInsensitive(reqHeaders, extraHttpHeadersToCapture); + + let responseHeader; + let combinedHeader = requestHeader; let method; let path; @@ -126,8 +129,11 @@ function instrumentClientHttp2Session(clientHttp2Session) { stream.on('response', resHeaders => { status = resHeaders[HTTP2_HEADER_STATUS]; - capturedHeaders = mergeExtraHeadersFromNormalizedObjectLiteral( - capturedHeaders, + + responseHeader = getExtraHeadersCaseInsensitive(resHeaders, extraHttpHeadersToCapture); + + combinedHeader = mergeExtraHeadersFromNormalizedObjectLiteral( + combinedHeader, resHeaders, extraHttpHeadersToCapture ); @@ -137,8 +143,16 @@ function instrumentClientHttp2Session(clientHttp2Session) { span.d = Date.now() - span.ts; span.ec = tracingUtil.shouldMarkAsError(status) ? 1 : 0; span.data.http.status = status; - if (capturedHeaders) { - span.data.http.header = capturedHeaders; + if (requestHeader) { + span.data.http.requestHeader = requestHeader; + } + + if (responseHeader) { + span.data.http.responseHeader = responseHeader; + } + + if (combinedHeader) { + span.data.http.header = combinedHeader; } span.transmit(); }); diff --git a/packages/core/src/tracing/instrumentation/protocols/http2Server.js b/packages/core/src/tracing/instrumentation/protocols/http2Server.js index 30cfefc4e3..81bd67685c 100644 --- a/packages/core/src/tracing/instrumentation/protocols/http2Server.js +++ b/packages/core/src/tracing/instrumentation/protocols/http2Server.js @@ -106,13 +106,16 @@ function shimEmit(realEmit) { if (pathParts.length >= 2) { pathParts[1] = filterParams(pathParts[1]); } + + const requestHeader = getExtraHeadersFromNormalizedObjectLiteral(headers, extraHttpHeadersToCapture); const spanData = { http: { operation: method, endpoints: sanitizeUrl(pathParts.shift()), params: pathParts.length > 0 ? pathParts.join('?') : undefined, connection: authority, - header: getExtraHeadersFromNormalizedObjectLiteral(headers, extraHttpHeadersToCapture) + header: requestHeader, + requestHeader } }; @@ -167,8 +170,12 @@ function shimEmit(realEmit) { // take over the span) but did not actually transmit this span. span.data.http = span.data.http || {}; span.data.http.status = status; + + const responseHeader = getExtraHeadersFromNormalizedObjectLiteral(resHeaders, extraHttpHeadersToCapture); + span.data.http.responseHeader = responseHeader; + span.data.http.header = mergeExtraHeadersCaseInsensitive( - span.data.http.header, + span.data.http.requestHeader, resHeaders, extraHttpHeadersToCapture ); diff --git a/packages/core/src/tracing/instrumentation/protocols/httpClient.js b/packages/core/src/tracing/instrumentation/protocols/httpClient.js index e1c18b2f37..4bf2e537b7 100644 --- a/packages/core/src/tracing/instrumentation/protocols/httpClient.js +++ b/packages/core/src/tracing/instrumentation/protocols/httpClient.js @@ -229,6 +229,8 @@ function instrument(coreModule, forceHttps) { span.stack = tracingUtil.getStackTrace(request); + let requestHeader; + const boundCallback = cls.ns.bind(function boundCallback(res) { span.data.http = { operation: clientRequest.method, @@ -237,9 +239,21 @@ function instrument(coreModule, forceHttps) { params }; - const headers = captureRequestHeaders(options, clientRequest, res); - if (headers) { - span.data.http.header = headers; + const responseHeader = captureResponseHeaders(res); + const combinedHeader = mergeExtraHeadersFromIncomingMessage( + requestHeader ? Object.assign({}, requestHeader) : undefined, + res, + extraHttpHeadersToCapture + ); + + if (requestHeader) { + span.data.http.requestHeader = requestHeader; + } + if (responseHeader) { + span.data.http.responseHeader = responseHeader; + } + if (combinedHeader) { + span.data.http.header = combinedHeader; } span.d = Date.now() - span.ts; @@ -262,6 +276,8 @@ function instrument(coreModule, forceHttps) { try { instanaHeadersHaveBeenAdded = tryToAddHeadersToOpts(options, span, w3cTraceContext); clientRequest = originalRequest.apply(coreModule, originalArgs); + // Capture outgoing request headers now that clientRequest exists and before any async response arrives. + requestHeader = captureRequestHeaders(options, clientRequest); removeInstanaHeadersFromOpts(options); } catch (e) { removeInstanaHeadersFromOpts(options); @@ -466,9 +482,12 @@ function setW3cHeadersOnRequest(clientRequest, w3cTraceContext) { } } -function captureRequestHeaders(options, clientRequest, response) { +function captureRequestHeaders(options, clientRequest) { let headers = getExtraHeadersFromOptions(options, extraHttpHeadersToCapture); headers = mergeExtraHeadersFromServerResponseOrClientRequest(headers, clientRequest, extraHttpHeadersToCapture); - headers = mergeExtraHeadersFromIncomingMessage(headers, response, extraHttpHeadersToCapture); return headers; } + +function captureResponseHeaders(response) { + return mergeExtraHeadersFromIncomingMessage(undefined, response, extraHttpHeadersToCapture); +} diff --git a/packages/core/src/tracing/instrumentation/protocols/httpServer.js b/packages/core/src/tracing/instrumentation/protocols/httpServer.js index 5ebc25227c..a0d790fb0a 100644 --- a/packages/core/src/tracing/instrumentation/protocols/httpServer.js +++ b/packages/core/src/tracing/instrumentation/protocols/httpServer.js @@ -86,13 +86,16 @@ function shimEmit(realEmit) { if (urlParts.length >= 2) { urlParts[1] = filterParams(urlParts[1]); } + + const requestHeader = getExtraHeadersFromMessage(req, extraHttpHeadersToCapture); const spanData = { http: { operation: req.method, endpoints: sanitizeUrl(urlParts.shift()), params: urlParts.length > 0 ? urlParts.join('?') : undefined, connection: req.headers.host, - header: getExtraHeadersFromMessage(req, extraHttpHeadersToCapture) + header: requestHeader ? Object.assign({}, requestHeader) : undefined, + requestHeader } }; @@ -183,6 +186,11 @@ function shimEmit(realEmit) { res, extraHttpHeadersToCapture ); + span.data.http.responseHeader = mergeExtraHeadersFromServerResponseOrClientRequest( + undefined, + res, + extraHttpHeadersToCapture + ); } if (!span.postponeTransmit) { diff --git a/packages/core/src/tracing/instrumentation/protocols/nativeFetch.js b/packages/core/src/tracing/instrumentation/protocols/nativeFetch.js index c857f55bfe..4aa473be2a 100644 --- a/packages/core/src/tracing/instrumentation/protocols/nativeFetch.js +++ b/packages/core/src/tracing/instrumentation/protocols/nativeFetch.js @@ -117,6 +117,7 @@ function instrument() { const resource = originalArgs[0]; let params; let capturedHeaders; + let capturedRequestHeaders; if (resource != null) { let rawUrl; @@ -127,6 +128,7 @@ function instrument() { rawUrl = resource.url; method = resource.method; capturedHeaders = getExtraHeadersFromFetchHeaders(resource.headers, extraHttpHeadersToCapture); + capturedRequestHeaders = capturedHeaders ? Object.assign({}, capturedHeaders) : undefined; } else if (typeof resource.toString === 'function') { // This also handles the case when the resource is a URL object, as well as any object that has a custom // stringifier. @@ -153,6 +155,7 @@ function instrument() { } else { capturedHeaders = getExtraHeadersCaseInsensitive(options.headers, extraHttpHeadersToCapture); } + capturedRequestHeaders = capturedHeaders ? Object.assign({}, capturedHeaders) : undefined; } } @@ -171,6 +174,12 @@ function instrument() { .then(response => { span.data.http.status = response.status; span.ec = tracingUtil.shouldMarkAsError(response.status) ? 1 : 0; + + const capturedResponseHeaders = mergeExtraHeadersFromFetchHeaders( + undefined, + response.headers, + extraHttpHeadersToCapture + ); capturedHeaders = mergeExtraHeadersFromFetchHeaders( capturedHeaders, response.headers, @@ -181,6 +190,12 @@ function instrument() { if (capturedHeaders != null && Object.keys(capturedHeaders).length > 0) { span.data.http.header = capturedHeaders; } + if (capturedRequestHeaders != null && Object.keys(capturedRequestHeaders).length > 0) { + span.data.http.requestHeader = capturedRequestHeaders; + } + if (capturedResponseHeaders != null && Object.keys(capturedResponseHeaders).length > 0) { + span.data.http.responseHeader = capturedResponseHeaders; + } span.transmit(); }) .catch(err => { diff --git a/packages/core/test/tracing/backend_mappers/mapper_test.js b/packages/core/test/tracing/backend_mappers/mapper_test.js index eb1ac41715..1faeb64e78 100644 --- a/packages/core/test/tracing/backend_mappers/mapper_test.js +++ b/packages/core/test/tracing/backend_mappers/mapper_test.js @@ -327,5 +327,38 @@ describe('tracing/backend_mappers', () => { const result = transform(span); expect(result).to.deep.equal(span); }); + + it('should strip requestHeader and responseHeader from http span data', () => { + span = { + n: 'node.http.server', + data: { + http: { + operation: 'GET', + endpoints: '/api/users', + connection: 'localhost', + status: 200, + header: { 'content-type': 'application/json' }, + requestHeader: { accept: 'application/json' }, + responseHeader: { 'cache-control': 'no-cache' } + } + } + }; + + const result = transform(span); + + expect(result.data.http).to.deep.equal({ + method: 'GET', + url: '/api/users', + host: 'localhost', + status: 200, + header: { 'content-type': 'application/json' } + }); + + expect(result.data.http).to.not.have.property('operation'); + expect(result.data.http).to.not.have.property('endpoints'); + expect(result.data.http).to.not.have.property('connection'); + expect(result.data.http).to.not.have.property('requestHeader'); + expect(result.data.http).to.not.have.property('responseHeader'); + }); }); });