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

Add parsing and conversion of WHATWG URL objects for client http
requests to ensure semantics of HTTP request are not modifed and
tracestate header is correctly added.

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

@@ -395,7 +395,7 @@ 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)) {
@@ -404,7 +404,8 @@ export class HttpPlugin extends BasePlugin<Http> {
 
       const { origin, pathname, method, optionsParsed } = utils.getRequestInfo(
         options,
-        typeof args[0] === 'object' && typeof options === 'string'
+        typeof args[0] === 'object' &&
+          (typeof options === 'string' || options instanceof url.URL)
           ? (args.shift() as RequestOptions)
           : undefined
       );

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 (optionsParsed.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 = options;
     pathname = (options as url.URL).pathname;
     if (!pathname && optionsParsed.path) {
       pathname = url.parse(optionsParsed.path).pathname || '/';
@@ -224,16 +244,13 @@ 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;
+  let method = optionsParsed.method;
   method = method ? 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

@@ -18,54 +18,51 @@ import * as http from 'http';
 import * as url from 'url';
 import { RequestOptions } from 'https';
 
-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(url: string | url.URL, options?: RequestOptions): GetResult;
+function get(url: RequestOptions): GetResult;
+function get(url: any, options?: any): GetResult {
+  return new Promise((resolve, reject) => {
+    let req: http.ClientRequest;
+    function getResponseCb(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(url, options, getResponseCb)
+        : http.get(url, getResponseCb);
+    req.on('error', err => {
+      reject(err);
     });
-  },
+    return req;
+  });
+}
+
+export const httpRequest = {
+  get,
 };