From ef2b6f76feb36e9c6224e3ed4b4809fae85fef59 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 24 Apr 2020 19:06:21 +0200 Subject: [PATCH 1/3] chore: splitting BasePlugin into browser and node to avoid dependency from node in browser --- packages/opentelemetry-core/karma.conf.js | 4 +- packages/opentelemetry-core/package.json | 2 +- packages/opentelemetry-core/src/index.ts | 1 - .../src/platform/BaseAbstractPlugin.ts | 59 +++++++++++++++++++ .../src/platform/browser/BasePlugin.ts | 43 ++++++++++++++ .../src/platform/browser/index.ts | 1 + .../node}/BasePlugin.ts | 23 ++------ .../src/platform/node/index.ts | 1 + .../opentelemetry-core/test/index-webpack.ts | 11 +++- .../{trace => platform}/BasePlugin.test.ts | 7 +-- .../test/platform/browser/BasePlugin.test.ts | 46 +++++++++++++++ 11 files changed, 168 insertions(+), 30 deletions(-) create mode 100644 packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts create mode 100644 packages/opentelemetry-core/src/platform/browser/BasePlugin.ts rename packages/opentelemetry-core/src/{trace/instrumentation => platform/node}/BasePlugin.ts (84%) rename packages/opentelemetry-core/test/{trace => platform}/BasePlugin.test.ts (91%) create mode 100644 packages/opentelemetry-core/test/platform/browser/BasePlugin.test.ts diff --git a/packages/opentelemetry-core/karma.conf.js b/packages/opentelemetry-core/karma.conf.js index 7183aab033..7b8c9678db 100644 --- a/packages/opentelemetry-core/karma.conf.js +++ b/packages/opentelemetry-core/karma.conf.js @@ -1,5 +1,5 @@ /*! - * Copyright 2019, OpenTelemetry Authors + * Copyright 2020, OpenTelemetry Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,6 @@ const karmaBaseConfig = require('../../karma.base'); module.exports = (config) => { config.set(Object.assign({}, karmaBaseConfig, { - webpack: karmaWebpackConfig + webpack: karmaWebpackConfig, })) }; diff --git a/packages/opentelemetry-core/package.json b/packages/opentelemetry-core/package.json index 0b1b3cee36..555e123808 100644 --- a/packages/opentelemetry-core/package.json +++ b/packages/opentelemetry-core/package.json @@ -10,7 +10,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 --exclude 'test/platform/browser/**/*.ts'", "test:browser": "nyc karma start --single-run", "tdd": "npm run tdd:node", "tdd:node": "npm run test -- --watch-extensions ts --watch", diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index 60b8b04f92..f236d9ceae 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -25,7 +25,6 @@ export * from './context/propagation/composite'; export * from './context/propagation/HttpTraceContext'; export * from './context/propagation/types'; export * from './platform'; -export * from './trace/instrumentation/BasePlugin'; export * from './trace/NoRecordingSpan'; export * from './trace/sampler/ProbabilitySampler'; export * from './trace/spancontext-utils'; diff --git a/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts b/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts new file mode 100644 index 0000000000..8514498e9a --- /dev/null +++ b/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts @@ -0,0 +1,59 @@ +/*! + * Copyright 2020, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + Tracer, + Plugin, + Logger, + PluginConfig, + TracerProvider, + PluginInternalFiles, +} from '@opentelemetry/api'; + +/** This class represent the base to patch plugin. */ +export abstract class BaseAbstractPlugin implements Plugin { + abstract readonly moduleName: string; // required for internalFilesExports + supportedVersions?: string[]; + readonly version?: string; // required for internalFilesExports + protected readonly _basedir?: string; // required for internalFilesExports + + protected _moduleExports!: T; + protected _internalFilesExports!: { [module: string]: unknown }; // output for internalFilesExports + protected readonly _internalFilesList?: PluginInternalFiles; // required for internalFilesExports + + protected _tracer!: Tracer; + protected _logger!: Logger; + protected _config!: PluginConfig; + + constructor( + protected readonly _tracerName: string, + protected readonly _tracerVersion?: string + ) {} + + disable(): void { + this.unpatch(); + } + + abstract enable( + moduleExports: T, + TracerProvider: TracerProvider, + logger: Logger, + config?: PluginConfig + ): T; + + protected abstract patch(): T; + protected abstract unpatch(): void; +} diff --git a/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts b/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts new file mode 100644 index 0000000000..f9b99d05cb --- /dev/null +++ b/packages/opentelemetry-core/src/platform/browser/BasePlugin.ts @@ -0,0 +1,43 @@ +/*! + * Copyright 2020, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + Logger, + Plugin, + PluginConfig, + TracerProvider, +} from '@opentelemetry/api'; +import { BaseAbstractPlugin } from '../BaseAbstractPlugin'; + +/** This class represent the base to patch plugin. */ +export abstract class BasePlugin extends BaseAbstractPlugin + implements Plugin { + enable( + moduleExports: T, + tracerProvider: TracerProvider, + logger: Logger, + config?: PluginConfig + ): T { + this._moduleExports = moduleExports; + this._tracer = tracerProvider.getTracer( + this._tracerName, + this._tracerVersion + ); + this._logger = logger; + if (config) this._config = config; + return this.patch(); + } +} diff --git a/packages/opentelemetry-core/src/platform/browser/index.ts b/packages/opentelemetry-core/src/platform/browser/index.ts index 4668bb35aa..1dce1d3df9 100644 --- a/packages/opentelemetry-core/src/platform/browser/index.ts +++ b/packages/opentelemetry-core/src/platform/browser/index.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +export * from './BasePlugin'; export * from './id'; export * from './performance'; export * from './timer-util'; diff --git a/packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts b/packages/opentelemetry-core/src/platform/node/BasePlugin.ts similarity index 84% rename from packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts rename to packages/opentelemetry-core/src/platform/node/BasePlugin.ts index 601939df36..f3b713bf67 100644 --- a/packages/opentelemetry-core/src/trace/instrumentation/BasePlugin.ts +++ b/packages/opentelemetry-core/src/platform/node/BasePlugin.ts @@ -15,7 +15,6 @@ */ import { - Tracer, Plugin, Logger, PluginConfig, @@ -25,26 +24,11 @@ import { } from '@opentelemetry/api'; import * as semver from 'semver'; import * as path from 'path'; +import { BaseAbstractPlugin } from '../BaseAbstractPlugin'; /** This class represent the base to patch plugin. */ -export abstract class BasePlugin implements Plugin { - supportedVersions?: string[]; - abstract readonly moduleName: string; // required for internalFilesExports - readonly version?: string; // required for internalFilesExports - protected readonly _basedir?: string; // required for internalFilesExports - - protected _moduleExports!: T; - protected _tracer!: Tracer; - protected _logger!: Logger; - protected _internalFilesExports!: { [module: string]: unknown }; // output for internalFilesExports - protected readonly _internalFilesList?: PluginInternalFiles; // required for internalFilesExports - protected _config!: PluginConfig; - - constructor( - protected readonly _tracerName: string, - protected readonly _tracerVersion?: string - ) {} - +export abstract class BasePlugin extends BaseAbstractPlugin + implements Plugin { enable( moduleExports: T, tracerProvider: TracerProvider, @@ -147,5 +131,6 @@ export abstract class BasePlugin implements Plugin { } protected abstract patch(): T; + protected abstract unpatch(): void; } diff --git a/packages/opentelemetry-core/src/platform/node/index.ts b/packages/opentelemetry-core/src/platform/node/index.ts index 4668bb35aa..1dce1d3df9 100644 --- a/packages/opentelemetry-core/src/platform/node/index.ts +++ b/packages/opentelemetry-core/src/platform/node/index.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +export * from './BasePlugin'; export * from './id'; export * from './performance'; export * from './timer-util'; diff --git a/packages/opentelemetry-core/test/index-webpack.ts b/packages/opentelemetry-core/test/index-webpack.ts index 7731f09091..2c89bb37ee 100644 --- a/packages/opentelemetry-core/test/index-webpack.ts +++ b/packages/opentelemetry-core/test/index-webpack.ts @@ -1,5 +1,5 @@ /*! - * Copyright 2019, OpenTelemetry Authors + * Copyright 2020, OpenTelemetry Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,8 +16,13 @@ // This file is the webpack entry point for the browser Karma tests. It requires // all modules ending in "test" from the current folder and all its subfolders. -const testsContext = require.context('.', true, /test$/); -testsContext.keys().forEach(testsContext); +const testsContextCommon = require.context('.', true, /test$/); +testsContextCommon.keys().forEach(key => { + if (key.indexOf('./platform/BasePlugin.test') >= 0) { + return function() {}; + } + return testsContextCommon(key); +}); const srcContext = require.context('.', true, /src$/); srcContext.keys().forEach(srcContext); diff --git a/packages/opentelemetry-core/test/trace/BasePlugin.test.ts b/packages/opentelemetry-core/test/platform/BasePlugin.test.ts similarity index 91% rename from packages/opentelemetry-core/test/trace/BasePlugin.test.ts rename to packages/opentelemetry-core/test/platform/BasePlugin.test.ts index 643b7562dd..115c07627c 100644 --- a/packages/opentelemetry-core/test/trace/BasePlugin.test.ts +++ b/packages/opentelemetry-core/test/platform/BasePlugin.test.ts @@ -18,15 +18,14 @@ import { NoopTracerProvider } from '@opentelemetry/api'; import * as assert from 'assert'; import * as path from 'path'; import { BasePlugin, NoopLogger } from '../../src'; -import * as types from './fixtures/test-package/foo/bar/internal'; +import * as types from '../trace/fixtures/test-package/foo/bar/internal'; const provider = new NoopTracerProvider(); const logger = new NoopLogger(); - describe('BasePlugin', () => { describe('internalFilesLoader', () => { it('should load internally exported files', () => { - const testPackage = require('./fixtures/test-package'); + const testPackage = require('../trace/fixtures/test-package'); const plugin = new TestPlugin(); assert.doesNotThrow(() => { plugin.enable(testPackage, provider, logger); @@ -81,4 +80,4 @@ class TestPlugin extends BasePlugin<{ [key: string]: Function }> { protected unpatch(): void {} } -const basedir = path.dirname(require.resolve('./fixtures/test-package')); +const basedir = path.dirname(require.resolve('../trace/fixtures/test-package')); diff --git a/packages/opentelemetry-core/test/platform/browser/BasePlugin.test.ts b/packages/opentelemetry-core/test/platform/browser/BasePlugin.test.ts new file mode 100644 index 0000000000..a98e9bd1f1 --- /dev/null +++ b/packages/opentelemetry-core/test/platform/browser/BasePlugin.test.ts @@ -0,0 +1,46 @@ +/** + * Copyright 2020, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { NOOP_TRACER, NoopTracerProvider } from '@opentelemetry/api'; +import * as assert from 'assert'; +import { BasePlugin, NoopLogger } from '../../../src'; + +const provider = new NoopTracerProvider(); +const logger = new NoopLogger(); +describe('BasePlugin', () => { + describe('enable', () => { + it('should enable plugin', () => { + const moduleExports = { foo: function() {} }; + const plugin = new TestPlugin('foo', '1'); + const patch = plugin.enable(moduleExports, provider, logger); + + assert.ok(plugin['_tracer'] === NOOP_TRACER); + assert.ok(plugin['_logger'] === logger); + assert.ok(patch === moduleExports); + }); + }); +}); + +class TestPlugin extends BasePlugin<{ [key: string]: Function }> { + readonly moduleName = 'test-package'; + readonly version = '0.1.0'; + + patch(): { [key: string]: Function } { + return this._moduleExports; + } + + protected unpatch(): void {} +} From 03ccab9a8011265a105de1e21510873de2d3622a Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Tue, 28 Apr 2020 12:36:58 +0200 Subject: [PATCH 2/3] chore: rearrange the params, typo --- .../src/platform/BaseAbstractPlugin.ts | 13 ++++++------- .../test/platform/browser/BasePlugin.test.ts | 8 +++++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts b/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts index 8514498e9a..2a15ebbe63 100644 --- a/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts +++ b/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts @@ -28,15 +28,14 @@ export abstract class BaseAbstractPlugin implements Plugin { abstract readonly moduleName: string; // required for internalFilesExports supportedVersions?: string[]; readonly version?: string; // required for internalFilesExports - protected readonly _basedir?: string; // required for internalFilesExports - protected _moduleExports!: T; + protected readonly _basedir?: string; // required for internalFilesExports + protected _config!: PluginConfig; protected _internalFilesExports!: { [module: string]: unknown }; // output for internalFilesExports protected readonly _internalFilesList?: PluginInternalFiles; // required for internalFilesExports - - protected _tracer!: Tracer; protected _logger!: Logger; - protected _config!: PluginConfig; + protected _moduleExports!: T; + protected _tracer!: Tracer; constructor( protected readonly _tracerName: string, @@ -49,8 +48,8 @@ export abstract class BaseAbstractPlugin implements Plugin { abstract enable( moduleExports: T, - TracerProvider: TracerProvider, - logger: Logger, + tracerProvider: TracerProvider, + logger: Logger, config?: PluginConfig ): T; diff --git a/packages/opentelemetry-core/test/platform/browser/BasePlugin.test.ts b/packages/opentelemetry-core/test/platform/browser/BasePlugin.test.ts index a98e9bd1f1..cca6a23649 100644 --- a/packages/opentelemetry-core/test/platform/browser/BasePlugin.test.ts +++ b/packages/opentelemetry-core/test/platform/browser/BasePlugin.test.ts @@ -27,9 +27,11 @@ describe('BasePlugin', () => { const plugin = new TestPlugin('foo', '1'); const patch = plugin.enable(moduleExports, provider, logger); - assert.ok(plugin['_tracer'] === NOOP_TRACER); - assert.ok(plugin['_logger'] === logger); - assert.ok(patch === moduleExports); + assert.strictEqual(plugin['_tracer'], NOOP_TRACER); + assert.strictEqual(plugin['_tracerName'], 'foo'); + assert.strictEqual(plugin['_tracerVersion'], '1'); + assert.strictEqual(plugin['_logger'], logger); + assert.strictEqual(patch, moduleExports); }); }); }); From 14783111ff555741ab4fc88d2698a79676b72225 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Tue, 28 Apr 2020 12:54:40 +0200 Subject: [PATCH 3/3] chore: lint --- packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts b/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts index 2a15ebbe63..57ec582d2d 100644 --- a/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts +++ b/packages/opentelemetry-core/src/platform/BaseAbstractPlugin.ts @@ -49,7 +49,7 @@ export abstract class BaseAbstractPlugin implements Plugin { abstract enable( moduleExports: T, tracerProvider: TracerProvider, - logger: Logger, + logger: Logger, config?: PluginConfig ): T;