diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 11e38bf053a..c6875b43758 100644 --- a/CONTRIBUTING.md +++ b/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 diff --git a/README.md b/README.md index b64fa35e78a..4eb1202b1f7 100644 --- a/README.md +++ b/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 diff --git a/doc/development-guide.md b/doc/development-guide.md index 2bdc162a87c..69071c9a056 100644 --- a/doc/development-guide.md +++ b/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: - - -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. diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index 1f762f429b9..939da135f23 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/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 + this: typeof globalThis, + ...args: Parameters ): Promise { + 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(); } } diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 1d0c9faac10..dfadaed3869 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/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 { 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); diff --git a/package.json b/package.json index 2a2dc9bf8f8..3ab896698aa 100644 --- a/package.json +++ b/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" }, diff --git a/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts b/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts index 222f138d070..96008e4172b 100644 --- a/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts +++ b/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts @@ -94,7 +94,7 @@ function sendWithXhr( urlStr: string, xhrHeaders: Record = {} ) { - const xhr = new window.XMLHttpRequest(); + const xhr = new XMLHttpRequest(); xhr.open('POST', urlStr); Object.entries(xhrHeaders).forEach(([k, v]) => { xhr.setRequestHeader(k, v); diff --git a/packages/opentelemetry-sdk-trace-web/src/utils.ts b/packages/opentelemetry-sdk-trace-web/src/utils.ts index 829955feb0f..58831f4f215 100644 --- a/packages/opentelemetry-sdk-trace-web/src/utils.ts +++ b/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 diff --git a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts b/packages/opentelemetry-sdk-trace-web/test/utils.test.ts index 085c6213521..b8217e4f135 100644 --- a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts +++ b/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 = [ + '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) { diff --git a/renovate.json b/renovate.json index 9677523701e..b3b968bd100 100644 --- a/renovate.json +++ b/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"] } diff --git a/style.md b/style.md new file mode 100644 index 00000000000..5b1a0d80b9a --- /dev/null +++ b/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. diff --git a/tsconfig.json b/tsconfig.json index 0bae1a80bef..2a2648786c4 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -2,8 +2,8 @@ "extends": "./tsconfig.base.json", "files": [], "typedocOptions": { - "packages": [ - "experimental/packages/opentelemetry-api-metrics", + "entryPointStrategy": "packages", + "entryPoints": [ "experimental/packages/opentelemetry-exporter-metrics-otlp-grpc", "experimental/packages/opentelemetry-exporter-metrics-otlp-http", "experimental/packages/opentelemetry-exporter-metrics-otlp-proto", @@ -26,7 +26,6 @@ "packages/opentelemetry-propagator-b3", "packages/opentelemetry-propagator-jaeger", "packages/opentelemetry-resources", - "packages/opentelemetry-sdk-metrics-base", "packages/opentelemetry-sdk-trace-base", "packages/opentelemetry-sdk-trace-node", "packages/opentelemetry-sdk-trace-web",