From 8aa912b98d57e7c141161bcc09d01ead849d7d89 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 6 Nov 2020 15:09:39 +0100 Subject: [PATCH 1/2] feat: replacing base plugin with instrumentation for xml-http-request --- .circleci/config.yml | 2 +- README.md | 4 +- .../examples/user-interaction/index.html | 2 +- .../examples/user-interaction/index.js | 4 +- .../examples/xml-http-request/index.js | 4 +- examples/tracer-web/package.json | 2 +- metapackages/plugins-web-core/README.md | 4 +- metapackages/plugins-web-core/package.json | 2 +- .../.eslintignore | 0 .../.eslintrc.js | 0 .../.npmignore | 0 .../LICENSE | 0 .../README.md | 31 +++++-- .../images/cors.jpg | Bin .../images/main.jpg | Bin .../images/request.jpg | Bin .../karma.conf.js | 0 .../package.json | 7 +- .../src/enums/EventNames.ts | 0 .../src/index.ts | 0 .../src/types.ts | 0 .../src/version.ts | 0 .../src/xhr.ts | 77 +++++++++++------- .../test/index-webpack.ts | 0 .../test/unmocked.test.ts | 4 +- .../test/xhr.test.ts | 21 ++--- .../tsconfig.json | 0 .../src/instrumentation.ts | 10 +++ .../src/platform/node/instrumentation.ts | 8 -- .../test/common/Instrumentation.test.ts | 1 + packages/opentelemetry-web/package.json | 1 + .../src/WebTracerProvider.ts | 10 ++- .../test/WebTracerProvider.test.ts | 17 +++- 33 files changed, 131 insertions(+), 80 deletions(-) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/.eslintignore (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/.eslintrc.js (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/.npmignore (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/LICENSE (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/README.md (65%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/images/cors.jpg (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/images/main.jpg (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/images/request.jpg (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/karma.conf.js (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/package.json (93%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/src/enums/EventNames.ts (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/src/index.ts (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/src/types.ts (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/src/version.ts (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/src/xhr.ts (87%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/test/index-webpack.ts (100%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/test/unmocked.test.ts (95%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/test/xhr.test.ts (98%) rename packages/{opentelemetry-plugin-xml-http-request => opentelemetry-instrumentation-xml-http-request}/tsconfig.json (100%) diff --git a/.circleci/config.yml b/.circleci/config.yml index 15b55c5a74..c47b2d6439 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -31,7 +31,7 @@ cache_2: &cache_2 - packages/opentelemetry-plugin-http/node_modules - packages/opentelemetry-plugin-https/node_modules - packages/opentelemetry-exporter-collector/node_modules - - packages/opentelemetry-plugin-xml-http-request/node_modules + - packages/opentelemetry-instrumentation-xml-http-request/node_modules - packages/opentelemetry-resource-detector-aws/node_modules - packages/opentelemetry-resource-detector-gcp/node_modules - packages/opentelemetry-resources/node_modules diff --git a/README.md b/README.md index 52450872dc..61e6ae7b41 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,7 @@ These plugins are hosted at - Example of using Web Tracer with UserInteractionPlugin and XMLHttpRequestPlugin with console exporter and collector exporter + Example of using Web Tracer with UserInteractionPlugin and XMLHttpRequestInstrumentation with console exporter and collector exporter
diff --git a/examples/tracer-web/examples/user-interaction/index.js b/examples/tracer-web/examples/user-interaction/index.js index 816a2465c7..de22402205 100644 --- a/examples/tracer-web/examples/user-interaction/index.js +++ b/examples/tracer-web/examples/user-interaction/index.js @@ -1,15 +1,15 @@ import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing'; import { WebTracerProvider } from '@opentelemetry/web'; -import { XMLHttpRequestPlugin } from '@opentelemetry/plugin-xml-http-request'; import { UserInteractionPlugin } from '@opentelemetry/plugin-user-interaction'; import { ZoneContextManager } from '@opentelemetry/context-zone'; import { CollectorTraceExporter } from '@opentelemetry/exporter-collector'; import { B3Propagator } from '@opentelemetry/propagator-b3'; +import { XMLHttpRequestInstrumentation } from '@opentelemetry/instrumentation-xml-http-request'; const providerWithZone = new WebTracerProvider({ plugins: [ new UserInteractionPlugin(), - new XMLHttpRequestPlugin({ + new XMLHttpRequestInstrumentation({ ignoreUrls: [/localhost/], propagateTraceHeaderCorsUrls: [ 'http://localhost:8090', diff --git a/examples/tracer-web/examples/xml-http-request/index.js b/examples/tracer-web/examples/xml-http-request/index.js index d0a148e7e3..2532037bc9 100644 --- a/examples/tracer-web/examples/xml-http-request/index.js +++ b/examples/tracer-web/examples/xml-http-request/index.js @@ -1,13 +1,13 @@ import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing'; import { WebTracerProvider } from '@opentelemetry/web'; -import { XMLHttpRequestPlugin } from '@opentelemetry/plugin-xml-http-request'; +import { XMLHttpRequestInstrumentation } from '@opentelemetry/instrumentation-xml-http-request'; import { ZoneContextManager } from '@opentelemetry/context-zone'; import { CollectorTraceExporter } from '@opentelemetry/exporter-collector'; import { B3Propagator } from '@opentelemetry/propagator-b3'; const providerWithZone = new WebTracerProvider({ plugins: [ - new XMLHttpRequestPlugin({ + new XMLHttpRequestInstrumentation({ ignoreUrls: [/localhost:8090\/sockjs-node/], propagateTraceHeaderCorsUrls: [ 'https://httpbin.org/get', diff --git a/examples/tracer-web/package.json b/examples/tracer-web/package.json index f999bfdd49..955122fb3b 100644 --- a/examples/tracer-web/package.json +++ b/examples/tracer-web/package.json @@ -43,7 +43,7 @@ "@opentelemetry/plugin-document-load": "^0.9.0", "@opentelemetry/plugin-fetch": "^0.12.0", "@opentelemetry/plugin-user-interaction": "^0.9.0", - "@opentelemetry/plugin-xml-http-request": "^0.12.0", + "@opentelemetry/instrumentation-xml-http-request": "^0.12.0", "@opentelemetry/tracing": "^0.12.0", "@opentelemetry/web": "^0.12.0" }, diff --git a/metapackages/plugins-web-core/README.md b/metapackages/plugins-web-core/README.md index 2fa0dcfb79..9f12103b5e 100644 --- a/metapackages/plugins-web-core/README.md +++ b/metapackages/plugins-web-core/README.md @@ -9,7 +9,7 @@ This package depends on all core web plugins maintained by OpenTelemetry authors ## Plugins -- [@opentelemetry/plugin-xml-http-request][otel-plugin-xml-http-request] +- [@opentelemetry/instrumentation-xml-http-request][otel-instrumentation-xml-http-request] ## Useful links @@ -30,4 +30,4 @@ Apache 2.0 - See [LICENSE][license-url] for more information. [npm-url]: https://www.npmjs.com/package/@opentelemetry/plugins-web-core [npm-img]: https://badge.fury.io/js/%40opentelemetry%2Fplugins-web-core.svg -[otel-plugin-xml-http-request]: https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-plugin-xml-http-request +[otel-instrumentation-xml-http-request]: https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-instrumentation-xml-http-request diff --git a/metapackages/plugins-web-core/package.json b/metapackages/plugins-web-core/package.json index a2c3a09b8e..a4ca9a62ff 100644 --- a/metapackages/plugins-web-core/package.json +++ b/metapackages/plugins-web-core/package.json @@ -16,6 +16,6 @@ "url": "https://github.com/open-telemetry/opentelemetry-js/issues" }, "dependencies": { - "@opentelemetry/plugin-xml-http-request": "^0.12.0" + "@opentelemetry/instrumentation-xml-http-request": "^0.12.0" } } diff --git a/packages/opentelemetry-plugin-xml-http-request/.eslintignore b/packages/opentelemetry-instrumentation-xml-http-request/.eslintignore similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/.eslintignore rename to packages/opentelemetry-instrumentation-xml-http-request/.eslintignore diff --git a/packages/opentelemetry-plugin-xml-http-request/.eslintrc.js b/packages/opentelemetry-instrumentation-xml-http-request/.eslintrc.js similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/.eslintrc.js rename to packages/opentelemetry-instrumentation-xml-http-request/.eslintrc.js diff --git a/packages/opentelemetry-plugin-xml-http-request/.npmignore b/packages/opentelemetry-instrumentation-xml-http-request/.npmignore similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/.npmignore rename to packages/opentelemetry-instrumentation-xml-http-request/.npmignore diff --git a/packages/opentelemetry-plugin-xml-http-request/LICENSE b/packages/opentelemetry-instrumentation-xml-http-request/LICENSE similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/LICENSE rename to packages/opentelemetry-instrumentation-xml-http-request/LICENSE diff --git a/packages/opentelemetry-plugin-xml-http-request/README.md b/packages/opentelemetry-instrumentation-xml-http-request/README.md similarity index 65% rename from packages/opentelemetry-plugin-xml-http-request/README.md rename to packages/opentelemetry-instrumentation-xml-http-request/README.md index 465c0c8782..89c2f63cec 100644 --- a/packages/opentelemetry-plugin-xml-http-request/README.md +++ b/packages/opentelemetry-instrumentation-xml-http-request/README.md @@ -11,7 +11,7 @@ This module provides auto instrumentation for web using XMLHttpRequest . ## Installation ```bash -npm install --save @opentelemetry/plugin-xml-http-request +npm install --save @opentelemetry/instrumentation-xml-http-request ``` ## Usage @@ -19,17 +19,30 @@ npm install --save @opentelemetry/plugin-xml-http-request ```js import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing'; import { WebTracer } from '@opentelemetry/web'; -import { XMLHttpRequestPlugin } from '@opentelemetry/plugin-xml-http-request'; +import { XMLHttpRequestInstrumentation } from '@opentelemetry/instrumentation-xml-http-request'; import { ZoneContextManager } from '@opentelemetry/context-zone'; +// this is still possible const webTracerWithZone = new WebTracer({ contextManager: new ZoneContextManager(), plugins: [ - new XMLHttpRequestPlugin({ + new XMLHttpRequestInstrumentation({ propagateTraceHeaderCorsUrls: ['http://localhost:8090'] }) ] }); +///////////////////////////////////////// + +// or plugin can be also initialised separately and then set the tracer provider or meter provider +const xmlHttpRequestInstrumentation = new XMLHttpRequestInstrumentation({ + propagateTraceHeaderCorsUrls: ['http://localhost:8090'] +}); +const webTracerWithZone = new WebTracer({ + contextManager: new ZoneContextManager(), +}); +xmlHttpRequestInstrumentation.setTracerProvider(webTracerWithZone); +///////////////////////////////////////// + webTracerWithZone.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter())); // and some test @@ -61,9 +74,9 @@ Apache 2.0 - See [LICENSE][license-url] for more information. [gitter-url]: https://gitter.im/open-telemetry/opentelemetry-node?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge [license-url]: https://github.com/open-telemetry/opentelemetry-js/blob/master/LICENSE [license-image]: https://img.shields.io/badge/license-Apache_2.0-green.svg?style=flat -[dependencies-image]: https://david-dm.org/open-telemetry/opentelemetry-js/status.svg?path=packages/opentelemetry-plugin-xml-http-request -[dependencies-url]: https://david-dm.org/open-telemetry/opentelemetry-js?path=packages%2Fopentelemetry-plugin-xml-http-request -[devDependencies-image]: https://david-dm.org/open-telemetry/opentelemetry-js/dev-status.svg?path=packages/opentelemetry-plugin-xml-http-request -[devDependencies-url]: https://david-dm.org/open-telemetry/opentelemetry-js?path=packages%2Fopentelemetry-plugin-xml-http-request&type=dev -[npm-url]: https://www.npmjs.com/package/@opentelemetry/plugin-xml-http-request -[npm-img]: https://badge.fury.io/js/%40opentelemetry%2Fplugin-xml-http-request.svg +[dependencies-image]: https://david-dm.org/open-telemetry/opentelemetry-js/status.svg?path=packages/opentelemetry-instrumentation-xml-http-request +[dependencies-url]: https://david-dm.org/open-telemetry/opentelemetry-js?path=packages%2Fopentelemetry-instrumentation-xml-http-request +[devDependencies-image]: https://david-dm.org/open-telemetry/opentelemetry-js/dev-status.svg?path=packages/opentelemetry-instrumentation-xml-http-request +[devDependencies-url]: https://david-dm.org/open-telemetry/opentelemetry-js?path=packages%2Fopentelemetry-instrumentation-xml-http-request&type=dev +[npm-url]: https://www.npmjs.com/package/@opentelemetry/instrumentation-xml-http-request +[npm-img]: https://badge.fury.io/js/%40opentelemetry%2Finstrumentation-xml-http-request.svg diff --git a/packages/opentelemetry-plugin-xml-http-request/images/cors.jpg b/packages/opentelemetry-instrumentation-xml-http-request/images/cors.jpg similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/images/cors.jpg rename to packages/opentelemetry-instrumentation-xml-http-request/images/cors.jpg diff --git a/packages/opentelemetry-plugin-xml-http-request/images/main.jpg b/packages/opentelemetry-instrumentation-xml-http-request/images/main.jpg similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/images/main.jpg rename to packages/opentelemetry-instrumentation-xml-http-request/images/main.jpg diff --git a/packages/opentelemetry-plugin-xml-http-request/images/request.jpg b/packages/opentelemetry-instrumentation-xml-http-request/images/request.jpg similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/images/request.jpg rename to packages/opentelemetry-instrumentation-xml-http-request/images/request.jpg diff --git a/packages/opentelemetry-plugin-xml-http-request/karma.conf.js b/packages/opentelemetry-instrumentation-xml-http-request/karma.conf.js similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/karma.conf.js rename to packages/opentelemetry-instrumentation-xml-http-request/karma.conf.js diff --git a/packages/opentelemetry-plugin-xml-http-request/package.json b/packages/opentelemetry-instrumentation-xml-http-request/package.json similarity index 93% rename from packages/opentelemetry-plugin-xml-http-request/package.json rename to packages/opentelemetry-instrumentation-xml-http-request/package.json index 5075e15c4d..cc9739ee30 100644 --- a/packages/opentelemetry-plugin-xml-http-request/package.json +++ b/packages/opentelemetry-instrumentation-xml-http-request/package.json @@ -1,5 +1,5 @@ { - "name": "@opentelemetry/plugin-xml-http-request", + "name": "@opentelemetry/instrumentation-xml-http-request", "version": "0.12.0", "description": "OpenTelemetry XMLHttpRequest automatic instrumentation package.", "main": "build/src/index.js", @@ -50,7 +50,6 @@ "@opentelemetry/tracing": "^0.12.0", "@types/mocha": "8.0.2", "@types/node": "14.0.27", - "@types/shimmer": "1.0.1", "@types/sinon": "9.0.4", "@types/webpack-env": "1.15.2", "babel-loader": "8.1.0", @@ -77,9 +76,9 @@ }, "dependencies": { "@opentelemetry/api": "^0.12.0", + "@opentelemetry/instrumentation": "^0.12.0", "@opentelemetry/core": "^0.12.0", "@opentelemetry/semantic-conventions": "^0.12.0", - "@opentelemetry/web": "^0.12.0", - "shimmer": "^1.2.1" + "@opentelemetry/web": "^0.12.0" } } diff --git a/packages/opentelemetry-plugin-xml-http-request/src/enums/EventNames.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/enums/EventNames.ts similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/src/enums/EventNames.ts rename to packages/opentelemetry-instrumentation-xml-http-request/src/enums/EventNames.ts diff --git a/packages/opentelemetry-plugin-xml-http-request/src/index.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/index.ts similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/src/index.ts rename to packages/opentelemetry-instrumentation-xml-http-request/src/index.ts diff --git a/packages/opentelemetry-plugin-xml-http-request/src/types.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/types.ts similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/src/types.ts rename to packages/opentelemetry-instrumentation-xml-http-request/src/types.ts diff --git a/packages/opentelemetry-plugin-xml-http-request/src/version.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/version.ts similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/src/version.ts rename to packages/opentelemetry-instrumentation-xml-http-request/src/version.ts diff --git a/packages/opentelemetry-plugin-xml-http-request/src/xhr.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts similarity index 87% rename from packages/opentelemetry-plugin-xml-http-request/src/xhr.ts rename to packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 8252b15fc8..e056bed636 100644 --- a/packages/opentelemetry-plugin-xml-http-request/src/xhr.ts +++ b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -16,12 +16,11 @@ import * as api from '@opentelemetry/api'; import { - BasePlugin, - hrTime, - isUrlIgnored, isWrapped, - otperformance, -} from '@opentelemetry/core'; + InstrumentationBase, + InstrumentationConfig, +} from '@opentelemetry/instrumentation'; +import { hrTime, isUrlIgnored, otperformance } from '@opentelemetry/core'; import { HttpAttribute } from '@opentelemetry/semantic-conventions'; import { addSpanNetworkEvents, @@ -30,7 +29,6 @@ import { PerformanceTimingNames as PTN, shouldPropagateTraceHeaders, } from '@opentelemetry/web'; -import * as shimmer from 'shimmer'; import { EventNames } from './enums/EventNames'; import { OpenFunction, @@ -49,7 +47,8 @@ const OBSERVER_WAIT_TIME_MS = 300; /** * XMLHttpRequest config */ -export interface XMLHttpRequestPluginConfig extends api.PluginConfig { +export interface XMLHttpRequestInstrumentationConfig + extends InstrumentationConfig { // the number of timing resources is limited, after the limit // (chrome 250, safari 150) the information is not collected anymore // the only way to prevent that is to regularly clean the resources @@ -58,12 +57,20 @@ export interface XMLHttpRequestPluginConfig extends api.PluginConfig { clearTimingResources?: boolean; // urls which should include trace headers when origin doesn't match propagateTraceHeaderCorsUrls?: PropagateTraceHeaderCorsUrls; + /** + * URLs that partially match any regex in ignoreUrls will not be traced. + * In addition, URLs that are _exact matches_ of strings in ignoreUrls will + * also not be traced. + */ + ignoreUrls?: Array; } /** * This class represents a XMLHttpRequest plugin for auto instrumentation */ -export class XMLHttpRequestPlugin extends BasePlugin { +export class XMLHttpRequestInstrumentation extends InstrumentationBase< + XMLHttpRequest +> { readonly component: string = 'xml-http-request'; readonly version: string = VERSION; moduleName = this.component; @@ -72,8 +79,20 @@ export class XMLHttpRequestPlugin extends BasePlugin { private _xhrMem = new WeakMap(); private _usedResources = new WeakSet(); - constructor(protected _config: XMLHttpRequestPluginConfig = {}) { - super('@opentelemetry/plugin-xml-http-request', VERSION); + constructor( + config: XMLHttpRequestInstrumentationConfig & InstrumentationConfig = {} + ) { + super( + '@opentelemetry/instrumentation-xml-http-request', + VERSION, + Object.assign({}, config) + ); + } + + init() {} + + private _getConfig(): XMLHttpRequestInstrumentationConfig { + return this._config; } /** @@ -86,7 +105,7 @@ export class XMLHttpRequestPlugin extends BasePlugin { if ( !shouldPropagateTraceHeaders( spanUrl, - this._config.propagateTraceHeaderCorsUrls + this._getConfig().propagateTraceHeaderCorsUrls ) ) { return; @@ -108,8 +127,8 @@ export class XMLHttpRequestPlugin extends BasePlugin { span: api.Span, corsPreFlightRequest: PerformanceResourceTiming ): void { - this._tracer.withSpan(span, () => { - const childSpan = this._tracer.startSpan('CORS Preflight', { + this.tracer.withSpan(span, () => { + const childSpan = this.tracer.startSpan('CORS Preflight', { startTime: corsPreFlightRequest[PTN.FETCH_START], }); addSpanNetworkEvents(childSpan, corsPreFlightRequest); @@ -177,12 +196,12 @@ export class XMLHttpRequestPlugin extends BasePlugin { /** * Clears the resource timings and all resources assigned with spans - * when {@link XMLHttpRequestPluginConfig.clearTimingResources} is + * when {@link XMLHttpRequestInstrumentationConfig.clearTimingResources} is * set to true (default false) * @private */ private _clearResources() { - if (this._tasksCount === 0 && this._config.clearTimingResources) { + if (this._tasksCount === 0 && this._getConfig().clearTimingResources) { ((otperformance as unknown) as Performance).clearResourceTimings(); this._xhrMem = new WeakMap(); this._usedResources = new WeakSet(); @@ -268,13 +287,13 @@ export class XMLHttpRequestPlugin extends BasePlugin { url: string, method: string ): api.Span | undefined { - if (isUrlIgnored(url, this._config.ignoreUrls)) { + if (isUrlIgnored(url, this._getConfig().ignoreUrls)) { this._logger.debug('ignoring span as url matches ignored url'); return; } const spanName = `HTTP ${method.toUpperCase()}`; - const currentSpan = this._tracer.startSpan(spanName, { + const currentSpan = this.tracer.startSpan(spanName, { kind: api.SpanKind.CLIENT, attributes: { [HttpAttribute.HTTP_METHOD]: method, @@ -417,7 +436,7 @@ export class XMLHttpRequestPlugin extends BasePlugin { const spanUrl = xhrMem.spanUrl; if (currentSpan && spanUrl) { - plugin._tracer.withSpan(currentSpan, () => { + plugin.tracer.withSpan(currentSpan, () => { plugin._tasksCount++; xhrMem.sendStartTime = hrTime(); currentSpan.addEvent(EventNames.METHOD_SEND); @@ -443,35 +462,33 @@ export class XMLHttpRequestPlugin extends BasePlugin { } /** - * implements patch function + * implements enable function */ - protected patch() { + enable() { this._logger.debug('applying patch to', this.moduleName, this.version); if (isWrapped(XMLHttpRequest.prototype.open)) { - shimmer.unwrap(XMLHttpRequest.prototype, 'open'); + this._unwrap(XMLHttpRequest.prototype, 'open'); this._logger.debug('removing previous patch from method open'); } if (isWrapped(XMLHttpRequest.prototype.send)) { - shimmer.unwrap(XMLHttpRequest.prototype, 'send'); + this._unwrap(XMLHttpRequest.prototype, 'send'); this._logger.debug('removing previous patch from method send'); } - shimmer.wrap(XMLHttpRequest.prototype, 'open', this._patchOpen()); - shimmer.wrap(XMLHttpRequest.prototype, 'send', this._patchSend()); - - return this._moduleExports; + this._wrap(XMLHttpRequest.prototype, 'open', this._patchOpen()); + this._wrap(XMLHttpRequest.prototype, 'send', this._patchSend()); } /** - * implements unpatch function + * implements disable function */ - protected unpatch() { + disable() { this._logger.debug('removing patch from', this.moduleName, this.version); - shimmer.unwrap(XMLHttpRequest.prototype, 'open'); - shimmer.unwrap(XMLHttpRequest.prototype, 'send'); + this._unwrap(XMLHttpRequest.prototype, 'open'); + this._unwrap(XMLHttpRequest.prototype, 'send'); this._tasksCount = 0; this._xhrMem = new WeakMap(); diff --git a/packages/opentelemetry-plugin-xml-http-request/test/index-webpack.ts b/packages/opentelemetry-instrumentation-xml-http-request/test/index-webpack.ts similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/test/index-webpack.ts rename to packages/opentelemetry-instrumentation-xml-http-request/test/index-webpack.ts diff --git a/packages/opentelemetry-plugin-xml-http-request/test/unmocked.test.ts b/packages/opentelemetry-instrumentation-xml-http-request/test/unmocked.test.ts similarity index 95% rename from packages/opentelemetry-plugin-xml-http-request/test/unmocked.test.ts rename to packages/opentelemetry-instrumentation-xml-http-request/test/unmocked.test.ts index 432c184e80..61e6846f77 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/unmocked.test.ts +++ b/packages/opentelemetry-instrumentation-xml-http-request/test/unmocked.test.ts @@ -17,7 +17,7 @@ import { Span } from '@opentelemetry/api'; import { HttpAttribute } from '@opentelemetry/semantic-conventions'; import { ReadableSpan, SpanProcessor } from '@opentelemetry/tracing'; import { WebTracerProvider } from '@opentelemetry/web'; -import { XMLHttpRequestPlugin } from '../src'; +import { XMLHttpRequestInstrumentation } from '../src'; import assert = require('assert'); class TestSpanProcessor implements SpanProcessor { @@ -41,7 +41,7 @@ describe('unmocked xhr', () => { let provider: WebTracerProvider; beforeEach(() => { provider = new WebTracerProvider({ - plugins: [new XMLHttpRequestPlugin()], + plugins: [new XMLHttpRequestInstrumentation()], }); testSpans = new TestSpanProcessor(); provider.addSpanProcessor(testSpans); diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts similarity index 98% rename from packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts rename to packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts index eb769068c8..133f45d8a6 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts @@ -18,7 +18,6 @@ import { LogLevel, otperformance as performance, isWrapped, - NoopLogger, } from '@opentelemetry/core'; import { B3Propagator, @@ -38,7 +37,7 @@ import { import * as assert from 'assert'; import * as sinon from 'sinon'; import { EventNames } from '../src/enums/EventNames'; -import { XMLHttpRequestPlugin } from '../src/xhr'; +import { XMLHttpRequestInstrumentation } from '../src/xhr'; class DummySpanExporter implements tracing.SpanExporter { export(spans: any) {} @@ -161,7 +160,7 @@ describe('xhr', () => { let spyEntries: any; const url = 'http://localhost:8090/xml-http-request.js'; let fakeNow = 0; - let xmlHttpRequestPlugin: XMLHttpRequestPlugin; + let xmlHttpRequestInstrumentation: XMLHttpRequestInstrumentation; clearData = () => { requests = []; @@ -192,10 +191,12 @@ describe('xhr', () => { spyEntries = sandbox.stub(performance, 'getEntriesByType'); spyEntries.withArgs('resource').returns(resources); - xmlHttpRequestPlugin = new XMLHttpRequestPlugin(config); + xmlHttpRequestInstrumentation = new XMLHttpRequestInstrumentation( + config + ); webTracerProviderWithZone = new WebTracerProvider({ logLevel: LogLevel.ERROR, - plugins: [xmlHttpRequestPlugin], + plugins: [xmlHttpRequestInstrumentation], }); webTracerWithZone = webTracerProviderWithZone.getTracer('xhr-test'); dummySpanExporter = new DummySpanExporter(); @@ -244,18 +245,14 @@ describe('xhr', () => { it('should patch to wrap XML HTTP Requests when enabled', () => { const xhttp = new XMLHttpRequest(); assert.ok(isWrapped(xhttp.send)); - xmlHttpRequestPlugin.enable( - XMLHttpRequest.prototype, - new api.NoopTracerProvider(), - new NoopLogger() - ); + xmlHttpRequestInstrumentation.enable(); assert.ok(isWrapped(xhttp.send)); }); it('should unpatch to unwrap XML HTTP Requests when disabled', () => { const xhttp = new XMLHttpRequest(); assert.ok(isWrapped(xhttp.send)); - xmlHttpRequestPlugin.disable(); + xmlHttpRequestInstrumentation.disable(); assert.ok(!isWrapped(xhttp.send)); }); @@ -717,7 +714,7 @@ describe('xhr', () => { webTracerWithZoneProvider = new WebTracerProvider({ logLevel: LogLevel.ERROR, - plugins: [new XMLHttpRequestPlugin()], + plugins: [new XMLHttpRequestInstrumentation()], }); dummySpanExporter = new DummySpanExporter(); exportSpy = sinon.stub(dummySpanExporter, 'export'); diff --git a/packages/opentelemetry-plugin-xml-http-request/tsconfig.json b/packages/opentelemetry-instrumentation-xml-http-request/tsconfig.json similarity index 100% rename from packages/opentelemetry-plugin-xml-http-request/tsconfig.json rename to packages/opentelemetry-instrumentation-xml-http-request/tsconfig.json diff --git a/packages/opentelemetry-instrumentation/src/instrumentation.ts b/packages/opentelemetry-instrumentation/src/instrumentation.ts index e6ee7c77c5..72748d4e40 100644 --- a/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -16,6 +16,7 @@ import * as api from '@opentelemetry/api'; import * as shimmer from 'shimmer'; +import { InstrumentationModuleDefinition } from './platform/node'; import * as types from './types'; /** @@ -97,4 +98,13 @@ export abstract class InstrumentationAbstract /* Enable plugin */ public abstract disable(): void; + + /** + * Init method in which plugin should define _modules and patches for + * methods + */ + protected abstract init(): + | InstrumentationModuleDefinition + | InstrumentationModuleDefinition[] + | void; } diff --git a/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts index a9a58757bc..d4acd2c7bb 100644 --- a/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -177,12 +177,4 @@ export abstract class InstrumentationBase } } } - - /** - * Init method in which plugin should define _modules and patches for - * methods - */ - protected abstract init(): - | InstrumentationModuleDefinition - | InstrumentationModuleDefinition[]; } diff --git a/packages/opentelemetry-instrumentation/test/common/Instrumentation.test.ts b/packages/opentelemetry-instrumentation/test/common/Instrumentation.test.ts index 8244f01031..62d43b75d6 100644 --- a/packages/opentelemetry-instrumentation/test/common/Instrumentation.test.ts +++ b/packages/opentelemetry-instrumentation/test/common/Instrumentation.test.ts @@ -24,6 +24,7 @@ class TestInstrumentation extends InstrumentationAbstract { } enable() {} disable() {} + init() {} } describe('BaseInstrumentation', () => { diff --git a/packages/opentelemetry-web/package.json b/packages/opentelemetry-web/package.json index 2fa66276d4..cccdcbe70b 100644 --- a/packages/opentelemetry-web/package.json +++ b/packages/opentelemetry-web/package.json @@ -79,6 +79,7 @@ "@opentelemetry/api": "^0.12.0", "@opentelemetry/context-base": "^0.12.0", "@opentelemetry/core": "^0.12.0", + "@opentelemetry/instrumentation": "^0.12.0", "@opentelemetry/semantic-conventions": "^0.12.0", "@opentelemetry/tracing": "^0.12.0" } diff --git a/packages/opentelemetry-web/src/WebTracerProvider.ts b/packages/opentelemetry-web/src/WebTracerProvider.ts index 79d23d938d..ff231a1e38 100644 --- a/packages/opentelemetry-web/src/WebTracerProvider.ts +++ b/packages/opentelemetry-web/src/WebTracerProvider.ts @@ -15,6 +15,7 @@ */ import { BasePlugin } from '@opentelemetry/core'; +import { InstrumentationBase } from '@opentelemetry/instrumentation/build/src/platform/browser'; import { BasicTracerProvider, SDKRegistrationConfig, @@ -29,7 +30,7 @@ export interface WebTracerConfig extends TracerConfig { /** * plugins to be used with tracer, they will be enabled automatically */ - plugins?: BasePlugin[]; + plugins?: (BasePlugin | InstrumentationBase)[]; } /** @@ -47,7 +48,12 @@ export class WebTracerProvider extends BasicTracerProvider { super(config); for (const plugin of config.plugins) { - plugin.enable([], this, this.logger); + if (plugin instanceof InstrumentationBase) { + plugin.setTracerProvider(this); + plugin.enable(); + } else { + plugin.enable([], this, this.logger); + } } if ((config as SDKRegistrationConfig).contextManager) { diff --git a/packages/opentelemetry-web/test/WebTracerProvider.test.ts b/packages/opentelemetry-web/test/WebTracerProvider.test.ts index 00a3d2a6b9..281f901fc5 100644 --- a/packages/opentelemetry-web/test/WebTracerProvider.test.ts +++ b/packages/opentelemetry-web/test/WebTracerProvider.test.ts @@ -18,6 +18,7 @@ import { context } from '@opentelemetry/api'; import { ContextManager } from '@opentelemetry/context-base'; import { ZoneContextManager } from '@opentelemetry/context-zone'; import { BasePlugin, NoopLogger } from '@opentelemetry/core'; +import { InstrumentationBase } from '@opentelemetry/instrumentation'; import { B3Propagator } from '@opentelemetry/propagator-b3'; import { Resource, TELEMETRY_SDK_RESOURCE } from '@opentelemetry/resources'; import { Span, Tracer } from '@opentelemetry/tracing'; @@ -36,6 +37,15 @@ class DummyPlugin extends BasePlugin { unpatch() {} } +class DummyInstrumentation extends InstrumentationBase { + constructor() { + super('dummy', '1'); + } + enable() {} + disable() {} + init() {} +} + describe('WebTracerProvider', () => { describe('constructor', () => { let defaultOptions: WebTracerConfig; @@ -62,16 +72,21 @@ describe('WebTracerProvider', () => { it('should enable all plugins', () => { const dummyPlugin1 = new DummyPlugin(); const dummyPlugin2 = new DummyPlugin(); + const dummyPlugin3 = new DummyInstrumentation(); const spyEnable1 = sinon.spy(dummyPlugin1, 'enable'); const spyEnable2 = sinon.spy(dummyPlugin2, 'enable'); + const spyEnable3 = sinon.spy(dummyPlugin3, 'enable'); + const spySetTracerProvider = sinon.spy(dummyPlugin3, 'setTracerProvider'); - const plugins = [dummyPlugin1, dummyPlugin2]; + const plugins = [dummyPlugin1, dummyPlugin2, dummyPlugin3]; const options = { plugins }; new WebTracerProvider(options); assert.ok(spyEnable1.calledOnce === true); assert.ok(spyEnable2.calledOnce === true); + assert.ok(spyEnable3.calledOnce === true); + assert.ok(spySetTracerProvider.calledOnce === true); }); it('should work without default context manager', () => { From 14c7ce4cc2d82bf822f538d8ddca2bf49c8720cc Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Mon, 9 Nov 2020 21:04:22 +0100 Subject: [PATCH 2/2] feat: fixing import and checking for instrumentation based on function instead of instance --- packages/opentelemetry-web/src/WebTracerProvider.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-web/src/WebTracerProvider.ts b/packages/opentelemetry-web/src/WebTracerProvider.ts index ff231a1e38..9d6fbd455c 100644 --- a/packages/opentelemetry-web/src/WebTracerProvider.ts +++ b/packages/opentelemetry-web/src/WebTracerProvider.ts @@ -15,7 +15,7 @@ */ import { BasePlugin } from '@opentelemetry/core'; -import { InstrumentationBase } from '@opentelemetry/instrumentation/build/src/platform/browser'; +import { InstrumentationBase } from '@opentelemetry/instrumentation'; import { BasicTracerProvider, SDKRegistrationConfig, @@ -48,9 +48,10 @@ export class WebTracerProvider extends BasicTracerProvider { super(config); for (const plugin of config.plugins) { - if (plugin instanceof InstrumentationBase) { - plugin.setTracerProvider(this); - plugin.enable(); + const instrumentation = (plugin as unknown) as InstrumentationBase; + if (typeof instrumentation.setTracerProvider === 'function') { + instrumentation.setTracerProvider(this); + instrumentation.enable(); } else { plugin.enable([], this, this.logger); }