Merge branch 'main' into fix/2670

CONTRIBUTING.md

@@ -43,7 +43,7 @@ You will receive the following error :
 ✖   type must be one of [ci, feat, fix, docs, style, refactor, perf, test, revert, chore] [type-enum]
 ```
 
-Here an exemple that will pass the verification: `git commit -s -am "chore(opentelemetry-core): update deps"`
+Here an example that will pass the verification: `git commit -s -am "chore(opentelemetry-core): update deps"`
 
 ### Fork
 

README.md

@@ -194,7 +194,6 @@ Approvers ([@open-telemetry/js-approvers](https://github.com/orgs/open-telemetry
 
 Maintainers ([@open-telemetry/js-maintainers](https://github.com/orgs/open-telemetry/teams/javascript-maintainers)):
 
-- [Bartlomiej Obecny](https://github.com/obecny), LightStep
 - [Daniel Dyla](https://github.com/dyladan), Dynatrace
 - [Valentin Marchaud](https://github.com/vmarchaud), Open Source Contributor
 
@@ -208,6 +207,7 @@ Maintainers ([@open-telemetry/js-maintainers](https://github.com/orgs/open-telem
 
 ### Thanks to all previous approvers and maintainers
 
+- [Bartlomiej Obecny](https://github.com/obecny), LightStep, Maintainer
 - [Daniel Khan](https://github.com/dkhan), Dynatrace, Maintainer
 - [Mayur Kale](https://github.com/mayurkale22), Google, Maintainer
 - [Brandon Gonzalez](https://github.com/bg451), LightStep, Approver

doc/development-guide.md

@@ -7,7 +7,7 @@ The code base is a monorepo. We use [Lerna](https://lerna.js.org/) for managing
 ## Requirements
 
 Since this project supports multiple Node versions, using a version
-manager such as [nvm](https://github.com/creationix/nvm) is recommended.
+manager such as [nvm](https://github.com/nvm-sh/nvm) is recommended.
 
 To get started once you have Node installed, run:
 
@@ -51,23 +51,6 @@ To fix the linter, use:
 npm run lint:fix
 ```
 
