From b8b6bf4287f822969cf65cc60795e5fdaa91a25f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Einar=20Nor=C3=B0fj=C3=B6r=C3=B0?= Date: Tue, 15 Feb 2022 21:49:06 -0500 Subject: [PATCH 1/6] fix: http server error status according to the spec 400-499 should be left UNSET in case of server span kind specifically > For HTTP status codes in the 4xx range > span status MUST be left unset in case > of SpanKind.SERVER and MUST be set to > Error in case of SpanKind.CLIENT. --- .../src/http.ts | 9 ++- .../src/utils.ts | 45 +++++------- .../test/functionals/utils.test.ts | 73 +++++++++++++------ .../test/utils/assertSpan.ts | 2 +- 4 files changed, 78 insertions(+), 51 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index b031cb57fc..8765ca35cf 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -311,7 +311,7 @@ export class HttpInstrumentation extends InstrumentationBase { if (response.aborted && !response.complete) { status = { code: SpanStatusCode.ERROR }; } else { - status = utils.parseResponseStatus(response.statusCode); + status = utils.parseClientResponseStatus(response.statusCode); } span.setStatus(status); @@ -333,7 +333,8 @@ export class HttpInstrumentation extends InstrumentationBase { }); response.on('error', (error: Err) => { this._diag.debug('outgoingRequest on error()', error); - utils.setSpanWithError(span, error, response); + const statusCode = utils.parseClientResponseStatus(response.statusCode); + utils.setSpanWithError(span, error, statusCode.code); this._closeHttpSpan(span); }); } @@ -346,7 +347,7 @@ export class HttpInstrumentation extends InstrumentationBase { }); request.on('error', (error: Err) => { this._diag.debug('outgoingRequest on request error()', error); - utils.setSpanWithError(span, error, request); + utils.setSpanWithError(span, error); this._closeHttpSpan(span); }); @@ -470,7 +471,7 @@ export class HttpInstrumentation extends InstrumentationBase { span .setAttributes(attributes) - .setStatus(utils.parseResponseStatus(response.statusCode)); + .setStatus(utils.parseServerResponseStatus(response.statusCode)); if (instrumentation._getConfig().applyCustomAttributesOnSpan) { safeExecuteInTheMiddle( diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index 0a102ae688..068da3bbd0 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -25,7 +25,6 @@ import { SemanticAttributes, } from '@opentelemetry/semantic-conventions'; import { - ClientRequest, IncomingHttpHeaders, IncomingMessage, OutgoingHttpHeaders, @@ -65,14 +64,26 @@ export const getAbsoluteUrl = ( return `${protocol}//${host}${path}`; }; + +export const parseServerResponseStatus = (statusCode?: number): Omit => { + if (statusCode === undefined) { + return { code: SpanStatusCode.ERROR }; + } + + // 1xx, 2xx, 3xx, 4xx are OK from the server perspective + if (statusCode >= 100 && statusCode < 500) { + return { code: SpanStatusCode.OK }; + } + + // All other codes are error + return { code: SpanStatusCode.ERROR }; +}; + /** * Parse status code from HTTP response. [More details](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-http.md#status) */ -export const parseResponseStatus = ( - statusCode: number | undefined, -): Omit => { - - if(statusCode === undefined) { +export const parseClientResponseStatus = (statusCode?: number): Omit => { + if (statusCode === undefined) { return { code: SpanStatusCode.ERROR }; } @@ -142,12 +153,12 @@ export const isIgnored = ( * Sets the span with the error passed in params * @param {Span} span the span that need to be set * @param {Error} error error that will be set to span - * @param {(IncomingMessage | ClientRequest)} [obj] used for enriching the status by checking the statusCode. + * @param {SpanStatusCode} [code] used for overriding the status */ export const setSpanWithError = ( span: Span, error: Err, - obj?: IncomingMessage | ClientRequest + code = SpanStatusCode.ERROR ): void => { const message = error.message; @@ -156,23 +167,7 @@ export const setSpanWithError = ( [AttributeNames.HTTP_ERROR_MESSAGE]: message, }); - if (!obj) { - span.setStatus({ code: SpanStatusCode.ERROR, message }); - return; - } - - let status: SpanStatus; - if ((obj as IncomingMessage).statusCode) { - status = parseResponseStatus((obj as IncomingMessage).statusCode); - } else if ((obj as ClientRequest).aborted) { - status = { code: SpanStatusCode.ERROR }; - } else { - status = { code: SpanStatusCode.ERROR }; - } - - status.message = message; - - span.setStatus(status); + span.setStatus({ code, message }); }; /** diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index aa1715a5a0..f54ca9f963 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -35,9 +35,9 @@ import { RPCType, setRPCMetadata } from '@opentelemetry/core'; import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; describe('Utility', () => { - describe('parseResponseStatus()', () => { + describe('parseClientResponseStatus()', () => { it('should return ERROR code by default', () => { - const status = utils.parseResponseStatus( + const status = utils.parseClientResponseStatus( (undefined as unknown) as number ); assert.deepStrictEqual(status, { code: SpanStatusCode.ERROR }); @@ -45,14 +45,35 @@ describe('Utility', () => { it('should return OK for Success HTTP status code', () => { for (let index = 100; index < 400; index++) { - const status = utils.parseResponseStatus(index); + const status = utils.parseClientResponseStatus(index); assert.deepStrictEqual(status, { code: SpanStatusCode.OK }); } }); it('should not return OK for Bad HTTP status code', () => { for (let index = 400; index <= 600; index++) { - const status = utils.parseResponseStatus(index); + const status = utils.parseClientResponseStatus(index); + assert.notStrictEqual(status.code, SpanStatusCode.OK); + } + }); + }); + + describe('parseServerResponseStatus()', () => { + it('should return ERROR code by default', () => { + const status = utils.parseClientResponseStatus(undefined); + assert.deepStrictEqual(status, { code: SpanStatusCode.ERROR }); + }); + + it('should return OK for Success HTTP status code', () => { + for (let index = 100; index < 500; index++) { + const status = utils.parseClientResponseStatus(index); + assert.deepStrictEqual(status, { code: SpanStatusCode.OK }); + } + }); + + it('should not return OK for Bad HTTP status code', () => { + for (let index = 500; index <= 600; index++) { + const status = utils.parseClientResponseStatus(index); assert.notStrictEqual(status.code, SpanStatusCode.OK); } }); @@ -227,23 +248,33 @@ describe('Utility', () => { describe('setSpanWithError()', () => { it('should have error attributes', () => { const errorMessage = 'test error'; - for (const obj of [undefined, { statusCode: 400 }]) { - const span = new Span( - new BasicTracerProvider().getTracer('default'), - ROOT_CONTEXT, - 'test', - { spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED }, - SpanKind.INTERNAL - ); - /* tslint:disable-next-line:no-any */ - utils.setSpanWithError(span, new Error(errorMessage), obj as any); - const attributes = span.attributes; - assert.strictEqual( - attributes[AttributeNames.HTTP_ERROR_MESSAGE], - errorMessage - ); - assert.ok(attributes[AttributeNames.HTTP_ERROR_NAME]); - } + const span = new Span( + new BasicTracerProvider().getTracer('default'), + ROOT_CONTEXT, + 'test', + { spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED }, + SpanKind.INTERNAL + ); + /* tslint:disable-next-line:no-any */ + utils.setSpanWithError(span, new Error(errorMessage)); + const attributes = span.attributes; + assert.strictEqual( + attributes[AttributeNames.HTTP_ERROR_MESSAGE], + errorMessage + ); + assert.ok(attributes[AttributeNames.HTTP_ERROR_NAME]); + }); + it('can override the status', () => { + const span = new Span( + new BasicTracerProvider().getTracer('default'), + ROOT_CONTEXT, + 'test', + { spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED }, + SpanKind.INTERNAL + ); + /* tslint:disable-next-line:no-any */ + utils.setSpanWithError(span, new Error('error'), SpanStatusCode.UNSET); + assert.strictEqual(span.status, SpanStatusCode.UNSET); }); }); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts b/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts index b887a46649..2d7bae085a 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts @@ -70,7 +70,7 @@ export const assertSpan = ( assert.deepStrictEqual( span.status, validations.forceStatus || - utils.parseResponseStatus(validations.httpStatusCode) + utils.parseClientResponseStatus(validations.httpStatusCode) ); assert.ok(span.endTime, 'must be finished'); From 8f3f4a9e073f662fa690607f0389a2d48d8c648b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Einar=20Nor=C3=B0fj=C3=B6r=C3=B0?= Date: Tue, 15 Feb 2022 22:41:42 -0500 Subject: [PATCH 2/6] chore: simplify --- .../src/http.ts | 8 +-- .../src/utils.ts | 32 +++------ .../test/functionals/utils.test.ts | 70 +++++++------------ .../test/utils/assertSpan.ts | 2 +- 4 files changed, 37 insertions(+), 75 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 8765ca35cf..def1636adc 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -311,7 +311,7 @@ export class HttpInstrumentation extends InstrumentationBase { if (response.aborted && !response.complete) { status = { code: SpanStatusCode.ERROR }; } else { - status = utils.parseClientResponseStatus(response.statusCode); + status = { code: utils.parseResponseStatus(SpanKind.CLIENT, response.statusCode) }; } span.setStatus(status); @@ -333,8 +333,8 @@ export class HttpInstrumentation extends InstrumentationBase { }); response.on('error', (error: Err) => { this._diag.debug('outgoingRequest on error()', error); - const statusCode = utils.parseClientResponseStatus(response.statusCode); - utils.setSpanWithError(span, error, statusCode.code); + const code = utils.parseResponseStatus(SpanKind.CLIENT, response.statusCode); + utils.setSpanWithError(span, error, code); this._closeHttpSpan(span); }); } @@ -471,7 +471,7 @@ export class HttpInstrumentation extends InstrumentationBase { span .setAttributes(attributes) - .setStatus(utils.parseServerResponseStatus(response.statusCode)); + .setStatus({ code: utils.parseResponseStatus(SpanKind.SERVER, response.statusCode) }); if (instrumentation._getConfig().applyCustomAttributesOnSpan) { safeExecuteInTheMiddle( diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index 068da3bbd0..3786b0b44e 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -18,7 +18,7 @@ import { SpanStatusCode, Span, SpanStatus, - context, + context, SpanKind, } from '@opentelemetry/api'; import { NetTransportValues, @@ -65,35 +65,19 @@ export const getAbsoluteUrl = ( return `${protocol}//${host}${path}`; }; -export const parseServerResponseStatus = (statusCode?: number): Omit => { - if (statusCode === undefined) { - return { code: SpanStatusCode.ERROR }; - } - - // 1xx, 2xx, 3xx, 4xx are OK from the server perspective - if (statusCode >= 100 && statusCode < 500) { - return { code: SpanStatusCode.OK }; - } - - // All other codes are error - return { code: SpanStatusCode.ERROR }; -}; - /** * Parse status code from HTTP response. [More details](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-http.md#status) */ -export const parseClientResponseStatus = (statusCode?: number): Omit => { - if (statusCode === undefined) { - return { code: SpanStatusCode.ERROR }; - } - - // 1xx, 2xx, 3xx are OK - if (statusCode >= 100 && statusCode < 400) { - return { code: SpanStatusCode.OK }; +export const parseResponseStatus = (kind: SpanKind, statusCode?: number): SpanStatusCode => { + const upperBound = kind === SpanKind.CLIENT ? 400 : 500; + // 1xx, 2xx, 3xx are OK on client and server + // 4xx is OK on server + if (statusCode && statusCode >= 100 && statusCode < upperBound) { + return SpanStatusCode.UNSET; } // All other codes are error - return { code: SpanStatusCode.ERROR }; + return SpanStatusCode.ERROR; }; /** diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index f54ca9f963..79fddc6267 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -13,68 +13,46 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { - SpanAttributes, - SpanStatusCode, - ROOT_CONTEXT, - SpanKind, - TraceFlags, - context, -} from '@opentelemetry/api'; -import { BasicTracerProvider, Span } from '@opentelemetry/sdk-trace-base'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import {context, ROOT_CONTEXT, SpanAttributes, SpanKind, SpanStatusCode, TraceFlags,} from '@opentelemetry/api'; +import {BasicTracerProvider, Span} from '@opentelemetry/sdk-trace-base'; +import {SemanticAttributes} from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; -import { IncomingMessage, ServerResponse } from 'http'; -import { Socket } from 'net'; +import {IncomingMessage, ServerResponse} from 'http'; +import {Socket} from 'net'; import * as sinon from 'sinon'; import * as url from 'url'; -import { IgnoreMatcher } from '../../src/types'; +import {IgnoreMatcher} from '../../src/types'; import * as utils from '../../src/utils'; -import { AttributeNames } from '../../src/enums/AttributeNames'; -import { RPCType, setRPCMetadata } from '@opentelemetry/core'; -import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; +import {AttributeNames} from '../../src/enums/AttributeNames'; +import {RPCType, setRPCMetadata} from '@opentelemetry/core'; +import {AsyncHooksContextManager} from '@opentelemetry/context-async-hooks'; describe('Utility', () => { - describe('parseClientResponseStatus()', () => { + describe('parseResponseStatus()', () => { it('should return ERROR code by default', () => { - const status = utils.parseClientResponseStatus( - (undefined as unknown) as number - ); - assert.deepStrictEqual(status, { code: SpanStatusCode.ERROR }); + const status = utils.parseResponseStatus(SpanKind.CLIENT, undefined); + assert.deepStrictEqual(status, SpanStatusCode.ERROR); }); - it('should return OK for Success HTTP status code', () => { + it('should return UNSET for Success HTTP status code', () => { for (let index = 100; index < 400; index++) { - const status = utils.parseClientResponseStatus(index); - assert.deepStrictEqual(status, { code: SpanStatusCode.OK }); + const status = utils.parseResponseStatus(SpanKind.CLIENT, index); + assert.deepStrictEqual(status, SpanStatusCode.UNSET); } - }); - - it('should not return OK for Bad HTTP status code', () => { - for (let index = 400; index <= 600; index++) { - const status = utils.parseClientResponseStatus(index); - assert.notStrictEqual(status.code, SpanStatusCode.OK); - } - }); - }); - - describe('parseServerResponseStatus()', () => { - it('should return ERROR code by default', () => { - const status = utils.parseClientResponseStatus(undefined); - assert.deepStrictEqual(status, { code: SpanStatusCode.ERROR }); - }); - - it('should return OK for Success HTTP status code', () => { for (let index = 100; index < 500; index++) { - const status = utils.parseClientResponseStatus(index); - assert.deepStrictEqual(status, { code: SpanStatusCode.OK }); + const status = utils.parseResponseStatus(SpanKind.SERVER, index); + assert.deepStrictEqual(status, SpanStatusCode.UNSET); } }); - it('should not return OK for Bad HTTP status code', () => { + it('should return ERROR for bad status codes', () => { + for (let index = 400; index <= 600; index++) { + const status = utils.parseResponseStatus(SpanKind.CLIENT, index); + assert.notStrictEqual(status, SpanStatusCode.UNSET); + } for (let index = 500; index <= 600; index++) { - const status = utils.parseClientResponseStatus(index); - assert.notStrictEqual(status.code, SpanStatusCode.OK); + const status = utils.parseResponseStatus(SpanKind.SERVER, index); + assert.notStrictEqual(status, SpanStatusCode.UNSET); } }); }); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts b/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts index 2d7bae085a..1afbffc029 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts @@ -70,7 +70,7 @@ export const assertSpan = ( assert.deepStrictEqual( span.status, validations.forceStatus || - utils.parseClientResponseStatus(validations.httpStatusCode) + { code: utils.parseResponseStatus(span.kind, validations.httpStatusCode) } ); assert.ok(span.endTime, 'must be finished'); From a9bb653a353af90e81661db558a0ff6c5b2779c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Einar=20Nor=C3=B0fj=C3=B6r=C3=B0?= Date: Tue, 15 Feb 2022 22:45:00 -0500 Subject: [PATCH 3/6] chore: imports --- .../test/functionals/utils.test.ts | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index 79fddc6267..cb7cfe3545 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -13,19 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import {context, ROOT_CONTEXT, SpanAttributes, SpanKind, SpanStatusCode, TraceFlags,} from '@opentelemetry/api'; -import {BasicTracerProvider, Span} from '@opentelemetry/sdk-trace-base'; -import {SemanticAttributes} from '@opentelemetry/semantic-conventions'; +import { + SpanAttributes, + SpanStatusCode, + ROOT_CONTEXT, + SpanKind, + TraceFlags, + context, +} from '@opentelemetry/api'; +import { BasicTracerProvider, Span } from '@opentelemetry/sdk-trace-base'; +import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; -import {IncomingMessage, ServerResponse} from 'http'; -import {Socket} from 'net'; +import { IncomingMessage, ServerResponse } from 'http'; +import { Socket } from 'net'; import * as sinon from 'sinon'; import * as url from 'url'; -import {IgnoreMatcher} from '../../src/types'; +import { IgnoreMatcher } from '../../src/types'; import * as utils from '../../src/utils'; -import {AttributeNames} from '../../src/enums/AttributeNames'; -import {RPCType, setRPCMetadata} from '@opentelemetry/core'; -import {AsyncHooksContextManager} from '@opentelemetry/context-async-hooks'; +import { AttributeNames } from '../../src/enums/AttributeNames'; +import { RPCType, setRPCMetadata } from '@opentelemetry/core'; +import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; describe('Utility', () => { describe('parseResponseStatus()', () => { From 045ab6900a92eba756633e09dd620dbff11420c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Einar=20Nor=C3=B0fj=C3=B6r=C3=B0?= Date: Wed, 23 Feb 2022 16:24:40 -0500 Subject: [PATCH 4/6] fix: lint --- .../packages/opentelemetry-instrumentation-http/src/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index 3786b0b44e..f050d64847 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -17,8 +17,8 @@ import { SpanAttributes, SpanStatusCode, Span, - SpanStatus, - context, SpanKind, + context, + SpanKind, } from '@opentelemetry/api'; import { NetTransportValues, From 1c7223c397cb9427628cf195bad2c2599f8de153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Einar=20Nor=C3=B0fj=C3=B6r=C3=B0?= Date: Wed, 16 Mar 2022 20:33:10 -0400 Subject: [PATCH 5/6] fix: tests --- .../test/functionals/utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index cb7cfe3545..69826ad5b1 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -259,7 +259,7 @@ describe('Utility', () => { ); /* tslint:disable-next-line:no-any */ utils.setSpanWithError(span, new Error('error'), SpanStatusCode.UNSET); - assert.strictEqual(span.status, SpanStatusCode.UNSET); + assert.strictEqual(span.status.code, SpanStatusCode.UNSET); }); }); From 37c3164a78567993158408b06fe6c647041a3c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Einar=20Nor=C3=B0fj=C3=B6r=C3=B0?= Date: Thu, 24 Mar 2022 12:53:40 -0400 Subject: [PATCH 6/6] feedback: do not have an optional statuscode parameter --- .../opentelemetry-instrumentation-http/src/http.ts | 3 ++- .../opentelemetry-instrumentation-http/src/utils.ts | 6 ++---- .../test/functionals/utils.test.ts | 12 ------------ 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index def1636adc..07dc688fac 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -333,8 +333,9 @@ export class HttpInstrumentation extends InstrumentationBase { }); response.on('error', (error: Err) => { this._diag.debug('outgoingRequest on error()', error); + utils.setSpanWithError(span, error); const code = utils.parseResponseStatus(SpanKind.CLIENT, response.statusCode); - utils.setSpanWithError(span, error, code); + span.setStatus({ code, message: error.message }); this._closeHttpSpan(span); }); } diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index f050d64847..68d7be7588 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -137,12 +137,10 @@ export const isIgnored = ( * Sets the span with the error passed in params * @param {Span} span the span that need to be set * @param {Error} error error that will be set to span - * @param {SpanStatusCode} [code] used for overriding the status */ export const setSpanWithError = ( span: Span, - error: Err, - code = SpanStatusCode.ERROR + error: Err ): void => { const message = error.message; @@ -151,7 +149,7 @@ export const setSpanWithError = ( [AttributeNames.HTTP_ERROR_MESSAGE]: message, }); - span.setStatus({ code, message }); + span.setStatus({ code: SpanStatusCode.ERROR, message }); }; /** diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index 69826ad5b1..062f160bdc 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -249,18 +249,6 @@ describe('Utility', () => { ); assert.ok(attributes[AttributeNames.HTTP_ERROR_NAME]); }); - it('can override the status', () => { - const span = new Span( - new BasicTracerProvider().getTracer('default'), - ROOT_CONTEXT, - 'test', - { spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED }, - SpanKind.INTERNAL - ); - /* tslint:disable-next-line:no-any */ - utils.setSpanWithError(span, new Error('error'), SpanStatusCode.UNSET); - assert.strictEqual(span.status.code, SpanStatusCode.UNSET); - }); }); describe('isValidOptionsType()', () => {