Skip to content

Commit

Permalink
feat(core): Deprecate Span.isSuccess in favor of reading span status
Browse files Browse the repository at this point in the history
  • Loading branch information
Lms24 committed Jan 17, 2024
1 parent e50cc8a commit 15d55ad
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 @@ -204,6 +204,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
- `span.instrumenter` This field was removed and will be replaced internally.
- `span.transaction`: Use `getRootSpan` utility function instead.
- `span.spanRecorder`: Span recording will be handled internally by the SDK.
- `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 @@ -442,6 +442,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 @@ -320,6 +320,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 15d55ad

Please sign in to comment.