Skip to content

Commit f3a6310

Browse files
david-lunadavid-luna
authored
refactor(sdk-trace-base): remove _registeredSpanProcessors from BasicTracerProvider (#5177)
1 parent b7343ef commit f3a6310

File tree

6 files changed

+202
-98
lines changed

6 files changed

+202
-98
lines changed

‎CHANGELOG_NEXT.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,6 @@
2323
### :house: (Internal)
2424

2525
* chore: remove checks for unsupported node versions [#4341](https://github.com/open-telemetry/opentelemetry-js/pull/4341) @dyladan
26+
* refactor(sdk-trace-base): remove `BasicTracerProvider._registeredSpanProcessors` private property. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5177) @david-luna
2627

2728
### :bug: (Bug Fix)

‎experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts‎

Lines changed: 50 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
NoopSpanProcessor,
5151
IdGenerator,
5252
AlwaysOffSampler,
53+
SpanProcessor,
5354
} from '@opentelemetry/sdk-trace-base';
5455
import * as assert from 'assert';
5556
import * as semver from 'semver';
@@ -257,13 +258,25 @@ describe('Node SDK', () => {
257258
);
258259
const apiTracerProvider =
259260
trace.getTracerProvider() as ProxyTracerProvider;
260-
assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider);
261+
const nodeTracerProvider = apiTracerProvider.getDelegate();
262+
263+
assert.ok(nodeTracerProvider instanceof NodeTracerProvider);
264+
265+
const spanProcessor = nodeTracerProvider.getActiveSpanProcessor() as any;
266+
267+
assert(
268+
spanProcessor.constructor.name === 'MultiSpanProcessor',
269+
'is MultiSpanProcessor'
270+
);
261271

262-
const listOfProcessors =
263-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
272+
const listOfProcessors = spanProcessor[
273+
'_spanProcessors'
274+
] as SpanProcessor[];
264275

