fix(instrumentation-http): add http.host attribute before sending the request

experimental/CHANGELOG.md

@@ -21,6 +21,7 @@ All notable changes to experimental packages in this project will be documented
 * fix(histogram): fix maximum when only values < -1 are provided [#3086](https://github.com/open-telemetry/opentelemetry-js/pull/3086) @pichlermarc
 * fix(sdk-metrics-base): fix PeriodicExportingMetricReader keeping Node.js process from exiting
   [#3106](https://github.com/open-telemetry/opentelemetry-js/pull/3106) @seemk
+* fix(instrumentation-http): add `http.host` attribute before sending the request #3054 @cuichenli
 
 ### :books: (Refine Doc)
 

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

@@ -292,7 +292,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
       (response: http.IncomingMessage & { aborted?: boolean }) => {
         const responseAttributes = utils.getOutgoingRequestAttributesOnResponse(
           response,
-          { hostname }
         );
         span.setAttributes(responseAttributes);
         if (this._getConfig().responseHook) {
@@ -352,6 +351,12 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
       this._closeHttpSpan(span);
     });
 
+    request.on('timeout', (error: Err) => {
+      this._diag.debug('outgoingRequest on request timeout()', error);
+      utils.setSpanWithError(span, new Error('Timeout'));
+      this._closeHttpSpan(span);
+    });
+
     this._diag.debug('http.ClientRequest return request');
     return request;
   }
@@ -561,13 +566,11 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
       }
 
       const operationName = `${component.toUpperCase()} ${method}`;
+      const { hostname, port } = utils.extractHostnameAndPort(optionsParsed);
 
