Skip to content

Commit

Permalink
feat: allow extending BasicTracerProvider with custom registered co…
Browse files Browse the repository at this point in the history 
…mponents (open-telemetry#3023)

Co-authored-by: Daniel Dyla <[email protected]>
  • Loading branch information
@rauno56 @dyladan
rauno56 and dyladan authored Jul 20, 2022
1 parent 1e8b896 commit 69cf899
Show file tree
Hide file tree
Showing 5 changed files with 297 additions and 78 deletions.
15 changes: 13 additions & 2 deletions packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,23 @@ export class BasicTracerProvider implements TracerProvider {
return this.activeSpanProcessor.shutdown();
}

/**
* TS cannot yet infer the type of this.constructor:
* https://github.com/Microsoft/TypeScript/issues/3841#issuecomment-337560146
* There is no need to override either of the getters in your child class.
* The type of the registered component maps should be the same across all
* classes in the inheritance tree.
*/
protected _getPropagator(name: string): TextMapPropagator | undefined {
return BasicTracerProvider._registeredPropagators.get(name)?.();
return (
(this.constructor as typeof BasicTracerProvider)._registeredPropagators
).get(name)?.();
}

protected _getSpanExporter(name: string): SpanExporter | undefined {
return BasicTracerProvider._registeredExporters.get(name)?.();
return (
(this.constructor as typeof BasicTracerProvider)._registeredExporters
).get(name)?.();
}

protected _buildPropagatorFromEnv(): TextMapPropagator | undefined {
Expand Down
176 changes: 131 additions & 45 deletions packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ import {
} from '@opentelemetry/api';
import { CompositePropagator } from '@opentelemetry/core';
import {
AlwaysOnSampler,
AlwaysOffSampler,
AlwaysOnSampler,
TraceState,
W3CTraceContextPropagator,
} from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import * as assert from 'assert';
Expand All @@ -45,26 +46,51 @@ import {
BatchSpanProcessor,
} from '../../src';

describe('BasicTracerProvider', () => {
let removeEvent: (() => void) | undefined;
class DummyPropagator implements TextMapPropagator {
inject(
context: Context,
carrier: any,
setter: TextMapSetter<any>
): void {
throw new Error('Method not implemented.');
}
extract(
context: Context,
carrier: any,
getter: TextMapGetter<any>
): Context {
throw new Error('Method not implemented.');
}
fields(): string[] {
throw new Error('Method not implemented.');
}
}

class DummyExporter extends InMemorySpanExporter {}

describe('BasicTracerProvider', () => {
let envSource: Record<string, any>;
let setGlobalPropagatorStub: sinon.SinonSpy<
[TextMapPropagator],
boolean
>;

if (typeof process === 'undefined') {
envSource = (globalThis as unknown) as Record<string, any>;
} else {
envSource = process.env as Record<string, any>;
}

beforeEach(() => {
// to avoid actually registering the TraceProvider and leaking env to other tests
sinon.stub(trace, 'setGlobalTracerProvider');
setGlobalPropagatorStub = sinon.spy(propagation, 'setGlobalPropagator');

context.disable();
});

afterEach(() => {
sinon.restore();
if (removeEvent) {
removeEvent();
removeEvent = undefined;
}
});

describe('constructor', () => {
Expand Down Expand Up @@ -240,40 +266,104 @@ describe('BasicTracerProvider', () => {
});
});

describe('.register()', () => {
describe('propagator', () => {
class DummyPropagator implements TextMapPropagator {
inject(
context: Context,
carrier: any,
setter: TextMapSetter<any>
): void {
throw new Error('Method not implemented.');
}
extract(
context: Context,
carrier: any,
getter: TextMapGetter<any>
): Context {
throw new Error('Method not implemented.');
describe('Custom TracerProvider through inheritance', () => {
beforeEach(() => {
envSource.OTEL_TRACES_EXPORTER = 'custom-exporter';
envSource.OTEL_PROPAGATORS = 'custom-propagator';
});

afterEach(() => {
delete envSource.OTEL_TRACES_EXPORTER;
delete envSource.OTEL_PROPAGATORS;
sinon.restore();
});

it('can be extended by overriding registered components', () => {
class CustomTracerProvider extends BasicTracerProvider {
protected static override readonly _registeredPropagators = new Map<
string,
() => TextMapPropagator
>([
...BasicTracerProvider._registeredPropagators,
['custom-propagator', () => new DummyPropagator()],
]);

protected static override readonly _registeredExporters = new Map<
string,
() => SpanExporter
>([
...BasicTracerProvider._registeredExporters,
['custom-exporter', () => new DummyExporter()],
]);
}

const provider = new CustomTracerProvider({});
assert(provider['_getPropagator']('tracecontext') instanceof W3CTraceContextPropagator);
/* BasicTracerProvider has no exporters by default, so skipping testing the exporter getter */

provider.register();
const processor = provider.getActiveSpanProcessor();
assert(processor instanceof BatchSpanProcessor);
// @ts-expect-error access configured to verify its the correct one
const exporter = processor._exporter;
assert(exporter instanceof DummyExporter);

sinon.assert.calledOnceWithExactly(setGlobalPropagatorStub, sinon.match.instanceOf(DummyPropagator));
});

it('the old way of extending still works', () => {
// this is an anti-pattern, but we test that for backwards compatibility
class CustomTracerProvider extends BasicTracerProvider {
protected static override readonly _registeredPropagators = new Map<
string,
() => TextMapPropagator
>([
['custom-propagator', () => new DummyPropagator()],
]);

protected static override readonly _registeredExporters = new Map<
string,
() => SpanExporter
>([
['custom-exporter', () => new DummyExporter()],
]);

protected override _getPropagator(name: string): TextMapPropagator | undefined {
return (
super._getPropagator(name) ||
CustomTracerProvider._registeredPropagators.get(name)?.()
);
}
fields(): string[] {
throw new Error('Method not implemented.');

protected override _getSpanExporter(name: string): SpanExporter | undefined {
return (
super._getSpanExporter(name) ||
CustomTracerProvider._registeredExporters.get(name)?.()
);
}
}

let setGlobalPropagatorStub: sinon.SinonSpy<
[TextMapPropagator],
boolean
>;
const provider = new CustomTracerProvider({});
provider.register();
const processor = provider.getActiveSpanProcessor();
assert(processor instanceof BatchSpanProcessor);
// @ts-expect-error access configured to verify its the correct one
const exporter = processor._exporter;
assert(exporter instanceof DummyExporter);

sinon.assert.calledOnceWithExactly(setGlobalPropagatorStub, sinon.match.instanceOf(DummyPropagator));
});
});

describe('.register()', () => {
describe('propagator', () => {
let originalPropagators: string | number | undefined | string[];
beforeEach(() => {
setGlobalPropagatorStub = sinon.spy(propagation, 'setGlobalPropagator');
originalPropagators = envSource.OTEL_PROPAGATORS;
});

afterEach(() => {
setGlobalPropagatorStub.restore();
sinon.restore();

// otherwise we may assign 'undefined' (a string)
if (originalPropagators !== undefined) {
Expand All @@ -288,21 +378,20 @@ describe('BasicTracerProvider', () => {
provider.register({
propagator: new DummyPropagator(),
});
assert.ok(
setGlobalPropagatorStub.calledOnceWithExactly(
sinon.match.instanceOf(DummyPropagator)
)

sinon.assert.calledOnceWithExactly(
setGlobalPropagatorStub,
sinon.match.instanceOf(DummyPropagator)
);
});

it('should be composite if 2 or more propagators provided in an environment variable', () => {
const provider = new BasicTracerProvider();
provider.register();

assert.ok(
setGlobalPropagatorStub.calledOnceWithExactly(
sinon.match.instanceOf(CompositePropagator)
)
sinon.assert.calledOnceWithExactly(
setGlobalPropagatorStub,
sinon.match.instanceOf(CompositePropagator)
);
assert.deepStrictEqual(setGlobalPropagatorStub.args[0][0].fields(), [
'traceparent',
Expand All @@ -318,13 +407,10 @@ describe('BasicTracerProvider', () => {
const provider = new BasicTracerProvider({});
provider.register();

assert.ok(
warnStub.calledOnceWithExactly(
'Propagator "missing-propagator" requested through environment variable is unavailable.'
)
sinon.assert.calledOnceWithExactly(
warnStub,
'Propagator "missing-propagator" requested through environment variable is unavailable.'
);

warnStub.restore();
});
});

Expand Down
9 changes: 1 addition & 8 deletions packages/opentelemetry-sdk-trace-node/src/NodeTracerProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { TextMapPropagator } from '@opentelemetry/api';
import {
AsyncHooksContextManager,
AsyncLocalStorageContextManager,
Expand All @@ -40,6 +39,7 @@ export class NodeTracerProvider extends BasicTracerProvider {
string,
PROPAGATOR_FACTORY
>([
...BasicTracerProvider._registeredPropagators,
[
'b3',
() =>
Expand Down Expand Up @@ -67,11 +67,4 @@ export class NodeTracerProvider extends BasicTracerProvider {

super.register(config);
}

protected override _getPropagator(name: string): TextMapPropagator | undefined {
return (
super._getPropagator(name) ||
NodeTracerProvider._registeredPropagators.get(name)?.()
);
}
}
Loading

0 comments on commit 69cf899

Please sign in to comment.