265-
assert(sdk['_tracerProvider'] instanceof NodeTracerProvider);
266-
assert(listOfProcessors.length === 3);
276+
assert(
277+
listOfProcessors.length === 3,
278+
'it has the right amount of processors'
279+
);
267280
assert(listOfProcessors[0] instanceof NoopSpanProcessor);
268281
assert(listOfProcessors[1] instanceof SimpleSpanProcessor);
269282
assert(listOfProcessors[2] instanceof BatchSpanProcessor);
@@ -1099,6 +1112,18 @@ describe('Node SDK', () => {
10991112
describe('setup exporter from env', () => {
11001113
let stubLoggerError: Sinon.SinonStub;
11011114

1115+
const getSdkSpanProcessors = (sdk: NodeSDK) => {
1116+
const tracerProvider = sdk['_tracerProvider'];
1117+
1118+
assert(tracerProvider instanceof NodeTracerProvider);
1119+
1120+
const activeSpanProcessor = tracerProvider.getActiveSpanProcessor();
1121+
1122+
assert(activeSpanProcessor.constructor.name === 'MultiSpanProcessor');
1123+
1124+
return (activeSpanProcessor as any)['_spanProcessors'] as SpanProcessor[];
1125+
};
1126+
11021127
beforeEach(() => {
11031128
stubLoggerError = Sinon.stub(diag, 'warn');
11041129
});
@@ -1109,8 +1134,7 @@ describe('setup exporter from env', () => {
11091134
it('should use default exporter when nor env neither SDK config is given', async () => {
11101135
const sdk = new NodeSDK();
11111136
sdk.start();
1112-
const listOfProcessors =
1113-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1137+
const listOfProcessors = getSdkSpanProcessors(sdk);
11141138

11151139
assert(listOfProcessors.length === 1);
11161140
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1124,8 +1148,7 @@ describe('setup exporter from env', () => {
11241148
traceExporter,
11251149
});
11261150
sdk.start();
1127-
const listOfProcessors =
1128-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1151+
const listOfProcessors = getSdkSpanProcessors(sdk);
11291152

11301153
assert(listOfProcessors.length === 1);
11311154
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1140,8 +1163,7 @@ describe('setup exporter from env', () => {
11401163
spanProcessor,
11411164
});
11421165
sdk.start();
1143-
const listOfProcessors =
1144-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1166+
const listOfProcessors = getSdkSpanProcessors(sdk);
11451167

11461168
assert(listOfProcessors.length === 1);
11471169
assert(listOfProcessors[0] instanceof SimpleSpanProcessor);
@@ -1156,8 +1178,7 @@ describe('setup exporter from env', () => {
11561178
traceExporter,
11571179
});
11581180
sdk.start();
1159-
const listOfProcessors =
1160-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1181+
const listOfProcessors = getSdkSpanProcessors(sdk);
11611182

11621183
assert(listOfProcessors.length === 1);
11631184
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1172,8 +1193,7 @@ describe('setup exporter from env', () => {
11721193
sampler: new AlwaysOffSampler(),
11731194
});
11741195
sdk.start();
1175-
const listOfProcessors =
1176-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1196+
const listOfProcessors = getSdkSpanProcessors(sdk);
11771197

11781198
assert.ok(
11791199
sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler
@@ -1190,9 +1210,7 @@ describe('setup exporter from env', () => {
11901210
env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = 'grpc';
11911211
const sdk = new NodeSDK();
11921212
sdk.start();
1193-
1194-
const listOfProcessors =
1195-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1213+
const listOfProcessors = getSdkSpanProcessors(sdk);
11961214

11971215
assert(listOfProcessors.length === 1);
11981216
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1208,9 +1226,7 @@ describe('setup exporter from env', () => {
12081226
env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = 'grpc';
12091227
const sdk = new NodeSDK();
12101228
sdk.start();
1211-
1212-
const listOfProcessors =
1213-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1229+
const listOfProcessors = getSdkSpanProcessors(sdk);
12141230

12151231
assert(listOfProcessors.length === 1);
12161232
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1231,12 +1247,10 @@ describe('setup exporter from env', () => {
12311247
'OTEL_TRACES_EXPORTER contains "none". SDK will not be initialized.'
12321248
);
12331249

1234-
const listOfProcessors =
1235-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1236-
const activeProcessor = sdk['_tracerProvider']?.getActiveSpanProcessor();
1250+
const listOfProcessors = getSdkSpanProcessors(sdk);
12371251

1238-
assert(listOfProcessors.length === 0);
1239-
assert(activeProcessor instanceof NoopSpanProcessor);
1252+
assert(listOfProcessors.length === 1);
1253+
assert(listOfProcessors[0] instanceof NoopSpanProcessor);
12401254
delete env.OTEL_TRACES_EXPORTER;
12411255
await sdk.shutdown();
12421256
});
@@ -1246,8 +1260,7 @@ describe('setup exporter from env', () => {
12461260
const sdk = new NodeSDK();
12471261
sdk.start();
12481262

1249-
const listOfProcessors =
1250-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1263+
const listOfProcessors = getSdkSpanProcessors(sdk);
12511264

12521265
assert(listOfProcessors.length === 1);
12531266
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1267,8 +1280,7 @@ describe('setup exporter from env', () => {
12671280
'OTEL_TRACES_EXPORTER contains "none" along with other exporters. Using default otlp exporter.'
12681281
);
12691282

1270-
const listOfProcessors =
1271-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1283+
const listOfProcessors = getSdkSpanProcessors(sdk);
12721284

12731285
assert(listOfProcessors.length === 1);
12741286
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1303,8 +1315,7 @@ describe('setup exporter from env', () => {
13031315
const sdk = new NodeSDK();
13041316
sdk.start();
13051317

1306-
const listOfProcessors =
1307-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1318+
const listOfProcessors = getSdkSpanProcessors(sdk);
13081319

13091320
assert(listOfProcessors.length === 1);
13101321
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1321,8 +1332,7 @@ describe('setup exporter from env', () => {
13211332
const sdk = new NodeSDK();
13221333
sdk.start();
13231334

1324-
const listOfProcessors =
1325-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1335+
const listOfProcessors = getSdkSpanProcessors(sdk);
13261336

13271337
assert(listOfProcessors.length === 2);
13281338
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1341,8 +1351,7 @@ describe('setup exporter from env', () => {
13411351
const sdk = new NodeSDK();
13421352
sdk.start();
13431353

1344-
const listOfProcessors =
1345-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1354+
const listOfProcessors = getSdkSpanProcessors(sdk);
13461355

13471356
assert(listOfProcessors.length === 1);
13481357
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1359,8 +1368,7 @@ describe('setup exporter from env', () => {
13591368
const sdk = new NodeSDK();
13601369
sdk.start();
13611370

1362-
const listOfProcessors =
1363-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1371+
const listOfProcessors = getSdkSpanProcessors(sdk);
13641372

13651373
assert(listOfProcessors.length === 2);
13661374
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1379,8 +1387,7 @@ describe('setup exporter from env', () => {
13791387
const sdk = new NodeSDK();
13801388
sdk.start();
13811389

1382-
const listOfProcessors =
1383-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1390+
const listOfProcessors = getSdkSpanProcessors(sdk);
13841391

13851392
assert(listOfProcessors.length === 3);
13861393
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1400,8 +1407,7 @@ describe('setup exporter from env', () => {
14001407
const sdk = new NodeSDK();
14011408
sdk.start();
14021409

1403-
const listOfProcessors =
1404-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1410+
const listOfProcessors = getSdkSpanProcessors(sdk);
14051411

14061412
assert(listOfProcessors.length === 2);
14071413
assert(listOfProcessors[0] instanceof SimpleSpanProcessor);
@@ -1417,8 +1423,7 @@ describe('setup exporter from env', () => {
14171423
const sdk = new NodeSDK();
14181424
sdk.start();
14191425

1420-
const listOfProcessors =
1421-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1426+
const listOfProcessors = getSdkSpanProcessors(sdk);
14221427

14231428
assert(listOfProcessors.length === 1);
14241429
assert(listOfProcessors[0] instanceof SimpleSpanProcessor);
@@ -1433,8 +1438,7 @@ describe('setup exporter from env', () => {
14331438
const sdk = new NodeSDK();
14341439
sdk.start();
14351440

1436-
const listOfProcessors =
1437-
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
1441+
const listOfProcessors = getSdkSpanProcessors(sdk);
14381442

14391443
assert(listOfProcessors.length === 1);
14401444
assert(listOfProcessors[0] instanceof SimpleSpanProcessor);

‎packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,9 @@ export class BasicTracerProvider implements TracerProvider {
6767
>();
6868

6969
private readonly _config: TracerConfig;
70-
private readonly _registeredSpanProcessors: SpanProcessor[] = [];
7170
private readonly _tracers: Map<string, Tracer> = new Map();
7271

73-
private activeSpanProcessor: SpanProcessor;
72+
private activeSpanProcessor: MultiSpanProcessor;
7473
readonly resource: IResource;
7574

7675
constructor(config: TracerConfig = {}) {
@@ -89,20 +88,20 @@ export class BasicTracerProvider implements TracerProvider {
8988
resource: this.resource,
9089
});
9190

91+
const spanProcessors: SpanProcessor[] = [];
92+
9293
if (config.spanProcessors?.length) {
93-
this._registeredSpanProcessors = [...config.spanProcessors];
94-
this.activeSpanProcessor = new MultiSpanProcessor(
95-
this._registeredSpanProcessors
96-
);
94+
spanProcessors.push(...config.spanProcessors);
9795
} else {
9896
const defaultExporter = this._buildExporterFromEnv();
99-
if (defaultExporter !== undefined) {
100-
const batchProcessor = new BatchSpanProcessor(defaultExporter);
101-
this.activeSpanProcessor = batchProcessor;
102-
} else {
103-
this.activeSpanProcessor = new NoopSpanProcessor();
104-
}
97+
spanProcessors.push(
98+
defaultExporter
99+
? new BatchSpanProcessor(defaultExporter)
100+
: new NoopSpanProcessor()
101+
);
105102
}
103+
104+
this.activeSpanProcessor = new MultiSpanProcessor(spanProcessors);
106105
}
107106

108107
getTracer(
@@ -154,7 +153,7 @@ export class BasicTracerProvider implements TracerProvider {
154153

155154
forceFlush(): Promise<void> {
156155
const timeout = this._config.forceFlushTimeoutMillis;
157-
const promises = this._registeredSpanProcessors.map(
156+
const promises = this.activeSpanProcessor['_spanProcessors'].map(
158157
(spanProcessor: SpanProcessor) => {
159158
return new Promise(resolve => {
160159
let state: ForceFlushState;

‎packages/opentelemetry-sdk-trace-base/src/Span.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,8 @@ export class SpanImpl implements Span {
9797
private readonly _startTimeProvided: boolean;
9898

9999
/**
100-
* Constructs a new Span instance.
101-
*
102-
* @deprecated calling Span constructor directly is not supported. Please use tracer.startSpan.
103-
* */
100+
* Constructs a new SpanImpl instance.
101+
*/
104102
constructor(
105103
parentTracer: Tracer,
106104
context: Context,

0 commit comments

Comments
 (0)