chore(plugin-https): sync https tests with http (open-telemetry#597)

* chore(plugin-https): sync https tests with http

* chore: use Http instead typeof http

* chore: review finding, improve https detection

* chore: fix node 8

* chore: fix path to test files

Author: Flarna

packages/opentelemetry-plugin-http/package.json

@@ -6,7 +6,7 @@
   "types": "build/src/index.d.ts",
   "repository": "open-telemetry/opentelemetry-js",
   "scripts": {
-    "test": "nyc ts-mocha -p tsconfig.json test/**/*/*.test.ts",
+    "test": "nyc ts-mocha -p tsconfig.json test/**/*.test.ts",
     "tdd": "yarn test -- --watch-extensions ts --watch",
     "clean": "rimraf build/*",
     "check": "gts check",

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

@@ -29,7 +29,7 @@ import { HttpPlugin, plugin } from '../../src/http';
 import { assertSpan } from '../utils/assertSpan';
 import { DummyPropagation } from '../utils/DummyPropagation';
 import { httpRequest } from '../utils/httpRequest';
-import * as utils from '../../src/utils';
+import { OT_REQUEST_HEADER } from '../../src/utils';
 import { HttpPluginConfig, Http } from '../../src/types';
 import { AttributeNames } from '../../src/enums/AttributeNames';
 
@@ -77,8 +77,8 @@ describe('HttpPlugin', () => {
     assert.strictEqual(process.versions.node, plugin.version);
   });
 
-  it('moduleName should be http', () => {
-    assert.strictEqual('http', plugin.moduleName);
+  it(`moduleName should be ${protocol}`, () => {
+    assert.strictEqual(protocol, plugin.moduleName);
   });
 
   describe('enable()', () => {
@@ -123,7 +123,7 @@ describe('HttpPlugin', () => {
 
       it('should generate valid spans (client side and server side)', async () => {
         const result = await httpRequest.get(
-          `http://${hostname}:${serverPort}${pathname}`
+          `${protocol}://${hostname}:${serverPort}${pathname}`
         );
         const spans = memoryExporter.getFinishedSpans();
         const [incomingSpan, outgoingSpan] = spans;
@@ -142,14 +142,14 @@ describe('HttpPlugin', () => {
         assertSpan(outgoingSpan, SpanKind.CLIENT, validations);
       });
 
-      it(`should not trace requests with '${utils.OT_REQUEST_HEADER}' header`, async () => {
+      it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => {
         const testPath = '/outgoing/do-not-trace';
         doNock(hostname, testPath, 200, 'Ok');
 
         const options = {
           host: hostname,
           path: testPath,
-          headers: { [utils.OT_REQUEST_HEADER]: 1 },
+          headers: { [OT_REQUEST_HEADER]: 1 },
         };
 
         const result = await httpRequest.get(options);
@@ -171,7 +171,7 @@ describe('HttpPlugin', () => {
             (url: string) => url.endsWith(`/ignored/function`),
           ],
           ignoreOutgoingUrls: [
-            `http://${hostname}:${serverPort}/ignored/string`,
+            `${protocol}://${hostname}:${serverPort}/ignored/string`,
             /\/ignored\/regexp$/i,
             (url: string) => url.endsWith(`/ignored/function`),
           ],
@@ -190,11 +190,11 @@ describe('HttpPlugin', () => {
         plugin.disable();
       });
 
-      it('http module should be patched', () => {
+      it(`${protocol} module should be patched`, () => {
         assert.strictEqual(http.Server.prototype.emit.__wrapped, true);
       });
 
-      it("should not patch if it's not a http module", () => {
+      it(`should not patch if it's not a ${protocol} module`, () => {
         const httpNotPatched = new HttpPlugin(
           plugin.component,
           process.versions.node
@@ -204,7 +204,7 @@ describe('HttpPlugin', () => {
 
       it('should generate valid spans (client side and server side)', async () => {
         const result = await httpRequest.get(
-          `http://${hostname}:${serverPort}${pathname}`
+          `${protocol}://${hostname}:${serverPort}${pathname}`
         );
         const spans = memoryExporter.getFinishedSpans();
         const [incomingSpan, outgoingSpan] = spans;
@@ -223,14 +223,14 @@ describe('HttpPlugin', () => {
         assertSpan(outgoingSpan, SpanKind.CLIENT, validations);
       });
 
-      it(`should not trace requests with '${utils.OT_REQUEST_HEADER}' header`, async () => {
+      it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => {
         const testPath = '/outgoing/do-not-trace';
         doNock(hostname, testPath, 200, 'Ok');
 
         const options = {
           host: hostname,
           path: testPath,
-          headers: { [utils.OT_REQUEST_HEADER]: 1 },
+          headers: { [OT_REQUEST_HEADER]: 1 },
         };
 
         const result = await httpRequest.get(options);
@@ -395,13 +395,13 @@ describe('HttpPlugin', () => {
       }
 
       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(
+        it(`should be tracable and not throw exception in ${protocol} plugin when passing the following argument ${JSON.stringify(
           arg
         )}`, async () => {
           try {
             await httpRequest.get(arg);
           } catch (error) {
-            // http request has been made
+            // request has been made
             // nock throw
             assert.ok(error.message.startsWith('Nock: No match for request'));
           }
@@ -411,14 +411,14 @@ describe('HttpPlugin', () => {
       }
 
       for (const arg of [true, 1, false, 0, '']) {
-        it(`should not throw exception in http plugin when passing the following argument ${JSON.stringify(
+        it(`should not throw exception in ${protocol} plugin when passing the following argument ${JSON.stringify(
           arg
         )}`, async () => {
           try {
             // @ts-ignore
             await httpRequest.get(arg);
           } catch (error) {
-            // http request has been made
+            // request has been made
             // nock throw
             assert.ok(
               error.stack.indexOf(
@@ -447,7 +447,7 @@ describe('HttpPlugin', () => {
 
         const promiseRequest = new Promise((resolve, reject) => {
           const req = http.request(
-            `http://${hostname}${testPath}`,
+            `${protocol}://${hostname}${testPath}`,
             (resp: http.IncomingMessage) => {
               let data = '';
               resp.on('data', chunk => {
@@ -488,7 +488,7 @@ describe('HttpPlugin', () => {
 
         const promiseRequest = new Promise((resolve, reject) => {
           const req = http.request(
-            `http://${hostname}${testPath}`,
+            `${protocol}://${hostname}${testPath}`,
             (resp: http.IncomingMessage) => {
               let data = '';
               resp.on('data', chunk => {
@@ -512,14 +512,14 @@ describe('HttpPlugin', () => {
       });
 
       it('should have 1 ended span when request is aborted', async () => {
-        nock('http://my.server.com')
+        nock(`${protocol}://my.server.com`)
           .get('/')
           .socketDelay(50)
           .reply(200, '<html></html>');
 
         const promiseRequest = new Promise((resolve, reject) => {
           const req = http.request(
-            'http://my.server.com',
+            `${protocol}://my.server.com`,
             (resp: http.IncomingMessage) => {
               let data = '';
               resp.on('data', chunk => {

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

@@ -38,6 +38,7 @@ import { HttpPluginConfig } from '../../src/types';
 import { customAttributeFunction } from './http-enable.test';
 
 const memoryExporter = new InMemorySpanExporter();
+const protocol = 'http';
 
 describe('Packages', () => {
   describe('get', () => {
@@ -93,8 +94,8 @@ describe('Packages', () => {
               // https://github.com/nock/nock/pull/1551
               // https://github.com/sindresorhus/got/commit/bf1aa5492ae2bc78cbbec6b7d764906fb156e6c2#diff-707a4781d57c42085155dcb27edb9ccbR258
               // TODO: check if this is still the case when new version
-              'http://info.cern.ch/'
-            : `http://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8`
+              `${protocol}://info.cern.ch/`
+            : `${protocol}://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8`
         );
         const result = await httpPackage.get(urlparsed.href!);
         if (!resHeaders) {

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

@@ -30,7 +30,7 @@ import {
   SimpleSpanProcessor,
 } from '@opentelemetry/tracing';
 import { HttpPluginConfig } from '../../src/types';
-
+const protocol = 'http';
 const serverPort = 32345;
 const hostname = 'localhost';
 const memoryExporter = new InMemorySpanExporter();
@@ -70,7 +70,7 @@ describe('HttpPlugin Integration tests', () => {
 
     before(() => {
       const ignoreConfig = [
-        `http://${hostname}:${serverPort}/ignored/string`,
+        `${protocol}://${hostname}:${serverPort}/ignored/string`,
         /\/ignored\/regexp$/i,
         (url: string) => url.endsWith(`/ignored/function`),
       ];
@@ -93,7 +93,9 @@ describe('HttpPlugin Integration tests', () => {
       let spans = memoryExporter.getFinishedSpans();
       assert.strictEqual(spans.length, 0);
 
-      const result = await httpRequest.get(`http://google.fr/?query=test`);
+      const result = await httpRequest.get(
+        `${protocol}://google.fr/?query=test`
+      );
 
       spans = memoryExporter.getFinishedSpans();
       const span = spans[0];
@@ -118,7 +120,7 @@ describe('HttpPlugin Integration tests', () => {
       assert.strictEqual(spans.length, 0);
 
       const result = await httpRequest.get(
-        new url.URL('http://google.fr/?query=test')
+        new url.URL(`${protocol}://google.fr/?query=test`)
       );
 
       spans = memoryExporter.getFinishedSpans();
@@ -144,7 +146,7 @@ describe('HttpPlugin Integration tests', () => {
       assert.strictEqual(spans.length, 0);
 
       const result = await httpRequest.get(
-        new url.URL('http://google.fr/?query=test'),
+        new url.URL(`${protocol}://google.fr/?query=test`),
         { headers: { 'x-foo': 'foo' } }
       );
 
@@ -168,7 +170,7 @@ describe('HttpPlugin Integration tests', () => {
     });
 
     it('custom attributes should show up on client spans', async () => {
-      const result = await httpRequest.get(`http://google.fr/`);
+      const result = await httpRequest.get(`${protocol}://google.fr/`);
       const spans = memoryExporter.getFinishedSpans();
       const span = spans[0];
       const validations = {
@@ -192,7 +194,7 @@ describe('HttpPlugin Integration tests', () => {
       assert.strictEqual(spans.length, 0);
       const options = Object.assign(
         { headers: { Expect: '100-continue' } },
-        url.parse('http://google.fr/')
+        url.parse(`${protocol}://google.fr/`)
       );
 
       const result = await httpRequest.get(options);
@@ -223,7 +225,7 @@ describe('HttpPlugin Integration tests', () => {
       { Expect: '100-continue', 'user-agent': 'http-plugin-test' },
       { 'user-agent': 'http-plugin-test' },
     ]) {
-      it(`should create a span for GET requests and add propagation when using the following signature: http.get(url, options, callback) and following headers: ${JSON.stringify(
+      it(`should create a span for GET requests and add propagation when using the following signature: get(url, options, callback) and following headers: ${JSON.stringify(
         headers
       )}`, done => {
         let validations: {
@@ -239,7 +241,7 @@ describe('HttpPlugin Integration tests', () => {
         assert.strictEqual(spans.length, 0);
         const options = { headers };
         const req = http.get(
-          'http://google.fr/',
+          `${protocol}://google.fr/`,
           options,
           (resp: http.IncomingMessage) => {
             const res = (resp as unknown) as http.IncomingMessage & {
@@ -266,7 +268,7 @@ describe('HttpPlugin Integration tests', () => {
           }
         );
 
-        req.once('close', () => {
+        req.on('close', () => {
           const spans = memoryExporter.getFinishedSpans();
           assert.strictEqual(spans.length, 1);
           assert.ok(spans[0].name.indexOf('GET /') >= 0);

packages/opentelemetry-plugin-https/README.md

@@ -5,7 +5,7 @@
 [![devDependencies][devDependencies-image]][devDependencies-url]
 [![Apache License][license-image]][license-image]
 
-This module provides automatic instrumentation for [`https`](http://nodejs.org/dist/latest/docs/api/https.html).
+This module provides automatic instrumentation for [`https`](http://nodejs.org/api/https.html).
 
 For automatic instrumentation see the
 [@opentelemetry/node](https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-node) package.

packages/opentelemetry-plugin-https/package.json

@@ -6,10 +6,11 @@
   "types": "build/src/index.d.ts",
   "repository": "open-telemetry/opentelemetry-js",
   "scripts": {
-    "test": "nyc ts-mocha -p tsconfig.json 'test/**/*.ts'",
+    "test": "nyc ts-mocha -p tsconfig.json test/**/*.test.ts",
     "tdd": "yarn test -- --watch-extensions ts --watch",
     "clean": "rimraf build/*",
     "check": "gts check",
+    "codecov": "nyc report --reporter=json && codecov -f coverage/*.json -p ../../",
     "precompile": "tsc --version",
     "compile": "tsc -p .",
     "fix": "gts fix",

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

@@ -17,6 +17,7 @@
 import { HttpPlugin, Func, HttpRequestArgs } from '@opentelemetry/plugin-http';
 import * as http from 'http';
 import * as https from 'https';
+import { URL } from 'url';
 import * as semver from 'semver';
 import * as shimmer from 'shimmer';
 import * as utils from './utils';
@@ -81,12 +82,13 @@ export class HttpsPlugin extends HttpPlugin {
     return (original: Func<http.ClientRequest>): Func<http.ClientRequest> => {
       const plugin = this;
       return function httpsOutgoingRequest(
-        options,
+        options: https.RequestOptions | string | URL,
         ...args: HttpRequestArgs
       ): http.ClientRequest {
         // Makes sure options will have default HTTPS parameters
-        if (typeof options === 'object') {
-          utils.setDefaultOptions(options);
+        if (typeof options === 'object' && !(options instanceof URL)) {
+          options = Object.assign({}, options);
+          utils.setDefaultOptions(options as https.RequestOptions);
         }
         return plugin._getPatchOutgoingRequestFunction()(original)(
           options,
@@ -105,17 +107,9 @@ export class HttpsPlugin extends HttpPlugin {
   ) {
     return (original: Func<http.ClientRequest>): Func<http.ClientRequest> => {
       return function httpsOutgoingRequest(
-        options: https.RequestOptions | string,
+        options: https.RequestOptions | string | URL,
         ...args: HttpRequestArgs
       ): http.ClientRequest {
-        const optionsType = typeof options;
-        // Makes sure options will have default HTTPS parameters
-        if (optionsType === 'object') {
-          utils.setDefaultOptions(options as https.RequestOptions);
-        } else if (typeof args[0] === 'object' && optionsType === 'string') {
-          utils.setDefaultOptions(args[0] as https.RequestOptions);
-        }
-
         return plugin._getPatchOutgoingGetFunction(clientRequest)(original)(
           options,
           ...args

packages/opentelemetry-plugin-https/test/functionals/https-disable.test.ts

@@ -14,16 +14,17 @@
  * limitations under the License.
  */
 
-import { NoopLogger } from '@opentelemetry/core';
-import { NodeTracer } from '@opentelemetry/node';
-import { Http } from '@opentelemetry/plugin-http';
 import * as assert from 'assert';
 import * as fs from 'fs';
 import * as https from 'https';
-import { AddressInfo } from 'net';
 import * as nock from 'nock';
 import * as sinon from 'sinon';
+
 import { plugin } from '../../src/https';
+import { NodeTracer } from '@opentelemetry/node';
+import { NoopLogger } from '@opentelemetry/core';
+import { Http } from '@opentelemetry/plugin-http';
+import { AddressInfo } from 'net';
 import { DummyPropagation } from '../utils/DummyPropagation';
 import { httpsRequest } from '../utils/httpsRequest';