fix(plugin-http): correct handling of WHATWG urls

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

@@ -395,18 +395,21 @@ export class HttpPlugin extends BasePlugin<Http> {
     const plugin = this;
     return function outgoingRequest(
       this: {},
-      options: RequestOptions | string,
+      options: url.URL | RequestOptions | string,
       ...args: unknown[]
     ): ClientRequest {
       if (!utils.isValidOptionsType(options)) {
         return original.apply(this, [options, ...args]);
       }
 
+      const extraOptions =
+        typeof args[0] === 'object' &&
+        (typeof options === 'string' || options instanceof url.URL)
+          ? (args.shift() as RequestOptions)
+          : undefined;
       const { origin, pathname, method, optionsParsed } = utils.getRequestInfo(
         options,
-        typeof args[0] === 'object' && typeof options === 'string'
-          ? (args.shift() as RequestOptions)
-          : undefined
+        extraOptions
       );
 
       options = optionsParsed;

packages/opentelemetry-plugin-http/src/utils.ts

@@ -200,21 +200,41 @@ export const setSpanWithError = (
  * @param [extraOptions] additional options for the request
  */
 export const getRequestInfo = (
-  options: RequestOptions | string,
+  options: url.URL | RequestOptions | string,
   extraOptions?: RequestOptions
 ) => {
   let pathname = '/';
   let origin = '';
-  let optionsParsed: url.URL | url.UrlWithStringQuery | RequestOptions;
+  let optionsParsed: RequestOptions;
   if (typeof options === 'string') {
     optionsParsed = url.parse(options);
     pathname = (optionsParsed as url.UrlWithStringQuery).pathname || '/';
     origin = `${optionsParsed.protocol || 'http:'}//${optionsParsed.host}`;
     if (extraOptions !== undefined) {
       Object.assign(optionsParsed, extraOptions);
     }
+  } else if (options instanceof url.URL) {
+    optionsParsed = {
+      protocol: options.protocol,
+      hostname:
+        typeof options.hostname === 'string' && options.hostname.startsWith('[')
+          ? options.hostname.slice(1, -1)
+          : options.hostname,
+      path: `${options.pathname || ''}${options.search || ''}`,
+    };
+    if (options.port !== '') {
+      optionsParsed.port = Number(options.port);
+    }
+    if (options.username || options.password) {
+      optionsParsed.auth = `${options.username}:${options.password}`;
+    }
+    pathname = options.pathname;
+    origin = options.origin;
+    if (extraOptions !== undefined) {
+      Object.assign(optionsParsed, extraOptions);
+    }
   } else {
-    optionsParsed = options as RequestOptions;
+    optionsParsed = Object.assign({}, options);
     pathname = (options as url.URL).pathname;
     if (!pathname && optionsParsed.path) {
       pathname = url.parse(optionsParsed.path).pathname || '/';
@@ -224,17 +244,15 @@ export const getRequestInfo = (
   }
 
   if (hasExpectHeader(optionsParsed)) {
-    (optionsParsed as RequestOptions).headers = Object.assign(
-      {},
-      (optionsParsed as RequestOptions).headers
-    );
-  } else if (!(optionsParsed as RequestOptions).headers) {
-    (optionsParsed as RequestOptions).headers = {};
+    optionsParsed.headers = Object.assign({}, optionsParsed.headers);
+  } else if (!optionsParsed.headers) {
+    optionsParsed.headers = {};
   }
   // some packages return method in lowercase..
   // ensure upperCase for consistency
-  let method = (optionsParsed as RequestOptions).method;
-  method = method ? method.toUpperCase() : 'GET';
+  const method = optionsParsed.method
+    ? optionsParsed.method.toUpperCase()
+    : 'GET';
 
   return { origin, pathname, method, optionsParsed };
 };

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

@@ -23,6 +23,7 @@ import { NodeTracer } from '@opentelemetry/node';
 import { CanonicalCode, Span as ISpan, SpanKind } from '@opentelemetry/types';
 import * as assert from 'assert';
 import * as http from 'http';
+import * as path from 'path';
 import * as nock from 'nock';
 import { HttpPlugin, plugin } from '../../src/http';
 import { assertSpan } from '../utils/assertSpan';
@@ -393,7 +394,7 @@ describe('HttpPlugin', () => {
         });
       }
 
-      for (const arg of ['string', '', {}, new Date()]) {
+      for (const arg of ['string', {}, new Date()]) {
         it(`should be tracable and not throw exception in http plugin when passing the following argument ${JSON.stringify(
           arg
         )}`, async () => {
@@ -409,7 +410,7 @@ describe('HttpPlugin', () => {
         });
       }
 
-      for (const arg of [true, 1, false, 0]) {
+      for (const arg of [true, 1, false, 0, '']) {
         it(`should not throw exception in http plugin when passing the following argument ${JSON.stringify(
           arg
         )}`, async () => {
@@ -420,7 +421,9 @@ describe('HttpPlugin', () => {
             // http request has been made
             // nock throw
             assert.ok(
-              error.stack.indexOf('/node_modules/nock/lib/intercept.js') > 0
+              error.stack.indexOf(
+                path.normalize('/node_modules/nock/lib/intercept.js')
+              ) > 0
             );
           }
           const spans = memoryExporter.getFinishedSpans();

packages/opentelemetry-plugin-http/test/functionals/utils.test.ts

@@ -74,18 +74,25 @@ describe('Utility', () => {
 
   describe('getRequestInfo()', () => {
     it('should get options object', () => {
-      const webUrl = 'http://google.fr/';
+      const webUrl = 'http://u:p@google.fr/aPath?qu=ry';
       const urlParsed = url.parse(webUrl);
       const urlParsedWithoutPathname = {
         ...urlParsed,
         pathname: undefined,
       };
-      for (const param of [webUrl, urlParsed, urlParsedWithoutPathname]) {
+      const whatWgUrl = new url.URL(webUrl);
+      for (const param of [
+        webUrl,
+        urlParsed,
+        urlParsedWithoutPathname,
+        whatWgUrl,
+      ]) {
         const result = utils.getRequestInfo(param);
         assert.strictEqual(result.optionsParsed.hostname, 'google.fr');
         assert.strictEqual(result.optionsParsed.protocol, 'http:');
-        assert.strictEqual(result.optionsParsed.path, '/');
-        assert.strictEqual(result.pathname, '/');
+        assert.strictEqual(result.optionsParsed.path, '/aPath?qu=ry');
+        assert.strictEqual(result.pathname, '/aPath');
+        assert.strictEqual(result.origin, 'http://google.fr');
       }
     });
   });

packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts

@@ -113,6 +113,60 @@ describe('HttpPlugin Integration tests', () => {
       assertSpan(span, SpanKind.CLIENT, validations);
     });
 
+    it('should create a rootSpan for GET requests and add propagation headers if URL is used', async () => {
+      let spans = memoryExporter.getFinishedSpans();
+      assert.strictEqual(spans.length, 0);
+
+      const result = await httpRequest.get(
+        new url.URL('http://google.fr/?query=test')
+      );
+
+      spans = memoryExporter.getFinishedSpans();
+      const span = spans[0];
+      const validations = {
+        hostname: 'google.fr',
+        httpStatusCode: result.statusCode!,
+        httpMethod: 'GET',
+        pathname: '/',
+        path: '/?query=test',
+        resHeaders: result.resHeaders,
+        reqHeaders: result.reqHeaders,
+        component: plugin.component,
+      };
+
+      assert.strictEqual(spans.length, 1);
+      assert.ok(span.name.indexOf('GET /') >= 0);
+      assertSpan(span, SpanKind.CLIENT, validations);
+    });
+
+    it('should create a rootSpan for GET requests and add propagation headers if URL and options are used', async () => {
+      let spans = memoryExporter.getFinishedSpans();
+      assert.strictEqual(spans.length, 0);
+
+      const result = await httpRequest.get(
+        new url.URL('http://google.fr/?query=test'),
+        { headers: { 'x-foo': 'foo' } }
+      );
+
+      spans = memoryExporter.getFinishedSpans();
+      const span = spans[0];
+      const validations = {
+        hostname: 'google.fr',
+        httpStatusCode: result.statusCode!,
+        httpMethod: 'GET',
+        pathname: '/',
+        path: '/?query=test',
+        resHeaders: result.resHeaders,
+        reqHeaders: result.reqHeaders,
+        component: plugin.component,
+      };
+
+      assert.strictEqual(spans.length, 1);
+      assert.ok(span.name.indexOf('GET /') >= 0);
+      assert.strictEqual(result.reqHeaders['x-foo'], 'foo');
+      assertSpan(span, SpanKind.CLIENT, validations);
+    });
+
     it('custom attributes should show up on client spans', async () => {
       const result = await httpRequest.get(`http://google.fr/`);
       const spans = memoryExporter.getFinishedSpans();

packages/opentelemetry-plugin-http/test/utils/httpRequest.ts

@@ -15,57 +15,54 @@
  */
 
 import * as http from 'http';
-import * as url from 'url';
-import { RequestOptions } from 'https';
+import { URL } from 'url';
 
-export const httpRequest = {
-  get: (
-    options: string | RequestOptions
-  ): Promise<{
-    data: string;
-    statusCode: number | undefined;
-    resHeaders: http.IncomingHttpHeaders;
-    reqHeaders: http.OutgoingHttpHeaders;
-    method: string | undefined;
-  }> => {
-    const _options =
-      typeof options === 'string'
-        ? Object.assign(url.parse(options), {
-            headers: {
-              'user-agent': 'http-plugin-test',
-            },
-          })
-        : options;
-    return new Promise((resolve, reject) => {
-      const req = http.get(_options, (resp: http.IncomingMessage) => {
-        const res = (resp as unknown) as http.IncomingMessage & {
-          req: http.IncomingMessage;
-        };
-        let data = '';
-        resp.on('data', chunk => {
-          data += chunk;
-        });
-        resp.on('end', () => {
-          resolve({
-            data,
-            statusCode: res.statusCode,
-            /* tslint:disable:no-any */
-            reqHeaders: (res.req as any).getHeaders
-              ? (res.req as any).getHeaders()
-              : (res.req as any)._headers,
-            /* tslint:enable:no-any */
-            resHeaders: res.headers,
-            method: res.req.method,
-          });
-        });
-        resp.on('error', err => {
-          reject(err);
+type GetResult = Promise<{
+  data: string;
+  statusCode: number | undefined;
+  resHeaders: http.IncomingHttpHeaders;
+  reqHeaders: http.OutgoingHttpHeaders;
+  method: string | undefined;
+}>;
+
+function get(input: string | URL, options?: http.RequestOptions): GetResult;
+function get(input: http.RequestOptions): GetResult;
+function get(input: any, options?: any): GetResult {
+  return new Promise((resolve, reject) => {
+    let req: http.ClientRequest;
+
+    function onGetResponseCb(resp: http.IncomingMessage): void {
+      const res = (resp as unknown) as http.IncomingMessage & {
+        req: http.IncomingMessage;
+      };
+      let data = '';
+      resp.on('data', chunk => {
+        data += chunk;
+      });
+      resp.on('end', () => {
+        resolve({
+          data,
+          statusCode: res.statusCode,
+          reqHeaders: req.getHeaders ? req.getHeaders() : (req as any)._headers,
+          resHeaders: res.headers,
+          method: res.req.method,
         });
       });
-      req.on('error', err => {
+      resp.on('error', err => {
         reject(err);
       });
-      return req;
+    }
+    req =
+      options != null
+        ? http.get(input, options, onGetResponseCb)
+        : http.get(input, onGetResponseCb);
+    req.on('error', err => {
+      reject(err);
     });
-  },
+    return req;
+  });
+}
+
+export const httpRequest = {
+  get,
 };