Skip to content

Commit

Permalink
feat(core): Deprecate Span.isSuccess() in favor of reading span sta…
Browse files Browse the repository at this point in the history
…tus (#10213)

Deprecate `Span.isSuccess()` in favor of directly reading the
span status via `spanToJSON(span).status === 'ok'` instead. There's no
need for this API as it's just syntactic sugar around the status.
  • Loading branch information
Lms24 authored Jan 17, 2024
1 parent 14bf0a0 commit f9ed885
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 1 deletion.
1 change: 1 addition & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
- `span.spanRecorder`: Span recording will be handled internally by the SDK.
- `span.status`: Use `.setStatus` to set or update and `spanToJSON()` to read the span status.
- `span.op`: Use `startSpan` functions to set, `setAttribute()` to update and `spanToJSON` to read the span operation.
- `span.isSuccess`: Use `spanToJSON(span).status === 'ok'` instead.
- `transaction.setMetadata()`: Use attributes instead, or set data on the scope.
- `transaction.metadata`: Use attributes instead, or set data on the scope.
- `transaction.setContext()`: Set context on the surrounding scope instead.
Expand Down
2 changes: 1 addition & 1 deletion docs/v8-new-performance-apis.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ below to see which things used to exist, and how they can/should be mapped going
| `setHttpStatus()` | ??? TODO |
| `setName()` | `updateName()` |
| `startChild()` | Call `Sentry.startSpan()` independently |
| `isSuccess()` | Removed (TODO) |
| `isSuccess()` | `spanToJSON(span).status === 'ok'` |
| `toTraceparent()` | `spanToTraceHeader(span)` |
| `toContext()` | Removed |
| `updateWithContext()` | Removed |
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/tracing/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,8 @@ export class Span implements SpanInterface {

/**
* @inheritDoc
*
* @deprecated Use `spanToJSON(span).status === 'ok'` instead.
*/
public isSuccess(): boolean {
return this._status === 'ok';
Expand Down
8 changes: 8 additions & 0 deletions packages/tracing/test/span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,21 +106,29 @@ describe('Span', () => {
expect(span.data['http.response.status_code']).toBe(404);
});

// TODO (v8): Remove
test('isSuccess', () => {
const span = new Span({});
expect(span.isSuccess()).toBe(false);
expect(spanToJSON(span).status).not.toBe('ok');
span.setHttpStatus(200);
expect(span.isSuccess()).toBe(true);
expect(spanToJSON(span).status).toBe('ok');
span.setStatus('permission_denied');
expect(span.isSuccess()).toBe(false);
expect(spanToJSON(span).status).not.toBe('ok');
span.setHttpStatus(0);
expect(span.isSuccess()).toBe(false);
expect(spanToJSON(span).status).not.toBe('ok');
span.setHttpStatus(-1);
expect(span.isSuccess()).toBe(false);
expect(spanToJSON(span).status).not.toBe('ok');
span.setHttpStatus(99);
expect(span.isSuccess()).toBe(false);
expect(spanToJSON(span).status).not.toBe('ok');
span.setHttpStatus(100);
expect(span.isSuccess()).toBe(true);
expect(spanToJSON(span).status).toBe('ok');
});
});

Expand Down
2 changes: 2 additions & 0 deletions packages/types/src/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ export interface Span extends SpanContext {

/**
* Determines whether span was successful (HTTP200)
*
* @deprecated Use `spanToJSON(span).status === 'ok'` instead.
*/
isSuccess(): boolean;

Expand Down

0 comments on commit f9ed885

Please sign in to comment.