Skip to content

Commit 4813ea4

Browse files
Copilothotlong
andcommitted
fix: address code review — NoopSpanBuilder and configurable batch size
- Replace null SpanBuilder with NoopSpanBuilder when not sampled - Extract auto-flush batch size to configurable option (exporter.batchSize) - Export NoopSpanBuilder and NOOP_SPAN_BUILDER from package index Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent edcad25 commit 4813ea4

File tree

3 files changed

+45
-5
lines changed

3 files changed

+45
-5
lines changed

packages/telemetry/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ export {
4848
export {
4949
SpanManager,
5050
SpanBuilder,
51+
NoopSpanBuilder,
52+
NOOP_SPAN_BUILDER,
5153
generateTraceId,
5254
generateSpanId,
5355
parseTraceparent,

packages/telemetry/src/plugin.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import type {
2828
PluginSecurityManifest,
2929
PluginStartupResult,
3030
} from './types.js';
31-
import { SpanManager, SpanBuilder, extractTraceContext, injectTraceContext } from './span-manager.js';
31+
import { SpanManager, SpanBuilder, NOOP_SPAN_BUILDER, extractTraceContext, injectTraceContext } from './span-manager.js';
3232
import { createExporter } from './exporters.js';
3333

3434
/**
@@ -166,7 +166,7 @@ export class TelemetryPlugin implements Plugin {
166166
options?: { kind?: 'internal' | 'server' | 'client'; attributes?: SpanAttributes; parentContext?: SpanContext },
167167
): Promise<T> {
168168
if (!this.config.enabled || !this.spanManager.shouldSample()) {
169-
return fn(null as any); // pass null when not sampled; fn should handle gracefully
169+
return fn(NOOP_SPAN_BUILDER);
170170
}
171171

172172
const span = this.spanManager.startSpan(name, options?.parentContext);

packages/telemetry/src/span-manager.ts

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,44 @@ export class SpanBuilder {
190190
}
191191
}
192192

193+
// ─── No-op Span Builder ────────────────────────────────────────────────────────
194+
195+
const NOOP_SPAN: Span = {
196+
traceId: '0'.repeat(32),
197+
spanId: '0'.repeat(16),
198+
name: 'noop',
199+
kind: 'internal',
200+
startTime: 0,
201+
endTime: 0,
202+
duration: 0,
203+
status: 'unset',
204+
attributes: {},
205+
events: [],
206+
links: [],
207+
resource: {},
208+
};
209+
210+
/**
211+
* No-op SpanBuilder — all methods are safe to call but do nothing.
212+
* Used when sampling is disabled to avoid null-checks in caller code.
213+
*/
214+
export class NoopSpanBuilder extends SpanBuilder {
215+
constructor() {
216+
super('noop', {}, () => {});
217+
}
218+
219+
override setKind(): this { return this; }
220+
override setAttribute(): this { return this; }
221+
override setAttributes(): this { return this; }
222+
override addEvent(): this { return this; }
223+
override addLink(): this { return this; }
224+
override getContext(): SpanContext { return { traceId: '0'.repeat(32), spanId: '0'.repeat(16), traceFlags: 0 }; }
225+
override end(): Span { return NOOP_SPAN; }
226+
}
227+
228+
/** Singleton no-op span builder */
229+
export const NOOP_SPAN_BUILDER = new NoopSpanBuilder();
230+
193231
// ─── Span Manager ──────────────────────────────────────────────────────────────
194232

195233
/**
@@ -199,7 +237,7 @@ export class SpanManager {
199237
private buffer: Span[] = [];
200238
private exporter?: SpanExporter;
201239
private resource: SpanAttributes;
202-
private config: Required<Pick<TelemetryConfig, 'samplingRate' | 'maxBufferSize' | 'maxAttributes' | 'maxEvents'>>;
240+
private config: Required<Pick<TelemetryConfig, 'samplingRate' | 'maxBufferSize' | 'maxAttributes' | 'maxEvents'>> & { batchSize: number };
203241
private exportTimer?: ReturnType<typeof setInterval>;
204242
private totalSpansCreated = 0;
205243
private totalSpansExported = 0;
@@ -212,6 +250,7 @@ export class SpanManager {
212250
maxBufferSize: config.maxBufferSize ?? 2048,
213251
maxAttributes: config.maxAttributes ?? 128,
214252
maxEvents: config.maxEvents ?? 128,
253+
batchSize: config.exporter?.batchSize ?? 512,
215254
};
216255

217256
this.resource = {
@@ -275,8 +314,7 @@ export class SpanManager {
275314
this.buffer.push(span);
276315

277316
// Auto-flush when batch size is reached
278-
const batchSize = 512;
279-
if (this.buffer.length >= batchSize && this.exporter) {
317+
if (this.buffer.length >= this.config.batchSize && this.exporter) {
280318
this.flush().catch(() => { /* swallow */ });
281319
}
282320
}

0 commit comments

Comments
 (0)