From 8d5f4652e68ea24a690c08128df23fb8cf932ea3 Mon Sep 17 00:00:00 2001 From: David Luna Date: Wed, 27 Nov 2024 16:51:44 +0100 Subject: [PATCH 1/3] chore(sdk-trace-base): rename activeSpanProcessor --- .../opentelemetry-sdk-node/test/sdk.test.ts | 4 +- .../src/BasicTracerProvider.ts | 11 ++-- .../test/common/BasicTracerProvider.test.ts | 52 +++++++++---------- .../test/common/MultiSpanProcessor.test.ts | 2 +- .../test/common/Tracer.test.ts | 38 +++++++------- .../common/export/TestTracingSpanExporter.ts | 2 +- .../test/NodeTracerProvider.test.ts | 16 +++--- 7 files changed, 62 insertions(+), 63 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index 064584a1f4d..c5cf4472317 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -262,7 +262,7 @@ describe('Node SDK', () => { assert.ok(nodeTracerProvider instanceof NodeTracerProvider); - const spanProcessor = nodeTracerProvider['activeSpanProcessor'] as any; + const spanProcessor = nodeTracerProvider['_activeSpanProcessor'] as any; assert( spanProcessor.constructor.name === 'MultiSpanProcessor', @@ -1117,7 +1117,7 @@ describe('setup exporter from env', () => { assert(tracerProvider instanceof NodeTracerProvider); - const activeSpanProcessor = tracerProvider['activeSpanProcessor']; + const activeSpanProcessor = tracerProvider['_activeSpanProcessor']; assert(activeSpanProcessor.constructor.name === 'MultiSpanProcessor'); diff --git a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts index 453d96186d0..b99d6a664f1 100644 --- a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts +++ b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts @@ -69,8 +69,7 @@ export class BasicTracerProvider implements TracerProvider { private readonly _config: TracerConfig; private readonly _tracers: Map = new Map(); private readonly _resource: IResource; - - private activeSpanProcessor: MultiSpanProcessor; + private readonly _activeSpanProcessor: MultiSpanProcessor; constructor(config: TracerConfig = {}) { const mergedConfig = merge( @@ -101,7 +100,7 @@ export class BasicTracerProvider implements TracerProvider { ); } - this.activeSpanProcessor = new MultiSpanProcessor(spanProcessors); + this._activeSpanProcessor = new MultiSpanProcessor(spanProcessors); } getTracer( @@ -117,7 +116,7 @@ export class BasicTracerProvider implements TracerProvider { { name, version, schemaUrl: options?.schemaUrl }, this._config, this._resource, - this.activeSpanProcessor + this._activeSpanProcessor ) ); } @@ -150,7 +149,7 @@ export class BasicTracerProvider implements TracerProvider { forceFlush(): Promise { const timeout = this._config.forceFlushTimeoutMillis; - const promises = this.activeSpanProcessor['_spanProcessors'].map( + const promises = this._activeSpanProcessor['_spanProcessors'].map( (spanProcessor: SpanProcessor) => { return new Promise(resolve => { let state: ForceFlushState; @@ -198,7 +197,7 @@ export class BasicTracerProvider implements TracerProvider { } shutdown(): Promise { - return this.activeSpanProcessor.shutdown(); + return this._activeSpanProcessor.shutdown(); } /** diff --git a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts index 4d1a3d399f2..62395128f9f 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts @@ -92,12 +92,12 @@ describe('BasicTracerProvider', () => { it('should use noop span processor by default', () => { const tracer = new BasicTracerProvider(); - assert.ok(tracer['activeSpanProcessor'] instanceof MultiSpanProcessor); + assert.ok(tracer['_activeSpanProcessor'] instanceof MultiSpanProcessor); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'].length === 1 + tracer['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'][0] instanceof + tracer['_activeSpanProcessor']['_spanProcessors'][0] instanceof NoopSpanProcessor ); }); @@ -105,12 +105,12 @@ describe('BasicTracerProvider', () => { const errorStub = sinon.spy(diag, 'error'); const tracer = new BasicTracerProvider(); - assert.ok(tracer['activeSpanProcessor'] instanceof MultiSpanProcessor); + assert.ok(tracer['_activeSpanProcessor'] instanceof MultiSpanProcessor); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'].length === 1 + tracer['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'][0] instanceof + tracer['_activeSpanProcessor']['_spanProcessors'][0] instanceof NoopSpanProcessor ); sinon.assert.notCalled(errorStub); @@ -123,12 +123,12 @@ describe('BasicTracerProvider', () => { const errorStub = sinon.spy(diag, 'error'); const tracer = new BasicTracerProvider(); - assert.ok(tracer['activeSpanProcessor'] instanceof MultiSpanProcessor); + assert.ok(tracer['_activeSpanProcessor'] instanceof MultiSpanProcessor); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'].length === 1 + tracer['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'][0] instanceof + tracer['_activeSpanProcessor']['_spanProcessors'][0] instanceof NoopSpanProcessor ); sinon.assert.calledWith( @@ -147,16 +147,16 @@ describe('BasicTracerProvider', () => { spanProcessors: [spanProcessor], }); - assert.ok(tracer['activeSpanProcessor'] instanceof MultiSpanProcessor); + assert.ok(tracer['_activeSpanProcessor'] instanceof MultiSpanProcessor); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'].length === 1 + tracer['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'][0] instanceof + tracer['_activeSpanProcessor']['_spanProcessors'][0] instanceof SimpleSpanProcessor ); assert.ok( - tracer['activeSpanProcessor']['_spanProcessors'][0][ + tracer['_activeSpanProcessor']['_spanProcessors'][0][ '_exporter' ] instanceof ConsoleSpanExporter ); @@ -466,16 +466,16 @@ 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'] instanceof MultiSpanProcessor); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'].length === 1 + provider['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0] instanceof + provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof BatchSpanProcessor ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0][ + provider['_activeSpanProcessor']['_spanProcessors'][0][ '_exporter' ] instanceof DummyExporter ); @@ -521,16 +521,16 @@ describe('BasicTracerProvider', () => { const provider = new CustomTracerProvider({}); provider.register(); - assert.ok(provider['activeSpanProcessor'] instanceof MultiSpanProcessor); + assert.ok(provider['_activeSpanProcessor'] instanceof MultiSpanProcessor); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'].length === 1 + provider['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0] instanceof + provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof BatchSpanProcessor ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0][ + provider['_activeSpanProcessor']['_spanProcessors'][0][ '_exporter' ] instanceof DummyExporter ); @@ -635,17 +635,17 @@ describe('BasicTracerProvider', () => { provider.register(); assert.ok( - provider['activeSpanProcessor'] instanceof MultiSpanProcessor + provider['_activeSpanProcessor'] instanceof MultiSpanProcessor ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'].length === 1 + provider['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0] instanceof + provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof BatchSpanProcessor ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0][ + provider['_activeSpanProcessor']['_spanProcessors'][0][ '_exporter' ] instanceof InMemorySpanExporter ); @@ -958,7 +958,7 @@ describe('BasicTracerProvider', () => { it('should trigger shutdown when manually invoked', () => { const tracerProvider = new BasicTracerProvider(); const shutdownStub = sinon.stub( - tracerProvider['activeSpanProcessor'], + tracerProvider['_activeSpanProcessor'], 'shutdown' ); tracerProvider.shutdown(); diff --git a/packages/opentelemetry-sdk-trace-base/test/common/MultiSpanProcessor.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/MultiSpanProcessor.test.ts index ddafa8b913b..34292ed8994 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/MultiSpanProcessor.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/MultiSpanProcessor.test.ts @@ -63,7 +63,7 @@ describe('MultiSpanProcessor', () => { assert.strictEqual(processor1.spans.length, 0); span.end(); assert.strictEqual(processor1.spans.length, 1); - tracerProvider['activeSpanProcessor'].shutdown(); + tracerProvider['_activeSpanProcessor'].shutdown(); }); it('should handle two span processor', async () => { diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts index 3912e9ae6f5..41e374e15ce 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts @@ -115,7 +115,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, {}, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); assert.ok(tracer instanceof Tracer); }); @@ -125,7 +125,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, {}, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); assert.strictEqual( tracer['_sampler'].toString(), @@ -138,7 +138,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new AlwaysOffSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan('span1'); assert.ok(!span.isRecording()); @@ -150,7 +150,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new AlwaysOnSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan('span2'); assert.ok(span.isRecording()); @@ -162,7 +162,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new TestSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan('span3'); assert.strictEqual((span as Span).attributes.testAttribute, 'foobar'); @@ -175,7 +175,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new TestSampler(traceState) }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan('stateSpan'); assert.strictEqual(span.spanContext().traceState, traceState); @@ -186,7 +186,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, {}, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const lib: InstrumentationLibrary = tracer.instrumentationLibrary; @@ -203,7 +203,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new TestSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan('span3', undefined, context); @@ -227,7 +227,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, {}, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan( 'aSpan', @@ -249,7 +249,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, {}, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan( 'aSpan', @@ -279,7 +279,7 @@ describe('Tracer', () => { { name: 'default' }, { sampler }, tp['_resource'], - tp['activeSpanProcessor'] + tp['_activeSpanProcessor'] ); const span = tracer.startSpan('a', {}, context) as Span; assert.strictEqual(span.parentSpanId, parent.spanId); @@ -315,7 +315,7 @@ describe('Tracer', () => { { name: 'default' }, { sampler }, tp['_resource'], - tp['activeSpanProcessor'] + tp['_activeSpanProcessor'] ); const span = tracer.startSpan('a', { root: true }, context) as Span; assert.strictEqual(span.parentSpanId, undefined); @@ -334,7 +334,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, {}, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan('my-span'); const context = span.spanContext(); @@ -349,7 +349,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, {}, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan('my-span'); const context = span.spanContext(); @@ -364,7 +364,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, {}, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const span = tracer.startSpan('my-span'); const context = span.spanContext(); @@ -377,7 +377,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new TestSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const spy = sinon.spy(tracer, 'startSpan'); @@ -401,7 +401,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new TestSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const spy = sinon.spy(tracer, 'startSpan'); @@ -429,7 +429,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new TestSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const ctxKey = createContextKey('foo'); @@ -465,7 +465,7 @@ describe('Tracer', () => { { name: 'default', version: '0.0.1' }, { sampler: new TestSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); const attributes = { diff --git a/packages/opentelemetry-sdk-trace-base/test/common/export/TestTracingSpanExporter.ts b/packages/opentelemetry-sdk-trace-base/test/common/export/TestTracingSpanExporter.ts index da79bc8ea16..3153b1f48c6 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/export/TestTracingSpanExporter.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/export/TestTracingSpanExporter.ts @@ -56,7 +56,7 @@ export class TestTracingSpanExporter extends InMemorySpanExporter { { name: 'default', version: '0.0.1' }, { sampler: new AlwaysOnSampler() }, tracerProvider['_resource'], - tracerProvider['activeSpanProcessor'] + tracerProvider['_activeSpanProcessor'] ); } diff --git a/packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts index 2bf1e769f80..f9017f15343 100644 --- a/packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts +++ b/packages/opentelemetry-sdk-trace-node/test/NodeTracerProvider.test.ts @@ -304,18 +304,18 @@ describe('NodeTracerProvider', () => { provider.register(); assert.ok( - provider['activeSpanProcessor'].constructor.name === + provider['_activeSpanProcessor'].constructor.name === 'MultiSpanProcessor' ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'].length === 1 + provider['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0] instanceof + provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof BatchSpanProcessor ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0][ + provider['_activeSpanProcessor']['_spanProcessors'][0][ '_exporter' ] instanceof DummyExporter ); @@ -361,18 +361,18 @@ describe('NodeTracerProvider', () => { provider.register(); assert.ok( - provider['activeSpanProcessor'].constructor.name === + provider['_activeSpanProcessor'].constructor.name === 'MultiSpanProcessor' ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'].length === 1 + provider['_activeSpanProcessor']['_spanProcessors'].length === 1 ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0] instanceof + provider['_activeSpanProcessor']['_spanProcessors'][0] instanceof BatchSpanProcessor ); assert.ok( - provider['activeSpanProcessor']['_spanProcessors'][0][ + provider['_activeSpanProcessor']['_spanProcessors'][0][ '_exporter' ] instanceof DummyExporter ); From 2f53a3a5ff6bfcfddcf1b309ce6b056f691afede Mon Sep 17 00:00:00 2001 From: David Luna Date: Wed, 27 Nov 2024 16:54:29 +0100 Subject: [PATCH 2/3] chore: update changelog --- CHANGELOG_NEXT.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG_NEXT.md b/CHANGELOG_NEXT.md index 83c7005bfad..40018391d1e 100644 --- a/CHANGELOG_NEXT.md +++ b/CHANGELOG_NEXT.md @@ -24,6 +24,7 @@ ### :house: (Internal) * chore: remove checks for unsupported node versions [#4341](https://github.com/open-telemetry/opentelemetry-js/pull/4341) @dyladan -* refactor(sdk-trace-base): remove `BasicTracerProvider._registeredSpanProcessors` private property. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5177) @david-luna +* refactor(sdk-trace-base): remove `BasicTracerProvider._registeredSpanProcessors` private property. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna +* refactor(sdk-trace-base): rename `BasicTracerProvider.activeSpanProcessor` private property. [#XXXX](https://github.com/open-telemetry/opentelemetry-js/pull/XXX) @david-luna ### :bug: (Bug Fix) From 021f7a7dd7a00fb80e756ec476deef909674b3e8 Mon Sep 17 00:00:00 2001 From: David Luna Date: Wed, 27 Nov 2024 16:57:56 +0100 Subject: [PATCH 3/3] chore: update changelog --- CHANGELOG_NEXT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG_NEXT.md b/CHANGELOG_NEXT.md index 40018391d1e..06717c2ebe2 100644 --- a/CHANGELOG_NEXT.md +++ b/CHANGELOG_NEXT.md @@ -25,6 +25,6 @@ * chore: remove checks for unsupported node versions [#4341](https://github.com/open-telemetry/opentelemetry-js/pull/4341) @dyladan * refactor(sdk-trace-base): remove `BasicTracerProvider._registeredSpanProcessors` private property. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna -* refactor(sdk-trace-base): rename `BasicTracerProvider.activeSpanProcessor` private property. [#XXXX](https://github.com/open-telemetry/opentelemetry-js/pull/XXX) @david-luna +* refactor(sdk-trace-base): rename `BasicTracerProvider.activeSpanProcessor` private property. [#5211](https://github.com/open-telemetry/opentelemetry-js/pull/5211) @david-luna ### :bug: (Bug Fix)