open-telemetry
/
opentelemetry-js
mirror of https://github.com/open-telemetry/opentelemetry-js.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

feat(sdk-trace)!: remove ability to have BasicTracerProvider instantiate exporters (#5239)

This commit is contained in:
Marc Pichler 2025-01-21 13:41:10 +01:00 committed by GitHub
parent 6c04a41443
commit c00f36ee43
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 24 additions and 254 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
CHANGELOG.md
View File

@ -26,6 +26,12 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
* refactor(sdk-trace-base)!: remove `new Span` constructor in favor of `Tracer.startSpan` API [#5048](https://github.com/open-telemetry/opentelemetry-js/pull/5048) @david-luna
* refactor(sdk-trace-base)!: remove `BasicTracerProvider.addSpanProcessor` API in favor of constructor options. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna
* refactor(sdk-trace-base)!: make `resource` property private in `BasicTracerProvider` and remove `getActiveSpanProcessor` API. [#5192](https://github.com/open-telemetry/opentelemetry-js/pull/5192) @david-luna
* feat(sdk-trace)!: remove ability to have BasicTracerProvider instantiate exporters [#5239](https://github.com/open-telemetry/opentelemetry-js/pull/5239) @pichlermarc
* When extending `BasicTracerProvider`, the class offered multiple methods to facilitate the creation of exporters and auto-pairing with `SpanProcessor`s.
* This functionality has been removed - users may now pass `SpanProcessor`s to the base class constructor when extending
* (user-facing): `_registeredExporters` has been removed
* (user-facing): `_getSpanExporter` has been removed
* (user-facing): `_buildExporterFromEnv` has been removed
* feat(core)!: remove deprecated `IdGenerator` and `RandomIdGenerator` [#5309](https://github.com/open-telemetry/opentelemetry-js/pull/5309) @pichlermarc
* feat(core)!: remove deprecated type `InstrumentationLibrary` [#5308](https://github.com/open-telemetry/opentelemetry-js/pull/5308) @pichlermarc
* (user-facing): please use equivalent type `InstrumentationScope` instead

1
experimental/packages/opentelemetry-sdk-node/src/sdk.ts
View File

@ -363,7 +363,6 @@ export class NodeSDK {
? this._tracerProviderConfig.spanProcessors
: getSpanProcessorsFromEnv();
// If the Provider is configured with Env Exporters, we need to check if the SDK had any manual configurations and set them here
this._tracerProvider = new NodeTracerProvider({
...this._configuration,
resource: this._resource,

5
experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
View File

@ -1465,7 +1465,7 @@ describe('setup exporter from env', () => {
await sdk.shutdown();
});
it('should use noop span processor when user sets env exporter to none', async () => {
it('should use empty span processor when user sets env exporter to none', async () => {
env.OTEL_TRACES_EXPORTER = 'none';
const sdk = new NodeSDK();
sdk.start();
@ -1478,8 +1478,7 @@ describe('setup exporter from env', () => {
const listOfProcessors = getSdkSpanProcessors(sdk);
assert(listOfProcessors.length === 1);
assert(listOfProcessors[0] instanceof NoopSpanProcessor);
assert.strictEqual(listOfProcessors.length, 0);
delete env.OTEL_TRACES_EXPORTER;
await sdk.shutdown();
});

32
packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts
View File

@ -34,10 +34,8 @@ import { SpanProcessor } from './SpanProcessor';
import { Tracer } from './Tracer';
import { loadDefaultConfig } from './config';
import { MultiSpanProcessor } from './MultiSpanProcessor';
import { NoopSpanProcessor } from './export/NoopSpanProcessor';
import { SDKRegistrationConfig, TracerConfig } from './types';
import { SpanExporter } from './export/SpanExporter';
import { BatchSpanProcessor } from './platform';
import { reconfigureLimits } from './utility';
export type PROPAGATOR_FACTORY = () => TextMapPropagator;
@ -62,11 +60,6 @@ export class BasicTracerProvider implements TracerProvider {
['baggage', () => new W3CBaggagePropagator()],
]);
protected static readonly _registeredExporters = new Map<
string,
EXPORTER_FACTORY
>();
private readonly _config: TracerConfig;
private readonly _tracers: Map<string, Tracer> = new Map();
private readonly _resource: IResource;
@ -88,13 +81,6 @@ export class BasicTracerProvider implements TracerProvider {
if (config.spanProcessors?.length) {
spanProcessors.push(...config.spanProcessors);
} else {
const defaultExporter = this._buildExporterFromEnv();
spanProcessors.push(
defaultExporter
? new BatchSpanProcessor(defaultExporter)
: new NoopSpanProcessor()
);
}
this._activeSpanProcessor = new MultiSpanProcessor(spanProcessors);
@ -210,12 +196,6 @@ export class BasicTracerProvider implements TracerProvider {
)._registeredPropagators.get(name)?.();
}
protected _getSpanExporter(name: string): SpanExporter | undefined {
return (
this.constructor as typeof BasicTracerProvider
)._registeredExporters.get(name)?.();
}
protected _buildPropagatorFromEnv(): TextMapPropagator | undefined {
// per spec, propagators from env must be deduplicated
const uniquePropagatorNames = Array.from(
@ -252,16 +232,4 @@ export class BasicTracerProvider implements TracerProvider {
});
}
}
protected _buildExporterFromEnv(): SpanExporter | undefined {
const exporterName = getEnv().OTEL_TRACES_EXPORTER;
if (exporterName === 'none' || exporterName === '') return;
const exporter = this._getSpanExporter(exporterName);
if (!exporter) {
diag.error(
`Exporter "${exporterName}" requested through environment variable is unavailable.`
);
}
return exporter;
}
}

177
packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts
View File

@ -36,9 +36,6 @@ import {
BasicTracerProvider,
NoopSpanProcessor,
Span,
InMemorySpanExporter,
SpanExporter,
BatchSpanProcessor,
AlwaysOnSampler,
AlwaysOffSampler,
ConsoleSpanExporter,
@ -59,8 +56,6 @@ class DummyPropagator implements TextMapPropagator {
}
}
class DummyExporter extends InMemorySpanExporter {}
describe('BasicTracerProvider', () => {
let envSource: Record<string, any>;
let setGlobalPropagatorStub: sinon.SinonSpy<[TextMapPropagator], boolean>;
@ -90,55 +85,19 @@ describe('BasicTracerProvider', () => {
assert.ok(tracer instanceof BasicTracerProvider);
});
it('should use noop span processor by default', () => {
const tracer = new BasicTracerProvider();
assert.ok(tracer['_activeSpanProcessor'] instanceof MultiSpanProcessor);
assert.ok(
tracer['_activeSpanProcessor']['_spanProcessors'].length === 1
);
assert.ok(
tracer['_activeSpanProcessor']['_spanProcessors'][0] instanceof
NoopSpanProcessor
);
});
it('should use noop span processor by default and no diag error', () => {
it('should use empty span processor by default', () => {
const errorStub = sinon.spy(diag, 'error');
const tracer = new BasicTracerProvider();
assert.ok(tracer['_activeSpanProcessor'] instanceof MultiSpanProcessor);
assert.ok(
tracer['_activeSpanProcessor']['_spanProcessors'].length === 1
);
assert.ok(
tracer['_activeSpanProcessor']['_spanProcessors'][0] instanceof
NoopSpanProcessor
assert.strictEqual(
tracer['_activeSpanProcessor']['_spanProcessors'].length,
0
);
sinon.assert.notCalled(errorStub);
});
});
describe('when user sets unavailable exporter', () => {
it('should use noop span processor by default and show diag error', () => {
envSource.OTEL_TRACES_EXPORTER = 'someExporter';
const errorStub = sinon.spy(diag, 'error');
const tracer = new BasicTracerProvider();
assert.ok(tracer['_activeSpanProcessor'] instanceof MultiSpanProcessor);
assert.ok(
tracer['_activeSpanProcessor']['_spanProcessors'].length === 1
);
assert.ok(
tracer['_activeSpanProcessor']['_spanProcessors'][0] instanceof
NoopSpanProcessor
);
sinon.assert.calledWith(
errorStub,
'Exporter "someExporter" requested through environment variable is unavailable.'
);
delete envSource.OTEL_TRACES_EXPORTER;
});
});
describe('when user sets span processors', () => {
it('should use the span processors defined in the config', () => {
const traceExporter = new ConsoleSpanExporter();
@ -448,14 +407,6 @@ describe('BasicTracerProvider', () => {
...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({});
@ -465,19 +416,9 @@ describe('BasicTracerProvider', () => {
);
/* BasicTracerProvider has no exporters by default, so skipping testing the exporter getter */
provider.register();
assert.ok(provider['_activeSpanProcessor'] instanceof MultiSpanProcessor);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'].length === 1
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof
BatchSpanProcessor
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0][
'_exporter'
] instanceof DummyExporter
assert.strictEqual(
provider['_activeSpanProcessor']['_spanProcessors'].length,
0
);
sinon.assert.calledOnceWithExactly(
@ -494,11 +435,6 @@ describe('BasicTracerProvider', () => {
() => 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 {
@ -507,32 +443,13 @@ describe('BasicTracerProvider', () => {
CustomTracerProvider._registeredPropagators.get(name)?.()
);
}
protected override _getSpanExporter(
name: string
): SpanExporter | undefined {
return (
super._getSpanExporter(name) ||
CustomTracerProvider._registeredExporters.get(name)?.()
);
}
}
const provider = new CustomTracerProvider({});
provider.register();
assert.ok(provider['_activeSpanProcessor'] instanceof MultiSpanProcessor);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'].length === 1
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof
BatchSpanProcessor
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0][
'_exporter'
] instanceof DummyExporter
assert.strictEqual(
provider['_activeSpanProcessor']['_spanProcessors'].length,
0
);
sinon.assert.calledOnceWithExactly(
@ -600,57 +517,6 @@ describe('BasicTracerProvider', () => {
);
});
});
describe('exporter', () => {
class CustomTracerProvider extends BasicTracerProvider {
protected override _getSpanExporter(
name: string
): SpanExporter | undefined {
return name === 'memory'
? new InMemorySpanExporter()
: BasicTracerProvider._registeredExporters.get(name)?.();
}
}
afterEach(() => {
delete envSource.OTEL_TRACES_EXPORTER;
});
it('logs error if there is no exporter registered with a given name', () => {
const errorStub = sinon.spy(diag, 'error');
envSource.OTEL_TRACES_EXPORTER = 'missing-exporter';
const provider = new BasicTracerProvider({});
provider.register();
assert.ok(
errorStub.getCall(0).args[0] ===
'Exporter "missing-exporter" requested through environment variable is unavailable.'
);
errorStub.restore();
});
it('registers trace exporter from environment variable', () => {
envSource.OTEL_TRACES_EXPORTER = 'memory';
const provider = new CustomTracerProvider({});
provider.register();
assert.ok(
provider['_activeSpanProcessor'] instanceof MultiSpanProcessor
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'].length === 1
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof
BatchSpanProcessor
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0][
'_exporter'
] instanceof InMemorySpanExporter
);
});
});
});
describe('.startSpan()', () => {
@ -834,29 +700,6 @@ describe('BasicTracerProvider', () => {
});
describe('.forceFlush()', () => {
it('should call forceFlush with the default processor', done => {
sinon.restore();
const forceFlushStub = sinon.stub(
NoopSpanProcessor.prototype,
'forceFlush'
);
forceFlushStub.resolves();
const tracerProvider = new BasicTracerProvider();
tracerProvider
.forceFlush()
.then(() => {
sinon.restore();
assert(forceFlushStub.calledOnce);
done();
})
.catch(error => {
sinon.restore();
done(error);
});
});
it('should call forceFlush on all registered span processors', done => {
sinon.restore();
const forceFlushStub = sinon.stub(

57
packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts
View File

@ -33,10 +33,7 @@ import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import {
AlwaysOffSampler,
AlwaysOnSampler,
BatchSpanProcessor,
InMemorySpanExporter,
Span,
SpanExporter,
} from '@opentelemetry/sdk-trace-base';
import { Resource } from '@opentelemetry/resources';
import { SEMRESATTRS_TELEMETRY_SDK_LANGUAGE } from '@opentelemetry/semantic-conventions';
@ -265,10 +262,7 @@ describe('NodeTracerProvider', () => {
}
}
class DummyExporter extends InMemorySpanExporter {}
beforeEach(() => {
process.env.OTEL_TRACES_EXPORTER = 'custom-exporter';
process.env.OTEL_PROPAGATORS = 'custom-propagator';
propagation.disable();
@ -276,7 +270,6 @@ describe('NodeTracerProvider', () => {
});
afterEach(() => {
delete process.env.OTEL_TRACES_EXPORTER;
delete process.env.OTEL_PROPAGATORS;
propagation.disable();
@ -293,33 +286,18 @@ describe('NodeTracerProvider', () => {
string,
() => TextMapPropagator
>([['custom-propagator', () => propagator]]);
protected static override readonly _registeredExporters = new Map<
string,
() => SpanExporter
>([['custom-exporter', () => new DummyExporter()]]);
}
const provider = new CustomTracerProvider({});
provider.register();
assert.ok(
provider['_activeSpanProcessor'].constructor.name ===
'MultiSpanProcessor'
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'].length === 1
assert.strictEqual(
provider['_activeSpanProcessor']['_spanProcessors'].length,
0
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof
BatchSpanProcessor
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0][
'_exporter'
] instanceof DummyExporter
);
assert.strictEqual(propagation['_getGlobalPropagator'](), propagator);
});
@ -333,11 +311,6 @@ describe('NodeTracerProvider', () => {
() => TextMapPropagator
>([['custom-propagator', () => propagator]]);
protected static override readonly _registeredExporters = new Map<
string,
() => SpanExporter
>([['custom-exporter', () => new DummyExporter()]]);
protected override _getPropagator(
name: string
): TextMapPropagator | undefined {
@ -346,35 +319,17 @@ describe('NodeTracerProvider', () => {
CustomTracerProvider._registeredPropagators.get(name)?.()
);
}
protected override _getSpanExporter(
name: string
): SpanExporter | undefined {
return (
super._getSpanExporter(name) ||
CustomTracerProvider._registeredExporters.get(name)?.()
);
}
}
const provider = new CustomTracerProvider({});
provider.register();
assert.ok(
provider['_activeSpanProcessor'].constructor.name ===
'MultiSpanProcessor'
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'].length === 1
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof
BatchSpanProcessor
);
assert.ok(
provider['_activeSpanProcessor']['_spanProcessors'][0][
'_exporter'
] instanceof DummyExporter
assert.strictEqual(
provider['_activeSpanProcessor']['_spanProcessors'].length,
0
);
assert.strictEqual(propagation['_getGlobalPropagator'](), propagator);