diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts index 4b52635eef..579d257003 100644 --- a/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/packages/opentelemetry-instrumentation-http/src/http.ts @@ -38,7 +38,6 @@ import { HttpInstrumentationConfig, HttpRequestArgs, Https, - ParsedRequestOptions, ResponseEndArgs, } from './types'; import * as utils from './utils'; @@ -272,20 +271,10 @@ export class HttpInstrumentation extends InstrumentationBase { * @param span representing the current operation */ private _traceClientRequest( - component: 'http' | 'https', request: http.ClientRequest, - options: ParsedRequestOptions, + hostname: string, span: Span ): http.ClientRequest { - const hostname = - options.hostname || - options.host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || - 'localhost'; - const attributes = utils.getOutgoingRequestAttributes(options, { - component, - hostname, - }); - span.setAttributes(attributes); if (this._getConfig().requestHook) { this._callRequestHook(span, request); } @@ -535,8 +524,19 @@ export class HttpInstrumentation extends InstrumentationBase { } const operationName = `${component.toUpperCase()} ${method}`; + + const hostname = + optionsParsed.hostname || + optionsParsed.host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || + 'localhost'; + const attributes = utils.getOutgoingRequestAttributes(optionsParsed, { + component, + hostname, + }); + const spanOptions: SpanOptions = { kind: SpanKind.CLIENT, + attributes, }; const span = instrumentation._startHttpSpan(operationName, spanOptions); @@ -572,9 +572,8 @@ export class HttpInstrumentation extends InstrumentationBase { instrumentation._diag.debug('%s instrumentation outgoingRequest', component); context.bind(parentContext, request); return instrumentation._traceClientRequest( - component, request, - optionsParsed, + hostname, span ); }); diff --git a/packages/opentelemetry-instrumentation-http/src/utils.ts b/packages/opentelemetry-instrumentation-http/src/utils.ts index fa1c55362a..194195142f 100644 --- a/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -69,8 +69,13 @@ export const getAbsoluteUrl = ( * 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 + 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 }; diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts index 218dbf2f4d..0debb99b71 100644 --- a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts @@ -480,12 +480,26 @@ describe('HttpInstrumentation', () => { } it('should have 1 ended span when request throw on bad "options" object', () => { - try { - http.request({ protocol: 'telnet' }); - } catch (error) { + assert.throws(() => http.request({ headers: { cookie: undefined} }), err => { const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - } + + const validations = { + httpStatusCode: undefined, + httpMethod: 'GET', + resHeaders: {}, + hostname: 'localhost', + pathname: '/', + forceStatus: { + code: SpanStatusCode.ERROR, + message: err.message, + }, + component: 'http', + noNetPeer: true, + } + assertSpan(spans[0], SpanKind.CLIENT, validations); + return true; + }); }); it('should have 1 ended span when response.end throw an exception', async () => { diff --git a/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts b/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts index 91263eb945..b28c7dfcaf 100644 --- a/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts +++ b/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts @@ -27,7 +27,7 @@ export const assertSpan = ( span: ReadableSpan, kind: SpanKind, validations: { - httpStatusCode: number; + httpStatusCode?: number; httpMethod: string; resHeaders: http.IncomingHttpHeaders; hostname: string; @@ -37,6 +37,7 @@ export const assertSpan = ( forceStatus?: SpanStatus; serverName?: string; component: string; + noNetPeer?: boolean; // we don't expect net peer info when request throw before being sent } ) => { assert.strictEqual(span.spanContext().traceId.length, 32); @@ -110,14 +111,16 @@ export const assertSpan = ( validations.hostname, 'must be consistent (PEER_NAME and hostname)' ); - assert.ok( - span.attributes[SemanticAttributes.NET_PEER_IP], - 'must have PEER_IP' - ); - assert.ok( - span.attributes[SemanticAttributes.NET_PEER_PORT], - 'must have PEER_PORT' - ); + if(!validations.noNetPeer) { + assert.ok( + span.attributes[SemanticAttributes.NET_PEER_IP], + 'must have PEER_IP' + ); + assert.ok( + span.attributes[SemanticAttributes.NET_PEER_PORT], + 'must have PEER_PORT' + ); + } assert.ok( (span.attributes[SemanticAttributes.HTTP_URL] as string).indexOf( span.attributes[SemanticAttributes.NET_PEER_NAME] as string