Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(sdk-trace-base): rename activeSpanProcessor private property #5211

Merged
merged 3 commits into from
Nov 27, 2024
Merged

refactor(sdk-trace-base): rename activeSpanProcessor private property #5211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d5f465
chore(sdk-trace-base): rename activeSpanProcessor
david-luna Nov 27, 2024
2f53a3a
chore: update changelog
david-luna Nov 27, 2024
021f7a7
chore: update changelog
david-luna Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG_NEXT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for reviewer: the link was incorrect

* refactor(sdk-trace-base): rename `BasicTracerProvider.activeSpanProcessor` private property. [#5211](https://github.com/open-telemetry/opentelemetry-js/pull/5211) @david-luna

### :bug: (Bug Fix)
4 changes: 2 additions & 2 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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');

Expand Down
11 changes: 5 additions & 6 deletions packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ export class BasicTracerProvider implements TracerProvider {
private readonly _config: TracerConfig;
private readonly _tracers: Map<string, Tracer> = new Map();
private readonly _resource: IResource;

private activeSpanProcessor: MultiSpanProcessor;
private readonly _activeSpanProcessor: MultiSpanProcessor;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for reviewer: also made it readonly because is not meant to be changed once is set in the constructor


constructor(config: TracerConfig = {}) {
const mergedConfig = merge(
Expand Down Expand Up @@ -101,7 +100,7 @@ export class BasicTracerProvider implements TracerProvider {
);
}

this.activeSpanProcessor = new MultiSpanProcessor(spanProcessors);
this._activeSpanProcessor = new MultiSpanProcessor(spanProcessors);
}

getTracer(
Expand All @@ -117,7 +116,7 @@ export class BasicTracerProvider implements TracerProvider {
{ name, version, schemaUrl: options?.schemaUrl },
this._config,
this._resource,
this.activeSpanProcessor
this._activeSpanProcessor
)
);
}
Expand Down Expand Up @@ -150,7 +149,7 @@ export class BasicTracerProvider implements TracerProvider {

forceFlush(): Promise<void> {
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;
Expand Down Expand Up @@ -198,7 +197,7 @@ export class BasicTracerProvider implements TracerProvider {
}

shutdown(): Promise<void> {
return this.activeSpanProcessor.shutdown();
return this._activeSpanProcessor.shutdown();
}

/**
Expand Down
52 changes: 26 additions & 26 deletions packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,25 +92,25 @@ 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
);
});
it('should use noop span processor by default and no diag error', () => {
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);
Expand All @@ -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(
Expand All @@ -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
);
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-sdk-trace-base/test/common/MultiSpanProcessor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
38 changes: 19 additions & 19 deletions packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('Tracer', () => {
{ name: 'default', version: '0.0.1' },
{},
tracerProvider['_resource'],
tracerProvider['activeSpanProcessor']
tracerProvider['_activeSpanProcessor']
);
assert.ok(tracer instanceof Tracer);
});
Expand All @@ -125,7 +125,7 @@ describe('Tracer', () => {
{ name: 'default', version: '0.0.1' },
{},
tracerProvider['_resource'],
tracerProvider['activeSpanProcessor']
tracerProvider['_activeSpanProcessor']
);
assert.strictEqual(
tracer['_sampler'].toString(),
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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');
Expand All @@ -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);
Expand All @@ -186,7 +186,7 @@ describe('Tracer', () => {
{ name: 'default', version: '0.0.1' },
{},
tracerProvider['_resource'],
tracerProvider['activeSpanProcessor']
tracerProvider['_activeSpanProcessor']
);

const lib: InstrumentationLibrary = tracer.instrumentationLibrary;
Expand All @@ -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);
Expand All @@ -227,7 +227,7 @@ describe('Tracer', () => {
{ name: 'default', version: '0.0.1' },
{},
tracerProvider['_resource'],
tracerProvider['activeSpanProcessor']
tracerProvider['_activeSpanProcessor']
);
const span = tracer.startSpan(
'aSpan',
Expand All @@ -249,7 +249,7 @@ describe('Tracer', () => {
{ name: 'default', version: '0.0.1' },
{},
tracerProvider['_resource'],
tracerProvider['activeSpanProcessor']
tracerProvider['_activeSpanProcessor']
);
const span = tracer.startSpan(
'aSpan',
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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');
Expand All @@ -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');
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -465,7 +465,7 @@ describe('Tracer', () => {
{ name: 'default', version: '0.0.1' },
{ sampler: new TestSampler() },
tracerProvider['_resource'],
tracerProvider['activeSpanProcessor']
tracerProvider['_activeSpanProcessor']
);

const attributes = {
Expand Down
Loading
Loading