From 950433f040c551e3639d8a1dc7e1c58ba49eab29 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 27 May 2021 13:51:03 -0400 Subject: [PATCH] chore: do not export singletons (#46) --- src/api/propagation.ts | 13 +++++----- src/index.ts | 4 ---- src/propagation/NoopTextMapPropagator.ts | 2 -- src/trace/NoopTracer.ts | 2 -- src/trace/NoopTracerProvider.ts | 6 ++--- src/trace/ProxyTracer.ts | 4 ++-- src/trace/ProxyTracerProvider.ts | 4 +++- test/api/api.test.ts | 24 +++++++++---------- test/internal/global.test.ts | 17 +++++++------ test/noop-implementations/noop-tracer.test.ts | 2 +- .../proxy-tracer.test.ts | 3 ++- 11 files changed, 39 insertions(+), 42 deletions(-) diff --git a/src/api/propagation.ts b/src/api/propagation.ts index 6ef7e8a9..bd1975a4 100644 --- a/src/api/propagation.ts +++ b/src/api/propagation.ts @@ -15,7 +15,12 @@ */ import { Context } from '../context/types'; -import { NOOP_TEXT_MAP_PROPAGATOR } from '../propagation/NoopTextMapPropagator'; +import { + getGlobal, + registerGlobal, + unregisterGlobal, +} from '../internal/global-utils'; +import { NoopTextMapPropagator } from '../propagation/NoopTextMapPropagator'; import { defaultTextMapGetter, defaultTextMapSetter, @@ -23,11 +28,6 @@ import { TextMapPropagator, TextMapSetter, } from '../propagation/TextMapPropagator'; -import { - getGlobal, - registerGlobal, - unregisterGlobal, -} from '../internal/global-utils'; import { getBaggage, setBaggage, @@ -36,6 +36,7 @@ import { import { createBaggage } from '../baggage/utils'; const API_NAME = 'propagation'; +const NOOP_TEXT_MAP_PROPAGATOR = new NoopTextMapPropagator(); /** * Singleton object which represents the entry point to the OpenTelemetry Propagation API diff --git a/src/index.ts b/src/index.ts index 79f1270d..78c26a80 100644 --- a/src/index.ts +++ b/src/index.ts @@ -19,12 +19,9 @@ export { baggageEntryMetadataFromString } from './baggage/utils'; export * from './common/Exception'; export * from './common/Time'; export * from './diag'; -export * from './propagation/NoopTextMapPropagator'; export * from './propagation/TextMapPropagator'; export * from './trace/attributes'; export * from './trace/link'; -export * from './trace/NoopTracer'; -export * from './trace/NoopTracerProvider'; export * from './trace/ProxyTracer'; export * from './trace/ProxyTracerProvider'; export * from './trace/Sampler'; @@ -49,7 +46,6 @@ export { } from './trace/spancontext-utils'; export * from './context/context'; -export * from './context/NoopContextManager'; export * from './context/types'; import { ContextAPI } from './api/context'; diff --git a/src/propagation/NoopTextMapPropagator.ts b/src/propagation/NoopTextMapPropagator.ts index 4d15ac8d..16d90cf9 100644 --- a/src/propagation/NoopTextMapPropagator.ts +++ b/src/propagation/NoopTextMapPropagator.ts @@ -31,5 +31,3 @@ export class NoopTextMapPropagator implements TextMapPropagator { return []; } } - -export const NOOP_TEXT_MAP_PROPAGATOR = new NoopTextMapPropagator(); diff --git a/src/trace/NoopTracer.ts b/src/trace/NoopTracer.ts index 7f810522..b44dde20 100644 --- a/src/trace/NoopTracer.ts +++ b/src/trace/NoopTracer.ts @@ -95,5 +95,3 @@ function isSpanContext(spanContext: any): spanContext is SpanContext { typeof spanContext['traceFlags'] === 'number' ); } - -export const NOOP_TRACER = new NoopTracer(); diff --git a/src/trace/NoopTracerProvider.ts b/src/trace/NoopTracerProvider.ts index 09c86bd7..0d23d18d 100644 --- a/src/trace/NoopTracerProvider.ts +++ b/src/trace/NoopTracerProvider.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { NOOP_TRACER } from './NoopTracer'; +import { NoopTracer } from './NoopTracer'; import { Tracer } from './tracer'; import { TracerProvider } from './tracer_provider'; @@ -26,8 +26,6 @@ import { TracerProvider } from './tracer_provider'; */ export class NoopTracerProvider implements TracerProvider { getTracer(_name?: string, _version?: string): Tracer { - return NOOP_TRACER; + return new NoopTracer(); } } - -export const NOOP_TRACER_PROVIDER = new NoopTracerProvider(); diff --git a/src/trace/ProxyTracer.ts b/src/trace/ProxyTracer.ts index 9cd399f9..987cc34b 100644 --- a/src/trace/ProxyTracer.ts +++ b/src/trace/ProxyTracer.ts @@ -15,7 +15,7 @@ */ import { Context } from '../context/types'; -import { NOOP_TRACER } from './NoopTracer'; +import { NoopTracer } from './NoopTracer'; import { ProxyTracerProvider } from './ProxyTracerProvider'; import { Span } from './span'; import { SpanOptions } from './SpanOptions'; @@ -60,7 +60,7 @@ export class ProxyTracer implements Tracer { const tracer = this._provider.getDelegateTracer(this.name, this.version); if (!tracer) { - return NOOP_TRACER; + return new NoopTracer(); } this._delegate = tracer; diff --git a/src/trace/ProxyTracerProvider.ts b/src/trace/ProxyTracerProvider.ts index e124dc95..0d2d0b0e 100644 --- a/src/trace/ProxyTracerProvider.ts +++ b/src/trace/ProxyTracerProvider.ts @@ -17,7 +17,9 @@ import { Tracer } from './tracer'; import { TracerProvider } from './tracer_provider'; import { ProxyTracer } from './ProxyTracer'; -import { NOOP_TRACER_PROVIDER } from './NoopTracerProvider'; +import { NoopTracerProvider } from './NoopTracerProvider'; + +const NOOP_TRACER_PROVIDER = new NoopTracerProvider(); /** * Tracer provider which provides {@link ProxyTracer}s. diff --git a/test/api/api.test.ts b/test/api/api.test.ts index 8049f25f..7f6bce84 100644 --- a/test/api/api.test.ts +++ b/test/api/api.test.ts @@ -16,25 +16,25 @@ import * as assert from 'assert'; import api, { - TraceFlags, - NoopTracerProvider, - NoopTracer, - SpanOptions, - Span, context, - trace, - propagation, - TextMapPropagator, Context, - TextMapSetter, - TextMapGetter, - ROOT_CONTEXT, - defaultTextMapSetter, defaultTextMapGetter, + defaultTextMapSetter, diag, + propagation, + ROOT_CONTEXT, + Span, + SpanOptions, + TextMapGetter, + TextMapPropagator, + TextMapSetter, + trace, + TraceFlags, } from '../../src'; import { DiagAPI } from '../../src/api/diag'; import { NonRecordingSpan } from '../../src/trace/NonRecordingSpan'; +import { NoopTracer } from '../../src/trace/NoopTracer'; +import { NoopTracerProvider } from '../../src/trace/NoopTracerProvider'; // DiagLogger implementation const diagLoggerFunctions = [ diff --git a/test/internal/global.test.ts b/test/internal/global.test.ts index 9d8f78f3..99fadb95 100644 --- a/test/internal/global.test.ts +++ b/test/internal/global.test.ts @@ -67,20 +67,19 @@ describe('Global Utils', () => { }); it('should disable both if one is disabled', () => { - const original = api1.context['_getContextManager'](); - - api1.context.setGlobalContextManager(new NoopContextManager()); + const manager = new NoopContextManager(); + api1.context.setGlobalContextManager(manager); - assert.notStrictEqual(original, api1.context['_getContextManager']()); + assert.strictEqual(manager, api1.context['_getContextManager']()); api2.context.disable(); - assert.strictEqual(original, api1.context['_getContextManager']()); + assert.notStrictEqual(manager, api1.context['_getContextManager']()); }); 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); + // ensure new context manager is returned assert.strictEqual(api1.context['_getContextManager'](), newContextManager); const globalInstance = getGlobal('context'); @@ -88,7 +87,11 @@ describe('Global Utils', () => { // @ts-expect-error we are modifying internals for testing purposes here _globalThis[Symbol.for(GLOBAL_API_SYMBOL_KEY)].version = '0.0.1'; - assert.strictEqual(api1.context['_getContextManager'](), original); + // ensure new context manager is not returned because version above is incompatible + assert.notStrictEqual( + api1.context['_getContextManager'](), + newContextManager + ); }); it('should log an error if there is a duplicate registration', () => { diff --git a/test/noop-implementations/noop-tracer.test.ts b/test/noop-implementations/noop-tracer.test.ts index 0d4ca1a3..7f23ff22 100644 --- a/test/noop-implementations/noop-tracer.test.ts +++ b/test/noop-implementations/noop-tracer.test.ts @@ -17,7 +17,6 @@ import * as assert from 'assert'; import { context, - NoopTracer, Span, SpanContext, SpanKind, @@ -25,6 +24,7 @@ import { TraceFlags, } from '../../src'; import { NonRecordingSpan } from '../../src/trace/NonRecordingSpan'; +import { NoopTracer } from '../../src/trace/NoopTracer'; describe('NoopTracer', () => { it('should not crash', () => { diff --git a/test/proxy-implementations/proxy-tracer.test.ts b/test/proxy-implementations/proxy-tracer.test.ts index 57e3e2dc..6b84b9df 100644 --- a/test/proxy-implementations/proxy-tracer.test.ts +++ b/test/proxy-implementations/proxy-tracer.test.ts @@ -18,7 +18,6 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { context, - NoopTracer, ProxyTracer, ProxyTracerProvider, ROOT_CONTEXT, @@ -29,6 +28,8 @@ import { TracerProvider, } from '../../src'; import { NonRecordingSpan } from '../../src/trace/NonRecordingSpan'; +import { NoopTracer } from '../../src/trace/NoopTracer'; + describe('ProxyTracer', () => { let provider: ProxyTracerProvider; const sandbox = sinon.createSandbox();