fix(instrumentation-http): set outgoing request attributes on start span

packages/opentelemetry-instrumentation-http/src/http.ts

@@ -38,7 +38,6 @@ import {
   HttpInstrumentationConfig,
   HttpRequestArgs,
   Https,
-  ParsedRequestOptions,
   ResponseEndArgs,
 } from './types';
 import * as utils from './utils';
@@ -72,7 +71,9 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
     return this._config;
   }
 
-  override setConfig(config: HttpInstrumentationConfig & InstrumentationConfig = {}) {
+  override setConfig(
+    config: HttpInstrumentationConfig & InstrumentationConfig = {}
+  ) {
     this._config = Object.assign({}, config);
   }
 
@@ -272,20 +273,10 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
    * @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);
     }
@@ -378,13 +369,17 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
         : '/';
       const method = request.method || 'GET';
 
-      instrumentation._diag.debug('%s instrumentation incomingRequest', component);
+      instrumentation._diag.debug(
+        '%s instrumentation incomingRequest',
+        component
+      );
 
       if (
         utils.isIgnored(
           pathname,
           instrumentation._getConfig().ignoreIncomingPaths,
-          (e: Error) => instrumentation._diag.error('caught ignoreIncomingPaths error: ', e)
+          (e: Error) =>
+            instrumentation._diag.error('caught ignoreIncomingPaths error: ', e)
         )
       ) {
         return context.with(suppressTracing(context.active()), () => {
@@ -528,15 +523,27 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
         utils.isIgnored(
           origin + pathname,
           instrumentation._getConfig().ignoreOutgoingUrls,
-          (e: Error) => instrumentation._diag.error('caught ignoreOutgoingUrls error: ', e)
+          (e: Error) =>
+            instrumentation._diag.error('caught ignoreOutgoingUrls error: ', e)
         )
       ) {
         return original.apply(this, [optionsParsed, ...args]);
       }
 
       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);
 
@@ -569,12 +576,14 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
           }
         );
 
-        instrumentation._diag.debug('%s instrumentation outgoingRequest', component);
+        instrumentation._diag.debug(
+          '%s instrumentation outgoingRequest',
+          component
+        );
         context.bind(parentContext, request);
         return instrumentation._traceClientRequest(
-          component,
           request,
-          optionsParsed,
+          hostname,
           span
         );
       });

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<SpanStatus, 'message'> => {
+
+  if(statusCode === undefined) {
+    return { code: SpanStatusCode.ERROR };
+  }
+
   // 1xx, 2xx, 3xx are OK
   if (statusCode >= 100 && statusCode < 400) {
     return { code: SpanStatusCode.OK };

packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts

@@ -479,12 +479,29 @@ describe('HttpInstrumentation', () => {
         });
       }
 
-      it('should have 1 ended span when request throw on bad "options" object', () => {
+      it('should have 1 ended span when request throw on bad "options" object', done => {
         try {
-          http.request({ protocol: 'telnet' });
+          http.request({ headers: { cookie: undefined} }); // <===== this makes http.request throw
+          done('exception should have being thrown but did not');
         } catch (error) {
           const spans = memoryExporter.getFinishedSpans();
           assert.strictEqual(spans.length, 1);
+
+          const validations = {
+            httpStatusCode: undefined,
+            httpMethod: 'GET',
+            resHeaders: {},
+            hostname: 'localhost',
+            pathname: '/',
+            forceStatus: {
+              code: SpanStatusCode.ERROR, 
+              message: 'Invalid value "undefined" for header "cookie"',
+            },
+            component: 'http',
+            noNetPeer: true,
+          }
+          assertSpan(spans[0], SpanKind.CLIENT, validations);
+          done();
         }
       });
 

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