Skip to content

Commit d5cbc78

Browse files
mydeamydea
committed
adjust to spans always existing
1 parent af495eb commit d5cbc78

File tree

2 files changed

+47
-41
lines changed

2 files changed

+47
-41
lines changed

‎packages/core/src/tracing/idleSpan.ts

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import { logger, timestampInSeconds } from '@sentry/utils';
33
import { getClient, getCurrentScope } from '../currentScopes';
44

55
import { DEBUG_BUILD } from '../debug-build';
6+
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
67
import { spanToJSON } from '../utils/spanUtils';
8+
import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
79
import { SPAN_STATUS_ERROR } from './spanstatus';
810
import { startInactiveSpan } from './trace';
911
import { getActiveSpan, getSpanDescendants, removeChildSpanFromSpan } from './utils';
@@ -71,10 +73,7 @@ interface IdleSpanOptions {
7173
* An idle span is a span that automatically finishes. It does this by tracking child spans as activities.
7274
* An idle span is always the active span.
7375
*/
74-
export function startIdleSpan(
75-
startSpanOptions: StartSpanOptions,
76-
options: Partial<IdleSpanOptions> = {},
77-
): Span | undefined {
76+
export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Partial<IdleSpanOptions> = {}): Span {
7877
// Activities store a list of active spans
7978
const activities = new Map<string, boolean>();
8079

@@ -101,21 +100,13 @@ export function startIdleSpan(
101100

102101
const client = getClient();
103102

104-
if (!client) {
105-
return;
103+
if (!client || !hasTracingEnabled()) {
104+
return new SentryNonRecordingSpan();
106105
}
107106

108107
const scope = getCurrentScope();
109108
const previousActiveSpan = getActiveSpan();
110-
const _span = _startIdleSpan(startSpanOptions);
111-
112-
// Span _should_ always be defined here, but TS does not know that...
113-
if (!_span) {
114-
return;
115-
}
116-
117-
// For TS, so that we know everything below here has a span
118-
const span = _span;
109+
const span = _startIdleSpan(startSpanOptions);
119110

120111
/**
121112
* Cancels the existing idle timeout, if there is one.
@@ -319,15 +310,13 @@ export function startIdleSpan(
319310
return span;
320311
}
321312

322-
function _startIdleSpan(options: StartSpanOptions): Span | undefined {
313+
function _startIdleSpan(options: StartSpanOptions): Span {
323314
const span = startInactiveSpan(options);
324315

325316
// eslint-disable-next-line deprecation/deprecation
326317
getCurrentScope().setSpan(span);
327318

328-
if (span) {
329-
DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`);
330-
}
319+
DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`);
331320

332321
return span;
333322
}

‎packages/core/test/lib/tracing/idleSpan.test.ts

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
22

33
import type { Event, Span } from '@sentry/types';
44
import {
5+
SentryNonRecordingSpan,
6+
SentrySpan,
57
addTracingExtensions,
68
getActiveSpan,
79
getClient,
@@ -40,6 +42,7 @@ describe('startIdleSpan', () => {
4042
it('sets & unsets the idle span on the scope', () => {
4143
const idleSpan = startIdleSpan({ name: 'foo' });
4244
expect(idleSpan).toBeDefined();
45+
expect(idleSpan).toBeInstanceOf(SentrySpan);
4346

4447
expect(getActiveSpan()).toBe(idleSpan);
4548

@@ -49,12 +52,26 @@ describe('startIdleSpan', () => {
4952
expect(getActiveSpan()).toBe(undefined);
5053
});
5154

55+
it('returns non recording span if tracing is disabled', () => {
56+
const options = getDefaultTestClientOptions({ dsn });
57+
const client = new TestClient(options);
58+
setCurrentClient(client);
59+
client.init();
60+
61+
const idleSpan = startIdleSpan({ name: 'foo' });
62+
expect(idleSpan).toBeDefined();
63+
expect(idleSpan).toBeInstanceOf(SentryNonRecordingSpan);
64+
65+
// not set as active span, though
66+
expect(getActiveSpan()).toBe(undefined);
67+
});
68+
5269
it('does not finish idle span if there are still active activities', () => {
53-
const idleSpan = startIdleSpan({ name: 'foo' })!;
70+
const idleSpan = startIdleSpan({ name: 'foo' });
5471
expect(idleSpan).toBeDefined();
5572

5673
startSpanManual({ name: 'inner1' }, span => {
57-
const childSpan = startInactiveSpan({ name: 'inner2' })!;
74+
const childSpan = startInactiveSpan({ name: 'inner2' });
5875

5976
span?.end();
6077
jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout + 1);
@@ -72,7 +89,7 @@ describe('startIdleSpan', () => {
7289

7390
it('calls beforeSpanEnd callback before finishing', () => {
7491
const beforeSpanEnd = jest.fn();
75-
const idleSpan = startIdleSpan({ name: 'foo' }, { beforeSpanEnd })!;
92+
const idleSpan = startIdleSpan({ name: 'foo' }, { beforeSpanEnd });
7693
expect(idleSpan).toBeDefined();
7794

7895
expect(beforeSpanEnd).not.toHaveBeenCalled();
@@ -108,7 +125,7 @@ describe('startIdleSpan', () => {
108125
const inner = startInactiveSpan({ name: 'from beforeSpanEnd', startTime: baseTimeInSeconds });
109126
inner?.end(baseTimeInSeconds);
110127
});
111-
const idleSpan = startIdleSpan({ name: 'idle span 2', startTime: baseTimeInSeconds }, { beforeSpanEnd })!;
128+
const idleSpan = startIdleSpan({ name: 'idle span 2', startTime: baseTimeInSeconds }, { beforeSpanEnd });
112129
expect(idleSpan).toBeDefined();
113130

114131
expect(beforeSpanEnd).not.toHaveBeenCalled();
@@ -152,26 +169,26 @@ describe('startIdleSpan', () => {
152169
// We want to accomodate a bit of drift there, so we ensure this starts earlier...
153170
const baseTimeInSeconds = Math.floor(Date.now() / 1000) - 9999;
154171

155-
const idleSpan = startIdleSpan({ name: 'idle span', startTime: baseTimeInSeconds })!;
172+
const idleSpan = startIdleSpan({ name: 'idle span', startTime: baseTimeInSeconds });
156173
expect(idleSpan).toBeDefined();
157174

158175
// regular child - should be kept
159176
const regularSpan = startInactiveSpan({
160177
name: 'regular span',
161178
startTime: baseTimeInSeconds + 2,
162-
})!;
179+
});
163180

164181
// discardedSpan - startTimestamp is too large
165-
const discardedSpan = startInactiveSpan({ name: 'discarded span', startTime: baseTimeInSeconds + 99 })!;
182+
const discardedSpan = startInactiveSpan({ name: 'discarded span', startTime: baseTimeInSeconds + 99 });
166183
// discardedSpan2 - endTime is too large
167-
const discardedSpan2 = startInactiveSpan({ name: 'discarded span', startTime: baseTimeInSeconds + 3 })!;
184+
const discardedSpan2 = startInactiveSpan({ name: 'discarded span', startTime: baseTimeInSeconds + 3 });
168185
discardedSpan2.end(baseTimeInSeconds + 99)!;
169186

170187
// Should be cancelled - will not finish
171188
const cancelledSpan = startInactiveSpan({
172189
name: 'cancelled span',
173190
startTime: baseTimeInSeconds + 4,
174-
})!;
191+
});
175192

176193
regularSpan.end(baseTimeInSeconds + 4);
177194
idleSpan.end(baseTimeInSeconds + 10);
@@ -225,7 +242,7 @@ describe('startIdleSpan', () => {
225242
hookSpans.push({ span, hook: 'spanEnd' });
226243
});
227244

228-
const idleSpan = startIdleSpan({ name: 'idle span' })!;
245+
const idleSpan = startIdleSpan({ name: 'idle span' });
229246
expect(idleSpan).toBeDefined();
230247

231248
expect(hookSpans).toEqual([{ span: idleSpan, hook: 'spanStart' }]);
@@ -250,7 +267,7 @@ describe('startIdleSpan', () => {
250267

251268
const recordDroppedEventSpy = jest.spyOn(client, 'recordDroppedEvent');
252269

253-
const idleSpan = startIdleSpan({ name: 'idle span' })!;
270+
const idleSpan = startIdleSpan({ name: 'idle span' });
254271
expect(idleSpan).toBeDefined();
255272

256273
idleSpan?.end();
@@ -260,15 +277,15 @@ describe('startIdleSpan', () => {
260277

261278
describe('idleTimeout', () => {
262279
it('finishes if no activities are added to the idle span', () => {
263-
const idleSpan = startIdleSpan({ name: 'idle span' })!;
280+
const idleSpan = startIdleSpan({ name: 'idle span' });
264281
expect(idleSpan).toBeDefined();
265282

266283
jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout);
267284
expect(spanToJSON(idleSpan).timestamp).toBeDefined();
268285
});
269286

270287
it('does not finish if a activity is started', () => {
271-
const idleSpan = startIdleSpan({ name: 'idle span' })!;
288+
const idleSpan = startIdleSpan({ name: 'idle span' });
272289
expect(idleSpan).toBeDefined();
273290

274291
startInactiveSpan({ name: 'span' });
@@ -279,7 +296,7 @@ describe('startIdleSpan', () => {
279296

280297
it('does not finish when idleTimeout is not exceed after last activity finished', () => {
281298
const idleTimeout = 10;
282-
const idleSpan = startIdleSpan({ name: 'idle span' }, { idleTimeout })!;
299+
const idleSpan = startIdleSpan({ name: 'idle span' }, { idleTimeout });
283300
expect(idleSpan).toBeDefined();
284301

285302
startSpan({ name: 'span1' }, () => {});
@@ -295,7 +312,7 @@ describe('startIdleSpan', () => {
295312

296313
it('finish when idleTimeout is exceeded after last activity finished', () => {
297314
const idleTimeout = 10;
298-
const idleSpan = startIdleSpan({ name: 'idle span', startTime: 1234 }, { idleTimeout })!;
315+
const idleSpan = startIdleSpan({ name: 'idle span', startTime: 1234 }, { idleTimeout });
299316
expect(idleSpan).toBeDefined();
300317

301318
startSpan({ name: 'span1' }, () => {});
@@ -312,7 +329,7 @@ describe('startIdleSpan', () => {
312329

313330
describe('child span timeout', () => {
314331
it('finishes when a child span exceed timeout', () => {
315-
const idleSpan = startIdleSpan({ name: 'idle span' })!;
332+
const idleSpan = startIdleSpan({ name: 'idle span' });
316333
expect(idleSpan).toBeDefined();
317334

318335
// Start any span to cancel idle timeout
@@ -333,7 +350,7 @@ describe('startIdleSpan', () => {
333350
});
334351

335352
it('resets after new activities are added', () => {
336-
const idleSpan = startIdleSpan({ name: 'idle span' }, { finalTimeout: 99_999 })!;
353+
const idleSpan = startIdleSpan({ name: 'idle span' }, { finalTimeout: 99_999 });
337354
expect(idleSpan).toBeDefined();
338355

339356
// Start any span to cancel idle timeout
@@ -370,7 +387,7 @@ describe('startIdleSpan', () => {
370387

371388
describe('disableAutoFinish', () => {
372389
it('skips idle timeout if disableAutoFinish=true', () => {
373-
const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true })!;
390+
const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true });
374391
expect(idleSpan).toBeDefined();
375392

376393
jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout);
@@ -387,7 +404,7 @@ describe('startIdleSpan', () => {
387404
});
388405

389406
it('skips span timeout if disableAutoFinish=true', () => {
390-
const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true, finalTimeout: 99_999 })!;
407+
const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true, finalTimeout: 99_999 });
391408
expect(idleSpan).toBeDefined();
392409

393410
startInactiveSpan({ name: 'inner' });
@@ -406,16 +423,16 @@ describe('startIdleSpan', () => {
406423
});
407424

408425
it('times out at final timeout if disableAutoFinish=true', () => {
409-
const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true })!;
426+
const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true });
410427
expect(idleSpan).toBeDefined();
411428

412429
jest.advanceTimersByTime(TRACING_DEFAULTS.finalTimeout);
413430
expect(spanToJSON(idleSpan).timestamp).toBeDefined();
414431
});
415432

416433
it('ignores it if hook is emitted with other span', () => {
417-
const span = startInactiveSpan({ name: 'other span' })!;
418-
const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true })!;
434+
const span = startInactiveSpan({ name: 'other span' });
435+
const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true });
419436
expect(idleSpan).toBeDefined();
420437

421438
jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout);

0 commit comments

Comments
 (0)