diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index b031cb57fc..07dc688fac 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 = { code: utils.parseResponseStatus(SpanKind.CLIENT, response.statusCode) }; } span.setStatus(status); @@ -333,7 +333,9 @@ export class HttpInstrumentation extends InstrumentationBase { }); response.on('error', (error: Err) => { this._diag.debug('outgoingRequest on error()', error); - utils.setSpanWithError(span, error, response); + utils.setSpanWithError(span, error); + const code = utils.parseResponseStatus(SpanKind.CLIENT, response.statusCode); + span.setStatus({ code, message: error.message }); this._closeHttpSpan(span); }); } @@ -346,7 +348,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 +472,7 @@ export class HttpInstrumentation extends InstrumentationBase { span .setAttributes(attributes) - .setStatus(utils.parseResponseStatus(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 0a102ae688..68d7be7588 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -17,15 +17,14 @@ import { SpanAttributes, SpanStatusCode, Span, - SpanStatus, context, + SpanKind, } from '@opentelemetry/api'; import { NetTransportValues, SemanticAttributes, } from '@opentelemetry/semantic-conventions'; import { - ClientRequest, IncomingHttpHeaders, IncomingMessage, OutgoingHttpHeaders, @@ -65,24 +64,20 @@ export const getAbsoluteUrl = ( return `${protocol}//${host}${path}`; }; + /** * 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) { - 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; }; /** @@ -142,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 {(IncomingMessage | ClientRequest)} [obj] used for enriching the status by checking the statusCode. */ export const setSpanWithError = ( span: Span, - error: Err, - obj?: IncomingMessage | ClientRequest + error: Err ): void => { const message = error.message; @@ -156,23 +149,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: 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 aa1715a5a0..062f160bdc 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -37,23 +37,29 @@ import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; describe('Utility', () => { describe('parseResponseStatus()', () => { it('should return ERROR code by default', () => { - const status = utils.parseResponseStatus( - (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.parseResponseStatus(index); - assert.deepStrictEqual(status, { code: SpanStatusCode.OK }); + const status = utils.parseResponseStatus(SpanKind.CLIENT, index); + assert.deepStrictEqual(status, SpanStatusCode.UNSET); + } + for (let index = 100; index < 500; index++) { + 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(index); - assert.notStrictEqual(status.code, SpanStatusCode.OK); + const status = utils.parseResponseStatus(SpanKind.CLIENT, index); + assert.notStrictEqual(status, SpanStatusCode.UNSET); + } + for (let index = 500; index <= 600; index++) { + const status = utils.parseResponseStatus(SpanKind.SERVER, index); + assert.notStrictEqual(status, SpanStatusCode.UNSET); } }); }); @@ -227,23 +233,21 @@ 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]); }); }); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts b/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts index b887a46649..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.parseResponseStatus(validations.httpStatusCode) + { code: utils.parseResponseStatus(span.kind, validations.httpStatusCode) } ); assert.ok(span.endTime, 'must be finished');