From 69ae974719c75bd9ba1a4c0efcc16bd244d1b023 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 24 Feb 2021 10:51:30 -0500 Subject: [PATCH 1/4] chore: use semver to determine API compatibility --- src/api/context.ts | 30 ++----- src/api/diag.ts | 73 ++++++----------- src/api/global-utils.ts | 70 ---------------- src/api/propagation.ts | 31 ++----- src/api/trace.ts | 34 +++----- src/internal/global-utils.ts | 92 +++++++++++++++++++++ src/internal/semver.ts | 113 ++++++++++++++++++++++++++ test/api/api.test.ts | 1 - test/{api => internal}/global.test.ts | 66 +++++++++++++-- test/internal/semver.test.ts | 101 +++++++++++++++++++++++ 10 files changed, 417 insertions(+), 194 deletions(-) delete mode 100644 src/api/global-utils.ts create mode 100644 src/internal/global-utils.ts create mode 100644 src/internal/semver.ts rename test/{api => internal}/global.test.ts (58%) create mode 100644 test/internal/semver.test.ts diff --git a/src/api/context.ts b/src/api/context.ts index f5c10a93..d6ea2445 100644 --- a/src/api/context.ts +++ b/src/api/context.ts @@ -17,12 +17,12 @@ import { NoopContextManager } from '../context/NoopContextManager'; import { Context, ContextManager } from '../context/types'; import { - API_BACKWARDS_COMPATIBILITY_VERSION, - GLOBAL_CONTEXT_MANAGER_API_KEY, - makeGetter, - _global, -} from './global-utils'; + getGlobal, + registerGlobal, + unregisterGlobal, +} from '../internal/global-utils'; +const API_NAME = 'context'; const NOOP_CONTEXT_MANAGER = new NoopContextManager(); /** @@ -49,17 +49,7 @@ export class ContextAPI { public setGlobalContextManager( contextManager: ContextManager ): ContextManager { - if (_global[GLOBAL_CONTEXT_MANAGER_API_KEY]) { - // global context manager has already been set - return this._getContextManager(); - } - - _global[GLOBAL_CONTEXT_MANAGER_API_KEY] = makeGetter( - API_BACKWARDS_COMPATIBILITY_VERSION, - contextManager, - NOOP_CONTEXT_MANAGER - ); - + registerGlobal(API_NAME, contextManager); return contextManager; } @@ -98,16 +88,12 @@ export class ContextAPI { } private _getContextManager(): ContextManager { - return ( - _global[GLOBAL_CONTEXT_MANAGER_API_KEY]?.( - API_BACKWARDS_COMPATIBILITY_VERSION - ) ?? NOOP_CONTEXT_MANAGER - ); + return getGlobal(API_NAME) || NOOP_CONTEXT_MANAGER; } /** Disable and remove the global context manager */ public disable() { this._getContextManager().disable(); - delete _global[GLOBAL_CONTEXT_MANAGER_API_KEY]; + unregisterGlobal(API_NAME); } } diff --git a/src/api/diag.ts b/src/api/diag.ts index b32b6b25..a1ba6061 100644 --- a/src/api/diag.ts +++ b/src/api/diag.ts @@ -15,50 +15,29 @@ */ import { createLogLevelDiagLogger } from '../diag/internal/logLevelLogger'; -import { createNoopDiagLogger } from '../diag/internal/noopLogger'; import { DiagLogFunction, DiagLogger, DiagLogLevel } from '../diag/types'; import { - API_BACKWARDS_COMPATIBILITY_VERSION, - GLOBAL_DIAG_LOGGER_API_KEY, - makeGetter, - _global, -} from './global-utils'; + getGlobal, + registerGlobal, + unregisterGlobal, +} from '../internal/global-utils'; -function nop() {} - -/** Internal simple Noop Diag API that returns a noop logger and does not allow any changes */ -function noopDiagApi(): DiagAPI { - return Object.assign( - { disable: nop, setLogger: nop }, - createNoopDiagLogger() - ); -} +const API_NAME = 'diag'; /** * Singleton object which represents the entry point to the OpenTelemetry internal * diagnostic API */ export class DiagAPI implements DiagLogger { + private static _instance?: DiagAPI; + /** Get the singleton instance of the DiagAPI API */ public static instance(): DiagAPI { - let theInst = null; - if (_global[GLOBAL_DIAG_LOGGER_API_KEY]) { - // Looks like a previous instance was set, so try and fetch it - theInst = _global[GLOBAL_DIAG_LOGGER_API_KEY]?.( - API_BACKWARDS_COMPATIBILITY_VERSION - ) as DiagAPI; + if (!this._instance) { + this._instance = new DiagAPI(); } - if (!theInst) { - theInst = new DiagAPI(); - _global[GLOBAL_DIAG_LOGGER_API_KEY] = makeGetter( - API_BACKWARDS_COMPATIBILITY_VERSION, - theInst, - noopDiagApi() - ); - } - - return theInst; + return this._instance; } /** @@ -66,14 +45,13 @@ export class DiagAPI implements DiagLogger { * @private */ private constructor() { - let _filteredLogger: DiagLogger | undefined; - function _logProxy(funcName: keyof DiagLogger): DiagLogFunction { return function () { + const logger = getGlobal('diag'); // shortcut if logger not set - if (!_filteredLogger) return; - return _filteredLogger[funcName].apply( - _filteredLogger, + if (!logger) return; + return logger[funcName].apply( + logger, // work around Function.prototype.apply types // eslint-disable-next-line @typescript-eslint/no-explicit-any arguments as any @@ -91,24 +69,21 @@ export class DiagAPI implements DiagLogger { logLevel: DiagLogLevel = DiagLogLevel.INFO ) => { if (logger === self) { - if (_filteredLogger) { - const err = new Error( - 'Cannot use diag as the logger for itself. Please use a DiagLogger implementation like ConsoleDiagLogger or a custom implementation' - ); - _filteredLogger.error(err.stack ?? err.message); - logger = _filteredLogger; - } else { - // There isn't much we can do here. - // Logging to the console might break the user application. - return; - } + // There isn't much we can do here. + // Logging to the console might break the user application. + // Try to log to self. If a logger was previously registered it will receive the log. + const err = new Error( + 'Cannot use diag as the logger for itself. Please use a DiagLogger implementation like ConsoleDiagLogger or a custom implementation' + ); + self.error(err.stack ?? err.message); + return; } - _filteredLogger = createLogLevelDiagLogger(logLevel, logger); + registerGlobal('diag', createLogLevelDiagLogger(logLevel, logger), true); }; self.disable = () => { - _filteredLogger = undefined; + unregisterGlobal(API_NAME); }; self.verbose = _logProxy('verbose'); diff --git a/src/api/global-utils.ts b/src/api/global-utils.ts deleted file mode 100644 index 5c86e6ed..00000000 --- a/src/api/global-utils.ts +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright The 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 { DiagAPI } from '../api/diag'; -import { ContextManager } from '../context/types'; -import { _globalThis } from '../platform'; -import { TextMapPropagator } from '../propagation/TextMapPropagator'; -import { TracerProvider } from '../trace/tracer_provider'; - -export const GLOBAL_CONTEXT_MANAGER_API_KEY = Symbol.for( - 'io.opentelemetry.js.api.context' -); - -export const GLOBAL_PROPAGATION_API_KEY = Symbol.for( - 'io.opentelemetry.js.api.propagation' -); -export const GLOBAL_TRACE_API_KEY = Symbol.for('io.opentelemetry.js.api.trace'); - -export const GLOBAL_DIAG_LOGGER_API_KEY = Symbol.for( - 'io.opentelemetry.js.api.diag' -); - -type Get = (version: number) => T; -type OtelGlobal = Partial<{ - [GLOBAL_CONTEXT_MANAGER_API_KEY]: Get; - [GLOBAL_PROPAGATION_API_KEY]: Get; - [GLOBAL_TRACE_API_KEY]: Get; - [GLOBAL_DIAG_LOGGER_API_KEY]: Get; -}>; - -export const _global = _globalThis as OtelGlobal; - -/** - * Make a function which accepts a version integer and returns the instance of an API if the version - * is compatible, or a fallback version (usually NOOP) if it is not. - * - * @param requiredVersion Backwards compatibility version which is required to return the instance - * @param instance Instance which should be returned if the required version is compatible - * @param fallback Fallback instance, usually NOOP, which will be returned if the required version is not compatible - */ -export function makeGetter( - requiredVersion: number, - instance: T, - fallback: T -): Get { - return (version: number): T => - version === requiredVersion ? instance : fallback; -} - -/** - * A number which should be incremented each time a backwards incompatible - * change is made to the API. This number is used when an API package - * attempts to access the global API to ensure it is getting a compatible - * version. If the global API is not compatible with the API package - * attempting to get it, a NOOP API implementation will be returned. - */ -export const API_BACKWARDS_COMPATIBILITY_VERSION = 5; diff --git a/src/api/propagation.ts b/src/api/propagation.ts index dd214413..ee6394d2 100644 --- a/src/api/propagation.ts +++ b/src/api/propagation.ts @@ -24,11 +24,12 @@ import { TextMapSetter, } from '../propagation/TextMapPropagator'; import { - API_BACKWARDS_COMPATIBILITY_VERSION, - GLOBAL_PROPAGATION_API_KEY, - makeGetter, - _global, -} from './global-utils'; + getGlobal, + registerGlobal, + unregisterGlobal, +} from '../internal/global-utils'; + +const API_NAME = 'propagation'; /** * Singleton object which represents the entry point to the OpenTelemetry Propagation API @@ -52,17 +53,7 @@ export class PropagationAPI { * Set the current propagator. Returns the initialized propagator */ public setGlobalPropagator(propagator: TextMapPropagator): TextMapPropagator { - if (_global[GLOBAL_PROPAGATION_API_KEY]) { - // global propagator has already been set - return this._getGlobalPropagator(); - } - - _global[GLOBAL_PROPAGATION_API_KEY] = makeGetter( - API_BACKWARDS_COMPATIBILITY_VERSION, - propagator, - NOOP_TEXT_MAP_PROPAGATOR - ); - + registerGlobal(API_NAME, propagator); return propagator; } @@ -105,14 +96,10 @@ export class PropagationAPI { /** Remove the global propagator */ public disable() { - delete _global[GLOBAL_PROPAGATION_API_KEY]; + unregisterGlobal(API_NAME); } private _getGlobalPropagator(): TextMapPropagator { - return ( - _global[GLOBAL_PROPAGATION_API_KEY]?.( - API_BACKWARDS_COMPATIBILITY_VERSION - ) ?? NOOP_TEXT_MAP_PROPAGATOR - ); + return getGlobal(API_NAME) || NOOP_TEXT_MAP_PROPAGATOR; } } diff --git a/src/api/trace.ts b/src/api/trace.ts index f0c20055..f9beba0c 100644 --- a/src/api/trace.ts +++ b/src/api/trace.ts @@ -14,17 +14,17 @@ * limitations under the License. */ -import { NOOP_TRACER_PROVIDER } from '../trace/NoopTracerProvider'; import { ProxyTracerProvider } from '../trace/ProxyTracerProvider'; import { Tracer } from '../trace/tracer'; import { TracerProvider } from '../trace/tracer_provider'; import { isSpanContextValid } from '../trace/spancontext-utils'; import { - API_BACKWARDS_COMPATIBILITY_VERSION, - GLOBAL_TRACE_API_KEY, - makeGetter, - _global, -} from './global-utils'; + getGlobal, + registerGlobal, + unregisterGlobal, +} from '../internal/global-utils'; + +const API_NAME = 'trace'; /** * Singleton object which represents the entry point to the OpenTelemetry Tracing API @@ -50,30 +50,16 @@ export class TraceAPI { * Set the current global tracer. Returns the initialized global tracer provider */ public setGlobalTracerProvider(provider: TracerProvider): TracerProvider { - if (_global[GLOBAL_TRACE_API_KEY]) { - // global tracer provider has already been set - return this.getTracerProvider(); - } - this._proxyTracerProvider.setDelegate(provider); - - _global[GLOBAL_TRACE_API_KEY] = makeGetter( - API_BACKWARDS_COMPATIBILITY_VERSION, - this._proxyTracerProvider, - NOOP_TRACER_PROVIDER - ); - - return this.getTracerProvider(); + registerGlobal(API_NAME, this._proxyTracerProvider); + return this._proxyTracerProvider; } /** * Returns the global tracer provider. */ public getTracerProvider(): TracerProvider { - return ( - _global[GLOBAL_TRACE_API_KEY]?.(API_BACKWARDS_COMPATIBILITY_VERSION) ?? - this._proxyTracerProvider - ); + return getGlobal(API_NAME) || this._proxyTracerProvider; } /** @@ -85,7 +71,7 @@ export class TraceAPI { /** Remove the global tracer provider */ public disable() { - delete _global[GLOBAL_TRACE_API_KEY]; + unregisterGlobal(API_NAME); this._proxyTracerProvider = new ProxyTracerProvider(); } diff --git a/src/internal/global-utils.ts b/src/internal/global-utils.ts new file mode 100644 index 00000000..4f811731 --- /dev/null +++ b/src/internal/global-utils.ts @@ -0,0 +1,92 @@ +/* + * Copyright The 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 { diag } from '..'; +import { ContextManager } from '../context/types'; +import { DiagLogger } from '../diag'; +import { _globalThis } from '../platform'; +import { TextMapPropagator } from '../propagation/TextMapPropagator'; +import type { TracerProvider } from '../trace/tracer_provider'; +import { VERSION } from '../version'; +import { isCompatible } from './semver'; + +const GLOBAL_OPENTELEMETRY_API_KEY = Symbol.for('io.opentelemetry.js.api'); + +const _global = _globalThis as OTelGlobal; + +export function registerGlobal( + type: Type, + instance: OTelGlobalAPI[Type], + allowOverride = false +): void { + _global[GLOBAL_OPENTELEMETRY_API_KEY] = _global[ + GLOBAL_OPENTELEMETRY_API_KEY + ] ?? { + version: VERSION, + }; + + const api = _global[GLOBAL_OPENTELEMETRY_API_KEY]!; + if (!allowOverride && api[type]) { + // already registered an API of this type + const err = new Error( + `@opentelemetry/api: Attempted duplicate registration of API: ${type}` + ); + diag.error(err.stack || err.message); + return; + } + + if (api.version != VERSION) { + // All registered APIs must be of the same version exactly + const err = new Error( + '@opentelemetry/api: All API registration versions must match' + ); + diag.error(err.stack || err.message); + return; + } + + api[type] = instance; +} + +export function getGlobal( + type: Type +): OTelGlobalAPI[Type] | undefined { + const version = _global[GLOBAL_OPENTELEMETRY_API_KEY]?.version; + if (!version || !isCompatible(version)) { + return; + } + return _global[GLOBAL_OPENTELEMETRY_API_KEY]?.[type]; +} + +export function unregisterGlobal(type: keyof OTelGlobalAPI) { + const api = _global[GLOBAL_OPENTELEMETRY_API_KEY]; + + if (api) { + delete api[type]; + } +} + +type OTelGlobal = { + [GLOBAL_OPENTELEMETRY_API_KEY]?: OTelGlobalAPI; +}; + +type OTelGlobalAPI = { + version: string; + + diag?: DiagLogger; + trace?: TracerProvider; + context?: ContextManager; + propagation?: TextMapPropagator; +}; diff --git a/src/internal/semver.ts b/src/internal/semver.ts new file mode 100644 index 00000000..fba640d4 --- /dev/null +++ b/src/internal/semver.ts @@ -0,0 +1,113 @@ +/* + * Copyright The 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 { VERSION } from '../version'; + +const re = /^(\d+)\.(\d+)\.(\d+)(?:-(.*))?$/; + +/** + * Create a function to test an API version to see if it is compatible with the provided ownVersion. + * + * The returned function has the following semantics: + * - Exact match is always compatible + * - Major versions must always match + * - The minor version of the API module requesting access to the global API must be greater or equal to the minor version of this API + * - Patch and build tag differences are not considered at this time + * + * @param ownVersion version which should be checked against + */ +export function _makeCompatibilityCheck( + ownVersion: string +): (version: string) => boolean { + const acceptedVersions = new Set([ownVersion]); + const rejectedVersions = new Set(); + + const myVersionMatch = ownVersion.match(re); + if (!myVersionMatch) { + throw new Error('Cannot parse own version'); + } + + const ownVersionParsed = { + major: +myVersionMatch[1], + minor: +myVersionMatch[2], + patch: +myVersionMatch[3], + }; + + return function isCompatible(version: string): boolean { + if (acceptedVersions.has(version)) { + return true; + } + + if (rejectedVersions.has(version)) { + return false; + } + + const m = version.match(re); + if (!m) { + // cannot parse other version + rejectedVersions.add(version); + return false; + } + + const otherVersionParsed = { + major: +m[1], + minor: +m[2], + patch: +m[3], + }; + + // major versions must match + if (ownVersionParsed.major != otherVersionParsed.major) { + rejectedVersions.add(version); + return false; + } + + // if major version is 0, minor is treated like major and patch is treated like minor + if (ownVersionParsed.major === 0) { + if (ownVersionParsed.minor != otherVersionParsed.minor) { + rejectedVersions.add(version); + return false; + } + + if (ownVersionParsed.patch < otherVersionParsed.patch) { + rejectedVersions.add(version); + return false; + } + + acceptedVersions.add(version); + return true; + } + + if (ownVersionParsed.minor < otherVersionParsed.minor) { + rejectedVersions.add(version); + return false; + } + + acceptedVersions.add(version); + return true; + }; +} + +/** + * Test an API version to see if it is compatible with this API. + * + * - Exact match is always compatible + * - Major versions must always match + * - The minor version of the API module requesting access to the global API must be greater or equal to the minor version of this API + * - Patch and build tag differences are not considered at this time + * + * @param version version of the API requesting an instance of the global API + */ +export const isCompatible = _makeCompatibilityCheck(VERSION); diff --git a/test/api/api.test.ts b/test/api/api.test.ts index 5287b38c..4fcad808 100644 --- a/test/api/api.test.ts +++ b/test/api/api.test.ts @@ -34,7 +34,6 @@ import api, { diag, } from '../../src'; import { DiagAPI } from '../../src/api/diag'; -import { _global } from '../../src/api/global-utils'; import { NoopSpan } from '../../src/trace/NoopSpan'; // DiagLogger implementation diff --git a/test/api/global.test.ts b/test/internal/global.test.ts similarity index 58% rename from test/api/global.test.ts rename to test/internal/global.test.ts index 44edfeff..27cf30bb 100644 --- a/test/api/global.test.ts +++ b/test/internal/global.test.ts @@ -15,11 +15,10 @@ */ import * as assert from 'assert'; -import { - GLOBAL_CONTEXT_MANAGER_API_KEY, - _global, -} from '../../src/api/global-utils'; +import { getGlobal } from '../../src/internal/global-utils'; +import { _globalThis } from '../../src/platform'; import { NoopContextManager } from '../../src/context/NoopContextManager'; +import sinon = require('sinon'); const api1 = require('../../src') as typeof import('../../src'); @@ -42,6 +41,9 @@ describe('Global Utils', () => { api1.context.disable(); api1.propagation.disable(); api1.trace.disable(); + api1.diag.disable(); + // @ts-expect-error we are modifying internals for testing purposes here + delete _globalThis[Symbol.for('io.opentelemetry.js.api')]; }); it('should change the global context manager', () => { @@ -72,9 +74,61 @@ describe('Global Utils', () => { it('should return the module NoOp implementation if the version is a mismatch', () => { const original = api1.context['_getContextManager'](); + const newContextManager = new NoopContextManager(); + api1.context.setGlobalContextManager(newContextManager); + + assert.strictEqual(api1.context['_getContextManager'](), newContextManager); + + const globalInstance = getGlobal('context'); + assert.ok(globalInstance); + // @ts-expect-error we are modifying internals for testing purposes here + _globalThis[Symbol.for('io.opentelemetry.js.api')].version = '0.0.1'; + + assert.strictEqual(api1.context['_getContextManager'](), original); + }); + + it('should log an error if there is a duplicate registration', () => { + const error = sinon.stub(); + api1.diag.setLogger({ + verbose: () => {}, + debug: () => {}, + info: () => {}, + warn: () => {}, + error, + }); + + api1.context.setGlobalContextManager(new NoopContextManager()); api1.context.setGlobalContextManager(new NoopContextManager()); - const afterSet = _global[GLOBAL_CONTEXT_MANAGER_API_KEY]!(-1); - assert.strictEqual(original, afterSet); + sinon.assert.calledOnce(error); + assert.strictEqual(error.firstCall.args.length, 1); + assert.ok( + error.firstCall.args[0].startsWith( + 'Error: @opentelemetry/api: Attempted duplicate registration of API: context' + ) + ); + }); + + it('should allow duplicate registration of the diag logger', () => { + const error1 = sinon.stub(); + const error2 = sinon.stub(); + api1.diag.setLogger({ + verbose: () => {}, + debug: () => {}, + info: () => {}, + warn: () => {}, + error: error1, + }); + + api1.diag.setLogger({ + verbose: () => {}, + debug: () => {}, + info: () => {}, + warn: () => {}, + error: error2, + }); + + sinon.assert.notCalled(error1); + sinon.assert.notCalled(error2); }); }); diff --git a/test/internal/semver.test.ts b/test/internal/semver.test.ts new file mode 100644 index 00000000..6020723b --- /dev/null +++ b/test/internal/semver.test.ts @@ -0,0 +1,101 @@ +/* + * Copyright The 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 * as assert from 'assert'; +import { + isCompatible, + _makeCompatibilityCheck, +} from '../../src/internal/semver'; +import { VERSION } from '../../src/version'; + +describe('Version Compatibility', () => { + it('should be compatible if versions are equal', () => { + assert.ok(isCompatible(VERSION)); + }); + + describe('throws if own version cannot be parsed', () => { + assert.throws(() => { + _makeCompatibilityCheck('this is not semver'); + }); + }); + + describe('incompatible if other version cannot be parsed', () => { + const check = _makeCompatibilityCheck('0.1.2'); + assert.ok(!check('this is not semver')); + }); + + describe('>= 1.x', () => { + it('should be compatible if major and minor versions are equal', () => { + const check = _makeCompatibilityCheck('1.2.3'); + assert.ok(check('1.2.2')); + assert.ok(check('1.2.2-alpha')); + assert.ok(check('1.2.4')); + assert.ok(check('1.2.4-alpha')); + }); + + it('should be compatible if major versions are equal and minor version is lesser', () => { + const check = _makeCompatibilityCheck('1.2.3'); + assert.ok(check('1.1.2')); + assert.ok(check('1.1.2-alpha')); + assert.ok(check('1.1.4')); + assert.ok(check('1.1.4-alpha')); + }); + + it('should be incompatible if major versions do not match', () => { + const check = _makeCompatibilityCheck('3.3.3'); + assert.ok(!check('0.3.3')); + assert.ok(!check('0.3.3')); + }); + + it('should be incompatible if major versions match but other minor version is greater than our minor version', () => { + const check = _makeCompatibilityCheck('1.2.3'); + assert.ok(!check('1.3.3-alpha')); + assert.ok(!check('1.3.3')); + }); + }); + + describe('0.x', () => { + it('should be compatible if minor and patch versions are equal', () => { + const check = _makeCompatibilityCheck('0.1.2'); + assert.ok(check('0.1.2')); + assert.ok(check('0.1.2-alpha')); + }); + + it('should be compatible if minor versions are equal and patch version is lesser', () => { + const check = _makeCompatibilityCheck('0.1.2'); + assert.ok(check('0.1.1')); + assert.ok(check('0.1.1-alpha')); + }); + + it('should be incompatible if minor versions do not match', () => { + const check = _makeCompatibilityCheck('0.3.3'); + assert.ok(!check('0.2.3')); + assert.ok(!check('0.4.3')); + }); + + it('should be incompatible if minor versions do not match', () => { + const check = _makeCompatibilityCheck('0.3.3'); + assert.ok(!check('0.2.3')); + assert.ok(!check('0.4.3')); + }); + + it('should be incompatible if minor versions match but other patch version is greater than our patch version', () => { + const check = _makeCompatibilityCheck('0.3.3'); + assert.ok(!check('0.3.4-alpha')); + assert.ok(!check('0.3.4')); + }); + }); +}); From 848c9969984e22c2cc79df1ce9ffc19d734e3905 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 26 Feb 2021 07:49:38 -0500 Subject: [PATCH 2/4] chore: strict equal --- src/internal/global-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/global-utils.ts b/src/internal/global-utils.ts index 4f811731..c6c58aca 100644 --- a/src/internal/global-utils.ts +++ b/src/internal/global-utils.ts @@ -48,7 +48,7 @@ export function registerGlobal( return; } - if (api.version != VERSION) { + if (api.version !== VERSION) { // All registered APIs must be of the same version exactly const err = new Error( '@opentelemetry/api: All API registration versions must match' From 3afeda0a9c39ee9c4eb5f38f76a5eec7744172d1 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 26 Feb 2021 14:10:47 -0500 Subject: [PATCH 3/4] chore: include major version in the API global symbol This will prevent any chance of a version X API interfering with a version Y API if X and Y are different major versions. --- src/internal/global-utils.ts | 5 ++++- test/internal/global.test.ts | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/internal/global-utils.ts b/src/internal/global-utils.ts index c6c58aca..68e7b9ca 100644 --- a/src/internal/global-utils.ts +++ b/src/internal/global-utils.ts @@ -23,7 +23,10 @@ import type { TracerProvider } from '../trace/tracer_provider'; import { VERSION } from '../version'; import { isCompatible } from './semver'; -const GLOBAL_OPENTELEMETRY_API_KEY = Symbol.for('io.opentelemetry.js.api'); +const major = VERSION.split('.')[0]; +const GLOBAL_OPENTELEMETRY_API_KEY = Symbol.for( + `io.opentelemetry.js.api.${major}` +); const _global = _globalThis as OTelGlobal; diff --git a/test/internal/global.test.ts b/test/internal/global.test.ts index 27cf30bb..fc1f0a76 100644 --- a/test/internal/global.test.ts +++ b/test/internal/global.test.ts @@ -43,7 +43,7 @@ describe('Global Utils', () => { api1.trace.disable(); api1.diag.disable(); // @ts-expect-error we are modifying internals for testing purposes here - delete _globalThis[Symbol.for('io.opentelemetry.js.api')]; + delete _globalThis[Symbol.for('io.opentelemetry.js.api.0')]; }); it('should change the global context manager', () => { @@ -82,7 +82,7 @@ describe('Global Utils', () => { const globalInstance = getGlobal('context'); assert.ok(globalInstance); // @ts-expect-error we are modifying internals for testing purposes here - _globalThis[Symbol.for('io.opentelemetry.js.api')].version = '0.0.1'; + _globalThis[Symbol.for('io.opentelemetry.js.api.0')].version = '0.0.1'; assert.strictEqual(api1.context['_getContextManager'](), original); }); From 9d4eb290a0ff00b6ff6091877af025f02f77d6eb Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 26 Feb 2021 14:23:21 -0500 Subject: [PATCH 4/4] chore: lint --- src/internal/semver.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal/semver.ts b/src/internal/semver.ts index fba640d4..e808df7b 100644 --- a/src/internal/semver.ts +++ b/src/internal/semver.ts @@ -69,14 +69,14 @@ export function _makeCompatibilityCheck( }; // major versions must match - if (ownVersionParsed.major != otherVersionParsed.major) { + if (ownVersionParsed.major !== otherVersionParsed.major) { rejectedVersions.add(version); return false; } // if major version is 0, minor is treated like major and patch is treated like minor if (ownVersionParsed.major === 0) { - if (ownVersionParsed.minor != otherVersionParsed.minor) { + if (ownVersionParsed.minor !== otherVersionParsed.minor) { rejectedVersions.add(version); return false; }