From 546e0a05db8bb4180468b4f9d03435840fb67a4f Mon Sep 17 00:00:00 2001 From: Fatih Aydilek Date: Wed, 7 Aug 2019 10:24:41 +0300 Subject: [PATCH 1/4] Set HTTP span as erroneous when the response code is 4XX 5xx Node.js --- src/plugins/integrations/HttpIntegration.ts | 5 +++ test/integration/http.integration.test.js | 35 ++++++++++++++++++- .../utils/http.integration.utils.js | 18 ++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/plugins/integrations/HttpIntegration.ts b/src/plugins/integrations/HttpIntegration.ts index 2cca629d..2ad63f87 100644 --- a/src/plugins/integrations/HttpIntegration.ts +++ b/src/plugins/integrations/HttpIntegration.ts @@ -9,6 +9,7 @@ import Utils from '../utils/Utils'; import * as url from 'url'; import ThundraLogger from '../../ThundraLogger'; import InvocationSupport from '../support/InvocationSupport'; +import HttpError from '../error/HttpError'; const shimmer = require('shimmer'); const has = require('lodash.has'); @@ -136,6 +137,10 @@ class HttpIntegration implements Integration { const resourceName: string = res.headers[TriggerHeaderTags.RESOURCE_NAME]; span._setOperationName(resourceName); } + const statusCode = res.statusCode.toString(); + if (statusCode.startsWith('4') || statusCode.startsWith('5')) { + span.setErrorTag(new HttpError(res.statusMessage)); + } span.setTag(HttpTags.HTTP_STATUS, res.statusCode); }); diff --git a/test/integration/http.integration.test.js b/test/integration/http.integration.test.js index 33c93233..2e116863 100644 --- a/test/integration/http.integration.test.js +++ b/test/integration/http.integration.test.js @@ -16,7 +16,7 @@ describe('HTTP integration', () => { return Http.get(sdk).then(() => { const span = tracer.getRecorder().spanList[0]; - expect(span.operationName).toBe('jsonplaceholder.typicode.com/users/1') + expect(span.operationName).toBe('jsonplaceholder.typicode.com/users/1'); expect(span.className).toBe('HTTP'); expect(span.domainName).toBe('API'); @@ -35,6 +35,39 @@ describe('HTTP integration', () => { }); }); + test('should set 4XX 5XX errors on HTTP calls', () => { + const integration = new HttpIntegration({ + httpPathDepth: 2, + }); + const sdk = require('http'); + + const tracer = new ThundraTracer(); + InvocationSupport.setFunctionName('functionName'); + + return Http.getError(sdk).then(() => { + const span = tracer.getRecorder().spanList[0]; + + expect(span.operationName).toBe('httpstat.us/404'); + expect(span.className).toBe('HTTP'); + expect(span.domainName).toBe('API'); + + expect(span.tags['operation.type']).toBe('GET'); + expect(span.tags['http.method']).toBe('GET'); + expect(span.tags['http.host']).toBe('httpstat.us'); + expect(span.tags['http.path']).toBe('/404'); + expect(span.tags['http.url']).toBe('httpstat.us/404'); + expect(span.tags['http.status_code']).toBe(404); + expect(span.tags['error']).toBe(true); + expect(span.tags['error.kind']).toBe('HttpError'); + expect(span.tags['error.message']).toBe('Not Found'); + expect(span.tags['topology.vertex']).toEqual(true); + expect(span.tags['trigger.domainName']).toEqual('API'); + expect(span.tags['trigger.className']).toEqual('AWS-Lambda'); + expect(span.tags['trigger.operationNames']).toEqual(['functionName']); + expect(span.tags['http.body']).not.toBeTruthy(); + }); + }); + test('should instrument HTTPS POST calls', () => { const integration = new HttpIntegration({ httpPathDepth: 0, diff --git a/test/integration/utils/http.integration.utils.js b/test/integration/utils/http.integration.utils.js index a55b8968..48618e47 100644 --- a/test/integration/utils/http.integration.utils.js +++ b/test/integration/utils/http.integration.utils.js @@ -16,6 +16,24 @@ module.exports.get = (http) => { }); }; +module.exports.getError = (http) => { + return new Promise((resolve) => { + const url = 'http://httpstat.us/404'; + http.get(url, (resp) => { + let data = ''; + resp.on('data', (chunk) => { + data += chunk; + }); + resp.on('end', () => { + resolve(data); + }); + }).on('error', (err) => { + // We resolve with error. + resolve(err); + }); + }); +}; + module.exports.post = (https) => { return new Promise((resolve) => { const data = JSON.stringify({ From bcbef8945150d6be17765eb7bc39d767d87fa437 Mon Sep 17 00:00:00 2001 From: Fatih Aydilek Date: Wed, 7 Aug 2019 13:51:45 +0300 Subject: [PATCH 2/4] Add env variable for disabling Http 4xx 5xx error. --- src/Constants.ts | 3 ++ src/plugins/config/TraceConfig.ts | 10 ++++++ src/plugins/integrations/HttpIntegration.ts | 6 +++- test/integration/http.integration.test.js | 37 ++++++++++++++++++++- 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/Constants.ts b/src/Constants.ts index 283d8e21..2af5495c 100644 --- a/src/Constants.ts +++ b/src/Constants.ts @@ -80,6 +80,9 @@ export const envVariableKeys = { THUNDRA_AGENT_COUNT_AWARE_SAMPLER_COUNT_FREQ: 'thundra_agent_lambda_sampler_countAware_countFreq', THUNDRA_AWS_INSTRUMENT_ON_LOAD: 'thundra_agent_lambda_trace_integrations_aws_instrument_onLoad', + + THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_4XX: 'thundra_agent_trace_integrations_http_set_error_on_4xx_response_code_disable', + THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_5XX: 'thundra_agent_trace_integrations_http_set_error_on_5xx_response_code_disable', }; export function getTimeoutMargin(region: string) { diff --git a/src/plugins/config/TraceConfig.ts b/src/plugins/config/TraceConfig.ts index 8b0e435f..a06a335a 100644 --- a/src/plugins/config/TraceConfig.ts +++ b/src/plugins/config/TraceConfig.ts @@ -18,6 +18,8 @@ class TraceConfig extends BasePluginConfig { maskResponse: (response: any) => any; disabledIntegrations: IntegrationConfig[]; disableInstrumentation: boolean; + disableHttp4xxError: boolean; + disableHttp5xxError: boolean; integrationsMap: Map; instrumenter: Instrumenter; maskRedisStatement: boolean; @@ -57,6 +59,14 @@ class TraceConfig extends BasePluginConfig { envVariableKeys.THUNDRA_LAMBDA_TRACE_INSTRUMENT_DISABLE) ? Utils.getConfiguration( envVariableKeys.THUNDRA_LAMBDA_TRACE_INSTRUMENT_DISABLE) === 'true' : options.disableInstrumentation; + this.disableHttp4xxError = Utils.getConfiguration( + envVariableKeys.THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_4XX) ? Utils.getConfiguration( + envVariableKeys.THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_4XX) === 'true' : options.disableHttp4xxError; + + this.disableHttp5xxError = Utils.getConfiguration( + envVariableKeys.THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_5XX) ? Utils.getConfiguration( + envVariableKeys.THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_5XX) === 'true' : options.disableHttp5xxError; + this.maskRedisStatement = Utils.getConfiguration( envVariableKeys.THUNDRA_MASK_REDIS_STATEMENT) ? Utils.getConfiguration( envVariableKeys.THUNDRA_MASK_REDIS_STATEMENT) === 'true' : options.maskRedisStatement; diff --git a/src/plugins/integrations/HttpIntegration.ts b/src/plugins/integrations/HttpIntegration.ts index 2ad63f87..650db266 100644 --- a/src/plugins/integrations/HttpIntegration.ts +++ b/src/plugins/integrations/HttpIntegration.ts @@ -62,6 +62,7 @@ class HttpIntegration implements Integration { const libHTTPS = lib[1]; const nodeVersion = process.version; const plugin = this; + function wrapper(request: any) { return function requestWrapper(options: any, callback: any) { try { @@ -138,7 +139,10 @@ class HttpIntegration implements Integration { span._setOperationName(resourceName); } const statusCode = res.statusCode.toString(); - if (statusCode.startsWith('4') || statusCode.startsWith('5')) { + if (!config.disableHttp5xxError && statusCode.startsWith('5')) { + span.setErrorTag(new HttpError(res.statusMessage)); + } + if (!config.disableHttp4xxError && statusCode.startsWith('4')) { span.setErrorTag(new HttpError(res.statusMessage)); } span.setTag(HttpTags.HTTP_STATUS, res.statusCode); diff --git a/test/integration/http.integration.test.js b/test/integration/http.integration.test.js index 2e116863..57da513f 100644 --- a/test/integration/http.integration.test.js +++ b/test/integration/http.integration.test.js @@ -46,7 +46,7 @@ describe('HTTP integration', () => { return Http.getError(sdk).then(() => { const span = tracer.getRecorder().spanList[0]; - + console.log(span); expect(span.operationName).toBe('httpstat.us/404'); expect(span.className).toBe('HTTP'); expect(span.domainName).toBe('API'); @@ -68,6 +68,41 @@ describe('HTTP integration', () => { }); }); + test('should disable 4XX 5XX errors on HTTP calls', () => { + const integration = new HttpIntegration({ + httpPathDepth: 2, + disableHttp4xxError:true + + }); + const sdk = require('http'); + + const tracer = new ThundraTracer(); + InvocationSupport.setFunctionName('functionName'); + + return Http.getError(sdk).then(() => { + const span = tracer.getRecorder().spanList[0]; + console.log(span); + expect(span.operationName).toBe('httpstat.us/404'); + expect(span.className).toBe('HTTP'); + expect(span.domainName).toBe('API'); + + expect(span.tags['operation.type']).toBe('GET'); + expect(span.tags['http.method']).toBe('GET'); + expect(span.tags['http.host']).toBe('httpstat.us'); + expect(span.tags['http.path']).toBe('/404'); + expect(span.tags['http.url']).toBe('httpstat.us/404'); + expect(span.tags['http.status_code']).toBe(404); + expect(span.tags['error']).toBe(undefined); + expect(span.tags['error.kind']).toBe(undefined); + expect(span.tags['error.message']).toBe(undefined); + expect(span.tags['topology.vertex']).toEqual(true); + expect(span.tags['trigger.domainName']).toEqual('API'); + expect(span.tags['trigger.className']).toEqual('AWS-Lambda'); + expect(span.tags['trigger.operationNames']).toEqual(['functionName']); + expect(span.tags['http.body']).not.toBeTruthy(); + }); + }); + test('should instrument HTTPS POST calls', () => { const integration = new HttpIntegration({ httpPathDepth: 0, From d2ddf47c09ebb29e1080f25af9072ba4f0e269a1 Mon Sep 17 00:00:00 2001 From: Fatih Aydilek Date: Wed, 7 Aug 2019 14:01:03 +0300 Subject: [PATCH 3/4] fix eslint max length error --- src/Constants.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Constants.ts b/src/Constants.ts index 2af5495c..1c10ea3e 100644 --- a/src/Constants.ts +++ b/src/Constants.ts @@ -81,8 +81,10 @@ export const envVariableKeys = { THUNDRA_AWS_INSTRUMENT_ON_LOAD: 'thundra_agent_lambda_trace_integrations_aws_instrument_onLoad', - THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_4XX: 'thundra_agent_trace_integrations_http_set_error_on_4xx_response_code_disable', - THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_5XX: 'thundra_agent_trace_integrations_http_set_error_on_5xx_response_code_disable', + THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_4XX: + 'thundra_agent_trace_integrations_http_set_error_on_4xx_response_code_disable', + THUNDRA_AGENT_TRACE_INTEGRATION_HTTP_ERROR_ON_5XX: + 'thundra_agent_trace_integrations_http_set_error_on_5xx_response_code_disable', }; export function getTimeoutMargin(region: string) { From 97b71f0969b569bb55c306e50e5ae36010da69e2 Mon Sep 17 00:00:00 2001 From: Fatih Aydilek Date: Thu, 8 Aug 2019 10:24:30 +0300 Subject: [PATCH 4/4] delete unnecessary console.log --- test/integration/http.integration.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/integration/http.integration.test.js b/test/integration/http.integration.test.js index 57da513f..52917840 100644 --- a/test/integration/http.integration.test.js +++ b/test/integration/http.integration.test.js @@ -46,7 +46,6 @@ describe('HTTP integration', () => { return Http.getError(sdk).then(() => { const span = tracer.getRecorder().spanList[0]; - console.log(span); expect(span.operationName).toBe('httpstat.us/404'); expect(span.className).toBe('HTTP'); expect(span.domainName).toBe('API'); @@ -81,7 +80,6 @@ describe('HTTP integration', () => { return Http.getError(sdk).then(() => { const span = tracer.getRecorder().spanList[0]; - console.log(span); expect(span.operationName).toBe('httpstat.us/404'); expect(span.className).toBe('HTTP'); expect(span.domainName).toBe('API');