-      const hostname =
-        optionsParsed.hostname ||
-        optionsParsed.host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') ||
-        'localhost';
       const attributes = utils.getOutgoingRequestAttributes(optionsParsed, {
         component,
+        port,
         hostname,
         hookAttributes: instrumentation._callStartSpanHook(
           optionsParsed,

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

@@ -295,20 +295,37 @@ export const isValidOptionsType = (options: unknown): boolean => {
   return type === 'string' || (type === 'object' && !Array.isArray(options));
 };
 
+export const extractHostnameAndPort = (
+  requestOptions: Pick<ParsedRequestOptions, 'hostname' | 'host' | 'port' | 'protocol'>
+): { hostname: string, port: number | string } => {
+  if (requestOptions.hostname && requestOptions.port) {
+    return {hostname: requestOptions.hostname, port: requestOptions.port};
+  }
+  const matches = requestOptions.host?.match(/^([^:/ ]+)(:\d{1,5})?/) || null;
+  const hostname = requestOptions.hostname || (matches === null ? 'localhost' : matches[1]);
+  let port = requestOptions.port;
+  if (!port) {
+    if (matches && matches[2]) {
+      // remove the leading ":". The extracted port would be something like ":8080"
+      port = matches[2].substring(1);
+    } else {
+      port = requestOptions.protocol === 'https:' ? '443' : '80';
+    }
+  }
+  return {hostname, port,};
+};
+
 /**
  * Returns outgoing request attributes scoped to the options passed to the request
  * @param {ParsedRequestOptions} requestOptions the same options used to make the request
  * @param {{ component: string, hostname: string, hookAttributes?: SpanAttributes }} options used to pass data needed to create attributes
  */
 export const getOutgoingRequestAttributes = (
   requestOptions: ParsedRequestOptions,
-  options: { component: string; hostname: string; hookAttributes?: SpanAttributes }
+  options: { component: string; hostname: string; port: string | number, hookAttributes?: SpanAttributes }
 ): SpanAttributes => {
-  const host = requestOptions.host;
-  const hostname =
-    requestOptions.hostname ||
-    host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') ||
-    'localhost';
+  const hostname = options.hostname;
+  const port = options.port;
   const requestMethod = requestOptions.method;
   const method = requestMethod ? requestMethod.toUpperCase() : 'GET';
   const headers = requestOptions.headers || {};
@@ -322,6 +339,7 @@ export const getOutgoingRequestAttributes = (
     [SemanticAttributes.HTTP_METHOD]: method,
     [SemanticAttributes.HTTP_TARGET]: requestOptions.path || '/',
     [SemanticAttributes.NET_PEER_NAME]: hostname,
+    [SemanticAttributes.HTTP_HOST]: requestOptions.headers?.host ?? `${hostname}:${port}`,
   };
 
   if (userAgent !== undefined) {
@@ -354,14 +372,12 @@ export const getAttributesFromHttpKind = (kind?: string): SpanAttributes => {
  */
 export const getOutgoingRequestAttributesOnResponse = (
   response: IncomingMessage,
-  options: { hostname: string }
 ): SpanAttributes => {
   const { statusCode, statusMessage, httpVersion, socket } = response;
   const { remoteAddress, remotePort } = socket;
   const attributes: SpanAttributes = {
     [SemanticAttributes.NET_PEER_IP]: remoteAddress,
     [SemanticAttributes.NET_PEER_PORT]: remotePort,
-    [SemanticAttributes.HTTP_HOST]: `${options.hostname}:${remotePort}`,
   };
   setResponseContentLengthAttribute(response, attributes);
 

experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts

@@ -28,11 +28,12 @@ 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, ParsedRequestOptions} 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 {extractHostnameAndPort} from '../../src/utils';
 
 describe('Utility', () => {
   describe('parseResponseStatus()', () => {
@@ -536,4 +537,58 @@ describe('Utility', () => {
       assert.deepStrictEqual(span.attributes['http.request.header.accept'], undefined);
     });
   });
+
+  describe('extractHostnameAndPort', () => {
+    it('should return the hostname and port defined in the parsedOptions', () => {
+      type tmpParsedOption = Pick<ParsedRequestOptions, 'hostname' | 'host' | 'port' | 'protocol'>;
+      const parsedOption: tmpParsedOption = {
+        hostname: 'www.google.com',
+        port: '80',
+        host: 'www.google.com',
+        protocol: 'http:'
+      };
+      const {hostname, port} = extractHostnameAndPort(parsedOption);
+      assert.strictEqual(hostname, parsedOption.hostname);
+      assert.strictEqual(port, parsedOption.port);
+    });
+
+    it('should return the hostname and port based on host field defined in the parsedOptions when hostname and port are missing', () => {
+      type tmpParsedOption = Pick<ParsedRequestOptions, 'hostname' | 'host' | 'port' | 'protocol'>;
+      const parsedOption: tmpParsedOption = {
+        hostname: null,
+        port: null,
+        host: 'www.google.com:8181',
+        protocol: 'http:'
+      };
+      const {hostname, port} = extractHostnameAndPort(parsedOption);
+      assert.strictEqual(hostname, 'www.google.com');
+      assert.strictEqual(port, '8181');
+    });
+
+    it('should infer the port number based on protocol https when can not extract it from host field', () => {
+      type tmpParsedOption = Pick<ParsedRequestOptions, 'hostname' | 'host' | 'port' | 'protocol'>;
+      const parsedOption: tmpParsedOption = {
+        hostname: null,
+        port: null,
+        host: 'www.google.com',
+        protocol: 'https:'
+      };
+      const {hostname, port} = extractHostnameAndPort(parsedOption);
+      assert.strictEqual(hostname, 'www.google.com');
+      assert.strictEqual(port, '443');
+    });
+
+    it('should infer the port number based on protocol http when can not extract it from host field', () => {
+      type tmpParsedOption = Pick<ParsedRequestOptions, 'hostname' | 'host' | 'port' | 'protocol'>;
+      const parsedOption: tmpParsedOption = {
+        hostname: null,
+        port: null,
+        host: 'www.google.com',
+        protocol: 'http:'
+      };
+      const {hostname, port} = extractHostnameAndPort(parsedOption);
+      assert.strictEqual(hostname, 'www.google.com');
+      assert.strictEqual(port, '80');
+    });
+  });
 });

experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts

@@ -57,6 +57,11 @@ describe('HttpInstrumentation Integration tests', () => {
   const sockets: Array<Socket> = [];
   before(done => {
     mockServer = http.createServer((req, res) => {
+      if (req.url === '/timeout') {
+        setTimeout(() => {
+          res.end();
+        }, 1000);
+      }
       res.statusCode = 200;
       res.setHeader('content-type', 'application/json');
       res.write(
@@ -359,5 +364,24 @@ describe('HttpInstrumentation Integration tests', () => {
       assert.strictEqual(spans.length, 2);
       assert.strictEqual(span.name, 'HTTP GET');
     });
+
+    it('should have correct spans even when request timeout', async () => {
+      let spans = memoryExporter.getFinishedSpans();
+      assert.strictEqual(spans.length, 0);
+
+      try {
+        await httpRequest.get(
+          `${protocol}://localhost:${mockServerPort}/timeout`, {timeout: 1}
+        );
+      } catch (err) {
+        assert.ok(err.message.startsWith('timeout'));
+      }
+
+      spans = memoryExporter.getFinishedSpans();
+      const span = spans.find(s => s.kind === SpanKind.CLIENT);
+      assert.ok(span);
+      assert.strictEqual(span.name, 'HTTP GET');
+      assert.strictEqual(span.attributes[SemanticAttributes.HTTP_HOST], `localhost:${mockServerPort}`);
+    });
   });
 });

experimental/packages/opentelemetry-instrumentation-http/test/utils/httpRequest.ts

@@ -59,6 +59,9 @@ function get(input: any, options?: any): GetResult {
     req.on('error', err => {
       reject(err);
     });
+    req.on('timeout', () => {
+      reject(new Error('timeout'));
+    });
     return req;
   });
 }