Skip to content

Commit d6ca9a8

Browse files
msonnbclaude
andauthored
feat(core)!: Remove endTimestamp from SentrySpanArguments (#23269)
## What Removes the `endTimestamp` property from `SentrySpanArguments` and related dead code in `SentrySpan` and `_startChildSpan`. ## Why `StartSpanOptions` never declared `endTimestamp`, but `parseSentrySpanArguments` spreads the user's options, so users could ignore TS and have the span to end immediately by adding the `endTimestamp` property. Closes #21405 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent e6e4a76 commit d6ca9a8

6 files changed

Lines changed: 18 additions & 44 deletions

File tree

‎docs/migration/v11-end-state.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1101,7 +1101,9 @@ The same applies when looking the integration up by name, e.g. via `client.getIn
11011101
`any`.
11021102
- Attribute typing and serialization were unified across the SDK.
11031103
- The `attributes` field on the `ScopeData` type is now required. `Scope.getScopeData()` always returned it, so this only affects code that constructs `ScopeData` objects manually — add `attributes: {}` there.
1104-
- The `SentrySpanArguments` interface and related dead code in `SentrySpan` were cleaned up.
1104+
- The `endTimestamp` property was removed from the `SentrySpanArguments` interface. It was never part of
1105+
`StartSpanOptions`, so it could only be passed by ignoring TypeScript, in which case the span ended itself
1106+
during construction. Call `span.end(timestamp)` instead.
11051107
- `BrowserOptions` now supports the `TransportOptions` generic.
11061108
- (Cloudflare) The `env` types and the generics on `withSentry` and `instrumentDurableObjectWithSentry` were reworked for better type safety. If you were not passing explicit generic type parameters, no changes are needed.
11071109

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,18 +116,10 @@ export class SentrySpan implements Span {
116116
if ('sampled' in spanContext) {
117117
this._sampled = spanContext.sampled;
118118
}
119-
if (spanContext.endTimestamp) {
120-
this._endTime = spanContext.endTimestamp;
121-
}
122119

123120
this._events = [];
124121

125122
this._isStandaloneSpan = spanContext.isStandalone;
126-
127-
// If the span is already ended, ensure we finalize the span immediately
128-
if (this._endTime) {
129-
this._onSpanEnded();
130-
}
131123
}
132124

133125
/** @inheritDoc */
@@ -241,12 +233,7 @@ export class SentrySpan implements Span {
241233

242234
/** @inheritdoc */
243235
public end(endTimestamp?: SpanTimeInput): void {
244-
// If already ended, skip the end-of-span processing, but still seal a tracer-provider span. The
245-
// seal at the bottom of this method is skipped on this early return, and `_endTime` may have been
246-
// set before this first `end()` call (e.g. via the constructor's `endTimestamp`), which would
247-
// otherwise leave the span mutable after `end()`. End-of-span processing already ran in that case.
248236
if (this._endTime) {
249-
this._frozen = spanIsTracerProviderSpan(this);
250237
return;
251238
}
252239

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -588,11 +588,6 @@ function _startChildSpan(
588588
}
589589

590590
client.emit('spanStart', childSpan);
591-
// If it has an endTimestamp, it's already ended
592-
if (spanArguments.endTimestamp) {
593-
client.emit('spanEnd', childSpan);
594-
client.emit('afterSpanEnd', childSpan);
595-
}
596591

597592
return childSpan;
598593
}

‎packages/core/src/types/span.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ export interface SpanContextData {
182182

183183
/**
184184
* Interface holding all properties that can be set on a Span on creation.
185-
* This is only used for the legacy span/transaction creation and will go away in v8.
186185
*/
187186
export interface SentrySpanArguments {
188187
/**
@@ -225,11 +224,6 @@ export interface SentrySpanArguments {
225224
*/
226225
startTimestamp?: number | undefined;
227226

228-
/**
229-
* Timestamp in seconds (epoch time) indicating when the span ended.
230-
*/
231-
endTimestamp?: number | undefined;
232-
233227
/**
234228
* Links to associate with the new span. Setting links here is preferred over addLink()
235229
* as certain context information is only available during span creation.

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

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -151,21 +151,20 @@ describe('SentrySpan', () => {
151151
expect(json.links).toHaveLength(1);
152152
});
153153

154-
it('seals a tracer-provider span that ended via the constructor endTimestamp', () => {
155-
// `_endTime` is set in the constructor, so `end()` early-returns before reaching the seal at the
156-
// bottom of its body. The span must still be sealed once `end()` is invoked.
154+
it('keeps a tracer-provider span sealed across repeated `end()` calls', () => {
157155
const span = new SentrySpan({
158156
name: 'original',
159157
startTimestamp: 1,
160-
endTimestamp: 2,
161158
attributes: { key: 'before' },
162159
});
163160
markSpanAsTracerProviderSpan(span);
164161

165-
span.end();
162+
span.end(2);
163+
span.end(3);
166164

167165
span.setAttribute('key', 'after');
168166
expect(spanToStaticSpanJSON(span).data?.['key']).toBe('before');
167+
expect(spanToStaticSpanJSON(span).timestamp).toBe(2);
169168
});
170169
});
171170

@@ -209,20 +208,18 @@ describe('SentrySpan', () => {
209208
name: 'not-sampled',
210209
isStandalone: true,
211210
startTimestamp: 1,
212-
endTimestamp: 2,
213211
sampled: false,
214212
});
215-
notSampledSpan.end();
213+
notSampledSpan.end(2);
216214
expect(mockSend).not.toHaveBeenCalled();
217215

218216
const sampledSpan = new SentrySpan({
219217
name: 'is-sampled',
220218
isStandalone: true,
221219
startTimestamp: 1,
222-
endTimestamp: 2,
223220
sampled: true,
224221
});
225-
sampledSpan.end();
222+
sampledSpan.end(2);
226223
expect(mockSend).toHaveBeenCalledTimes(1);
227224
});
228225

@@ -243,10 +240,9 @@ describe('SentrySpan', () => {
243240
name: 'test',
244241
isStandalone: true,
245242
startTimestamp: 1,
246-
endTimestamp: 2,
247243
sampled: true,
248244
});
249-
span.end();
245+
span.end(2);
250246
expect(mockSend).toHaveBeenCalled();
251247
});
252248

@@ -275,10 +271,9 @@ describe('SentrySpan', () => {
275271
name: 'test',
276272
isStandalone: true,
277273
startTimestamp: 1,
278-
endTimestamp: 2,
279274
sampled: true,
280275
});
281-
span.end();
276+
span.end(2);
282277

283278
expect(beforeSendSpan).toHaveBeenCalledTimes(1);
284279
expect(mockSend).toHaveBeenCalled();
@@ -313,10 +308,9 @@ describe('SentrySpan', () => {
313308
name: 'test',
314309
isStandalone: true,
315310
startTimestamp: 1,
316-
endTimestamp: 2,
317311
sampled: true,
318312
});
319-
span.end();
313+
span.end(2);
320314

321315
expect(seen[0]!['my.scope.attr']).toBeUndefined();
322316
});
@@ -575,8 +569,9 @@ describe('SentrySpan', () => {
575569
it('skips if span is already ended', () => {
576570
const startTimestamp = timestampInSeconds() - 5;
577571
const endTimestamp = timestampInSeconds() - 1;
578-
const span = new SentrySpan({ startTimestamp, endTimestamp });
572+
const span = new SentrySpan({ startTimestamp });
579573

574+
span.end(endTimestamp);
580575
span.end();
581576

582577
expect(spanToStaticSpanJSON(span).timestamp).toBe(endTimestamp);
@@ -590,7 +585,8 @@ describe('SentrySpan', () => {
590585
});
591586

592587
it('returns false for sampled, finished span', () => {
593-
const span = new SentrySpan({ sampled: true, endTimestamp: Date.now() });
588+
const span = new SentrySpan({ sampled: true });
589+
span.end();
594590
expect(span.isRecording()).toEqual(false);
595591
});
596592

‎packages/core/test/lib/utils/spanUtils.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,12 +342,12 @@ describe('spanToStaticSpanJSON', () => {
342342
spanId: '5678',
343343
traceId: 'abcd',
344344
startTimestamp: 123,
345-
endTimestamp: 456,
346345
attributes: {
347346
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto',
348347
},
349348
});
350349
span.setStatus({ code: SPAN_STATUS_OK });
350+
span.end(456);
351351

352352
expect(spanToStaticSpanJSON(span)).toEqual({
353353
description: 'test name',
@@ -450,7 +450,6 @@ describe('spanToStaticSpanJSON', () => {
450450
spanId: '5678',
451451
traceId: 'abcd',
452452
startTimestamp: 123,
453-
endTimestamp: 456,
454453
attributes: {
455454
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto',
456455
attr1: 'value1',
@@ -472,6 +471,7 @@ describe('spanToStaticSpanJSON', () => {
472471
});
473472
span.setStatus({ code: SPAN_STATUS_OK });
474473
span.setAttribute('attr4', [1, 2, 3]);
474+
span.end(456);
475475

476476
expect(spanToJSON(span)).toEqual({
477477
name: 'test name',

0 commit comments

Comments
 (0)