Skip to content

Commit 4756568

Browse files
committed
feat(core): Add span.isRecording() instead of span.sampled
To align this with OTEL API.
1 parent e5f3428 commit 4756568

File tree

9 files changed

+47
-4
lines changed

9 files changed

+47
-4
lines changed

MIGRATION.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
1515
* `span.toContext()`: Access the fields directly instead.
1616
* `span.updateWithContext(newSpanContext)`: Update the fields directly instead.
1717
* `span.setName(newName)`: Use `span.updateName(newName)` instead.
18+
* `span.sampled`: Use `span.isRecording()` instead.
1819

1920
## Deprecate `pushScope` & `popScope` in favor of `withScope`
2021

packages/browser/src/profiling/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ export function shouldProfileTransaction(transaction: Transaction): boolean {
515515
return false;
516516
}
517517

518-
if (!transaction.sampled) {
518+
if (!transaction.isRecording()) {
519519
if (DEBUG_BUILD) {
520520
logger.log('[Profiling] Discarding profile because transaction was not sampled.');
521521
}

packages/bun/test/integrations/bunserver.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ describe('Bun Serve Integration', () => {
8080
client.on('finishTransaction', transaction => {
8181
expect(transaction.traceId).toBe(TRACE_ID);
8282
expect(transaction.parentSpanId).toBe(PARENT_SPAN_ID);
83-
expect(transaction.sampled).toBe(true);
83+
expect(transaction.isRecording()).toBe(true);
8484

8585
expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' });
8686
});

packages/core/src/tracing/hubextensions.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ function _startTransaction(
5454
The transaction will not be sampled. Please use the ${configInstrumenter} instrumentation to start transactions.`,
5555
);
5656

57+
// eslint-disable-next-line deprecation/deprecation
5758
transactionContext.sampled = false;
5859
}
5960

@@ -63,7 +64,7 @@ The transaction will not be sampled. Please use the ${configInstrumenter} instru
6364
transactionContext,
6465
...customSamplingContext,
6566
});
66-
if (transaction.sampled) {
67+
if (transaction.isRecording()) {
6768
transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
6869
}
6970
if (client && client.emit) {
@@ -93,7 +94,7 @@ export function startIdleTransaction(
9394
transactionContext,
9495
...customSamplingContext,
9596
});
96-
if (transaction.sampled) {
97+
if (transaction.isRecording()) {
9798
transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
9899
}
99100
if (client && client.emit) {

packages/core/src/tracing/sampling.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,16 @@ export function sampleTransaction<T extends Transaction>(
2121
): T {
2222
// nothing to do if tracing is not enabled
2323
if (!hasTracingEnabled(options)) {
24+
// eslint-disable-next-line deprecation/deprecation
2425
transaction.sampled = false;
2526
return transaction;
2627
}
2728

2829
// if the user has forced a sampling decision by passing a `sampled` value in their transaction context, go with that
30+
// eslint-disable-next-line deprecation/deprecation
2931
if (transaction.sampled !== undefined) {
3032
transaction.setMetadata({
33+
// eslint-disable-next-line deprecation/deprecation
3134
sampleRate: Number(transaction.sampled),
3235
});
3336
return transaction;
@@ -60,6 +63,7 @@ export function sampleTransaction<T extends Transaction>(
6063
// only valid values are booleans or numbers between 0 and 1.)
6164
if (!isValidSampleRate(sampleRate)) {
6265
DEBUG_BUILD && logger.warn('[Tracing] Discarding transaction because of invalid sample rate.');
66+
// eslint-disable-next-line deprecation/deprecation
6367
transaction.sampled = false;
6468
return transaction;
6569
}
@@ -74,15 +78,18 @@ export function sampleTransaction<T extends Transaction>(
7478
: 'a negative sampling decision was inherited or tracesSampleRate is set to 0'
7579
}`,
7680
);
81+
// eslint-disable-next-line deprecation/deprecation
7782
transaction.sampled = false;
7883
return transaction;
7984
}
8085

8186
// Now we roll the dice. Math.random is inclusive of 0, but not of 1, so strict < is safe here. In case sampleRate is
8287
// a boolean, the < comparison will cause it to be automatically cast to 1 if it's true and 0 if it's false.
88+
// eslint-disable-next-line deprecation/deprecation
8389
transaction.sampled = Math.random() < (sampleRate as number | boolean);
8490

8591
// if we're not going to keep it, we're done
92+
// eslint-disable-next-line deprecation/deprecation
8693
if (!transaction.sampled) {
8794
DEBUG_BUILD &&
8895
logger.log(

packages/core/src/tracing/span.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ export class Span implements SpanInterface {
153153
}
154154
// We want to include booleans as well here
155155
if ('sampled' in spanContext) {
156+
// eslint-disable-next-line deprecation/deprecation
156157
this.sampled = spanContext.sampled;
157158
}
158159
if (spanContext.op) {
@@ -192,6 +193,7 @@ export class Span implements SpanInterface {
192193
const childSpan = new Span({
193194
...spanContext,
194195
parentSpanId: this.spanId,
196+
// eslint-disable-next-line deprecation/deprecation
195197
sampled: this.sampled,
196198
traceId: this.traceId,
197199
});
@@ -351,6 +353,7 @@ export class Span implements SpanInterface {
351353
this.endTimestamp = spanContext.endTimestamp;
352354
this.op = spanContext.op;
353355
this.parentSpanId = spanContext.parentSpanId;
356+
// eslint-disable-next-line deprecation/deprecation
354357
this.sampled = spanContext.sampled;
355358
this.spanId = spanContext.spanId || this.spanId;
356359
this.startTimestamp = spanContext.startTimestamp || this.startTimestamp;
@@ -410,6 +413,11 @@ export class Span implements SpanInterface {
410413
});
411414
}
412415

416+
/** @inheritdoc */
417+
public isRecording(): boolean {
418+
return !this.endTimestamp && !!this.sampled;
419+
}
420+
413421
/**
414422
* Get the merged data for this span.
415423
* For now, this combines `data` and `attributes` together,

packages/core/test/lib/tracing/span.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,23 @@ describe('span', () => {
174174
});
175175
});
176176

177+
describe('isRecording', () => {
178+
it('returns true for sampled span', () => {
179+
const span = new Span({ sampled: true });
180+
expect(span.isRecording()).toEqual(true);
181+
});
182+
183+
it('returns false for sampled, finished span', () => {
184+
const span = new Span({ sampled: true, endTimestamp: Date.now() });
185+
expect(span.isRecording()).toEqual(false);
186+
});
187+
188+
it('returns false for unsampled span', () => {
189+
const span = new Span({ sampled: false });
190+
expect(span.isRecording()).toEqual(false);
191+
});
192+
});
193+
177194
// Ensure that attributes & data are merged together
178195
describe('_getData', () => {
179196
it('works without data & attributes', () => {

packages/tracing-internal/src/browser/browsertracing.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ export class BrowserTracing implements Integration {
333333
this._latestRouteName = finalContext.name;
334334
this._latestRouteSource = finalContext.metadata && finalContext.metadata.source;
335335

336+
// eslint-disable-next-line deprecation/deprecation
336337
if (finalContext.sampled === false) {
337338
DEBUG_BUILD && logger.log(`[Tracing] Will not send ${finalContext.op} transaction because of beforeNavigate.`);
338339
}

packages/types/src/span.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ export interface SpanContext {
5353

5454
/**
5555
* Was this span chosen to be sent as part of the sample?
56+
*
57+
* @deprecated Use `isRecording()` instead.
5658
*/
5759
sampled?: boolean;
5860

@@ -265,4 +267,10 @@ export interface Span extends SpanContext {
265267
timestamp?: number;
266268
trace_id: string;
267269
};
270+
271+
/**
272+
* If this is span is actually recording data.
273+
* This will return false if tracing is disabled, this span was not sampled or if the span is already finished.
274+
*/
275+
isRecording(): boolean;
268276
}

0 commit comments

Comments
 (0)