-## Continuous Integration
-
-We rely on CircleCI 2.0 for our tests. If you want to test how the CI behaves
-locally, you can use the CircleCI Command Line Interface as described here:
-<https://circleci.com/docs/2.0/local-jobs/>
-
-After installing the `circleci` CLI, simply run one of the following:
-
-```sh
-circleci build --job lint
-circleci build --job node8
-circleci build --job node10
-circleci build --job node11
-circleci build --job node12
-circleci build --job node12-browsers
-```
-
 ## Docs
 
 We use [typedoc](https://www.npmjs.com/package/typedoc) to generate the api documentation.

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

@@ -27,6 +27,7 @@ import { AttributeNames } from './enums/AttributeNames';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import { FetchError, FetchResponse, SpanData } from './types';
 import { VERSION } from './version';
+import { _globalThis } from '@opentelemetry/core';
 
 // how long to wait for observer to collect information about resources
 // this is needed as event "load" is called before observer
@@ -288,13 +289,14 @@ export class FetchInstrumentation extends InstrumentationBase<
   /**
    * Patches the constructor of fetch
    */
-  private _patchConstructor(): (original: Window['fetch']) => Window['fetch'] {
+  private _patchConstructor(): (original: typeof fetch) => typeof fetch {
     return original => {
       const plugin = this;
       return function patchConstructor(
-        this: Window,
-        ...args: Parameters<Window['fetch']>
+        this: typeof globalThis,
+        ...args: Parameters<typeof fetch>
       ): Promise<Response> {
+        const self = this;
         const url = args[0] instanceof Request ? args[0].url : args[0];
         const options = args[0] instanceof Request ? args[0] : args[1] || {};
         const createdSpan = plugin._createSpan(url, options);
@@ -377,11 +379,13 @@ export class FetchInstrumentation extends InstrumentationBase<
             () => {
               plugin._addHeaders(options, url);
               plugin._tasksCount++;
+              // TypeScript complains about arrow function captured a this typed as globalThis
+              // ts(7041)
               return original
-                .apply(this, options instanceof Request ? [options] : [url, options])
+                .apply(self, options instanceof Request ? [options] : [url, options])
                 .then(
-                  onSuccess.bind(this, createdSpan, resolve),
-                  onError.bind(this, createdSpan, reject)
+                  onSuccess.bind(self, createdSpan, resolve),
+                  onError.bind(self, createdSpan, reject)
                 );
             }
           );
@@ -420,18 +424,17 @@ export class FetchInstrumentation extends InstrumentationBase<
   private _prepareSpanData(spanUrl: string): SpanData {
     const startTime = core.hrTime();
     const entries: PerformanceResourceTiming[] = [];
-    if (typeof window.PerformanceObserver === 'undefined') {
+    if (PerformanceObserver == null) {
       return { entries, startTime, spanUrl };
     }
 
     const observer: PerformanceObserver = new PerformanceObserver(list => {
       const perfObsEntries = list.getEntries() as PerformanceResourceTiming[];
-      const urlNormalizingAnchor = web.getUrlNormalizingAnchor();
-      urlNormalizingAnchor.href = spanUrl;
+      const parsedUrl = web.parseUrl(spanUrl);
       perfObsEntries.forEach(entry => {
         if (
           entry.initiatorType === 'fetch' &&
-          entry.name === urlNormalizingAnchor.href
+          entry.name === parsedUrl.href
         ) {
           entries.push(entry);
         }
@@ -447,18 +450,18 @@ export class FetchInstrumentation extends InstrumentationBase<
    * implements enable function
    */
   override enable(): void {
-    if (isWrapped(window.fetch)) {
-      this._unwrap(window, 'fetch');
+    if (isWrapped(fetch)) {
+      this._unwrap(_globalThis, 'fetch');
       this._diag.debug('removing previous patch for constructor');
     }
-    this._wrap(window, 'fetch', this._patchConstructor());
+    this._wrap(_globalThis, 'fetch', this._patchConstructor());
   }
 
   /**
    * implements unpatch function
    */
   override disable(): void {
-    this._unwrap(window, 'fetch');
+    this._unwrap(_globalThis, 'fetch');
     this._usedResources = new WeakSet<PerformanceResourceTiming>();
   }
 }

experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts

@@ -29,7 +29,6 @@ import {
   parseUrl,
   PerformanceTimingNames as PTN,
   shouldPropagateTraceHeaders,
-  getUrlNormalizingAnchor
 } from '@opentelemetry/sdk-trace-web';
 import { EventNames } from './enums/EventNames';
 import {
@@ -209,21 +208,20 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
     const xhrMem = this._xhrMem.get(xhr);
     if (
       !xhrMem ||
-      typeof window.PerformanceObserver === 'undefined' ||
-      typeof window.PerformanceResourceTiming === 'undefined'
+      PerformanceObserver == null ||
+      PerformanceResourceTiming == null
     ) {
       return;
     }
     xhrMem.createdResources = {
       observer: new PerformanceObserver(list => {
         const entries = list.getEntries() as PerformanceResourceTiming[];
-        const urlNormalizingAnchor = getUrlNormalizingAnchor();
-        urlNormalizingAnchor.href = spanUrl;
+        const parsedUrl = parseUrl(spanUrl);
 
         entries.forEach(entry => {
           if (
             entry.initiatorType === 'xmlhttprequest' &&
-            entry.name === urlNormalizingAnchor.href
+            entry.name === parsedUrl.href
           ) {
             if (xhrMem.createdResources) {
               xhrMem.createdResources.entries.push(entry);

package.json

@@ -2,8 +2,6 @@
   "name": "opentelemetry",
   "version": "0.1.0",
   "description": "OpenTelemetry is a distributed tracing and stats collection framework.",
-  "main": "build/src/index.js",
-  "types": "build/src/index.d.ts",
   "scripts": {
     "precompile": "lerna run version",
     "compile": "tsc --build",
@@ -65,7 +63,7 @@
     "lerna-changelog": "1.0.1",
     "markdownlint-cli": "0.29.0",
     "semver": "7.3.5",
-    "typedoc": "0.22.9",
+    "typedoc": "0.22.10",
     "typescript": "4.4.4",
     "update-ts-references": "2.4.1"
   },

packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts

@@ -94,7 +94,7 @@ function sendWithXhr(
   urlStr: string,
   xhrHeaders: Record<string, string> = {}
 ) {
-  const xhr = new window.XMLHttpRequest();
+  const xhr = new XMLHttpRequest();
   xhr.open('POST', urlStr);
   Object.entries(xhrHeaders).forEach(([k, v]) => {
     xhr.setRequestHeader(k, v);

packages/opentelemetry-sdk-trace-web/src/utils.ts

@@ -30,7 +30,7 @@ import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 
 // Used to normalize relative URLs
 let urlNormalizingAnchor: HTMLAnchorElement | undefined;
-export function getUrlNormalizingAnchor(): HTMLAnchorElement {
+function getUrlNormalizingAnchor(): HTMLAnchorElement {
   if (!urlNormalizingAnchor) {
     urlNormalizingAnchor = document.createElement('a');
   }
@@ -140,9 +140,8 @@ export function getResource(
   initiatorType?: string
 ): PerformanceResourceTimingInfo {
   // de-relativize the URL before usage (does no harm to absolute URLs)
-  const urlNormalizingAnchor = getUrlNormalizingAnchor();
-  urlNormalizingAnchor.href = spanUrl;
-  spanUrl = urlNormalizingAnchor.href;
+  const parsedSpanUrl = parseUrl(spanUrl);
+  spanUrl = parsedSpanUrl.toString();
 
   const filteredResources = filterResourcesForSpan(
     spanUrl,
@@ -165,7 +164,6 @@ export function getResource(
   }
   const sorted = sortResources(filteredResources);
 
-  const parsedSpanUrl = parseUrl(spanUrl);
   if (parsedSpanUrl.origin !== window.location.origin && sorted.length > 1) {
     let corsPreFlightRequest: PerformanceResourceTiming | undefined = sorted[0];
     let mainRequest: PerformanceResourceTiming = findMainRequest(
@@ -280,15 +278,48 @@ function filterResourcesForSpan(
 }
 
 /**
- * Parses url using anchor element
+ * The URLLike interface represents an URL and HTMLAnchorElement compatible fields.
+ */
+export interface URLLike {
+  hash: string;
+  host: string;
+  hostname: string;
+  href: string;
+  readonly origin: string;
+  password: string;
+  pathname: string;
+  port: string;
+  protocol: string;
+  search: string;
+  username: string;
+}
+
+/**
+ * Parses url using URL constructor or fallback to anchor element.
  * @param url
  */
-export function parseUrl(url: string): HTMLAnchorElement {
-  const element = document.createElement('a');
+export function parseUrl(url: string): URLLike {
+  if (typeof URL === 'function') {
+    return new URL(url);
+  }
+  const element = getUrlNormalizingAnchor();
   element.href = url;
   return element;
 }
 
+/**
+ * Parses url using URL constructor or fallback to anchor element and serialize
+ * it to a string.
+ *
+ * Performs the steps described in https://html.spec.whatwg.org/multipage/urls-and-fetching.html#parse-a-url
+ *
+ * @param url
+ */
+export function normalizeUrl(url: string): string {
+  const urlLike = parseUrl(url);
+  return urlLike.href;
+}
+
 /**
  * Get element XPath
  * @param target - target element

packages/opentelemetry-sdk-trace-web/test/utils.test.ts

@@ -29,8 +29,11 @@ import {
   addSpanNetworkEvents,
   getElementXPath,
   getResource,
+  normalizeUrl,
+  parseUrl,
   PerformanceEntries,
   shouldPropagateTraceHeaders,
+  URLLike,
 } from '../src';
 import { PerformanceTimingNames as PTN } from '../src/enums/PerformanceTimingNames';
 
@@ -587,6 +590,49 @@ describe('utils', () => {
       assert.strictEqual(result, false);
     });
   });
+
+  describe('parseUrl', () => {
+    const urlFields: Array<keyof URLLike> = [
+      'hash',
+      'host',
+      'hostname',
+      'href',
+      'origin',
+      'password',
+      'pathname',
+      'port',
+      'protocol',
+      'search',
+      'username',
+    ];
+    it('should parse url', () => {
+      const url = parseUrl('https://opentelemetry.io/foo');
+      urlFields.forEach(field => {
+        assert.strictEqual(typeof url[field], 'string');
+      });
+    });
+
+    it('should parse url with fallback', () => {
+      sinon.stub(window, 'URL').value(undefined);
+      const url = parseUrl('https://opentelemetry.io/foo');
+      urlFields.forEach(field => {
+        assert.strictEqual(typeof url[field], 'string');
+      });
+    });
+  });
+
+  describe('normalizeUrl', () => {
+    it('should normalize url', () => {
+      const url = normalizeUrl('https://opentelemetry.io/你好');
+      assert.strictEqual(url, 'https://opentelemetry.io/%E4%BD%A0%E5%A5%BD');
+    });
+
+    it('should parse url with fallback', () => {
+      sinon.stub(window, 'URL').value(undefined);
+      const url = normalizeUrl('https://opentelemetry.io/你好');
+      assert.strictEqual(url, 'https://opentelemetry.io/%E4%BD%A0%E5%A5%BD');
+    });
+  });
 });
 
 function getElementByXpath(path: string) {

renovate.json

@@ -17,7 +17,7 @@
     }
   ],
   "ignoreDeps": ["gcp-metadata", "got", "mocha", "husky", "karma-webpack"],
-  "assignees": ["@dyladan", "@obecny", "@vmarchaud"],
+  "assignees": ["@dyladan", "@vmarchaud"],
   "schedule": ["before 3am on Friday"],
   "labels": ["dependencies"]
 }

style.md

@@ -0,0 +1,37 @@
+# OpenTelemetry JS Style Guide
+
+This guide is meant to be a supplement to the linting rules.
+It is not exhaustive, nor should the suggestions in this guide be considered hard rules.
+Suggestions for changes and additions to this doc are welcome and this doc should not be considered to be set in stone.
+There may be code written before this guide which does not follow the suggestions in this guide.
+That code is not required to be updated, but maybe a good starting place for new contributors getting used to the codebase.
+
+## Test coverage
+
+In general, all changes should be tested.
+New features generally require tests to be added and bugfixes require tests to ensure there are no regressions.
+
+## Linting
+
+The lint check must pass in order for a PR to be merged.
+In some cases, it may be acceptable to disable a linting rule for a specific line or file.
+It may also be acceptable in some cases to modify the linting rules themselves if a sufficient argument is made that the rule should be changed.
+
+## `null` and `undefined` should be treated the same
+
+In general, null and undefined should be treated the same.
+In case of the rare exception where `null` is used to mean something different than `undefined` it should be documented clearly in the jsdocs.
+
+- Prefer `undefined` instead of `null` in most cases
+- Prefer `value == null` instead of `value == null || value == undefined`
+
+## Prefer `===` over `==`
+
+`===`/`!==` should be preferred over `==` and `!=`.
+An exception to this is when checking for `null`/`undefined` when it is preferred to use `== null` in order to treat `null` and `undefined` the same.
+
+## Prefer options objects to long lists of optional positional parameters
+
+For functions/methods/constructors with optional parameters, the parameters should be passed as an options object.
+This allows options to be added later without changing the function signature and avoids long lists of arguments.
+Required arguments should be at the start of the arguments list.