From 15d55ade3b190b3c65044d492345644ee0ba863c Mon Sep 17 00:00:00 2001
From: Lukas Stracke <lukas.stracke@sentry.io>
Date: Wed, 17 Jan 2024 12:46:41 +0100
Subject: [PATCH] feat(core): Deprecate `Span.isSuccess` in favor of reading
 span status

---
 MIGRATION.md                       | 1 +
 docs/v8-new-performance-apis.md    | 2 +-
 packages/core/src/tracing/span.ts  | 2 ++
 packages/tracing/test/span.test.ts | 8 ++++++++
 packages/types/src/span.ts         | 2 ++
 5 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/MIGRATION.md b/MIGRATION.md
index 7927dd6b49a0..680d27fb397a 100644
--- a/MIGRATION.md
+++ b/MIGRATION.md
@@ -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.
diff --git a/docs/v8-new-performance-apis.md b/docs/v8-new-performance-apis.md
index 2f531858dd2c..2f258ec386cb 100644
--- a/docs/v8-new-performance-apis.md
+++ b/docs/v8-new-performance-apis.md
@@ -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                                              |
diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts
index 2d6037aff9f3..beee1fefc2f2 100644
--- a/packages/core/src/tracing/span.ts
+++ b/packages/core/src/tracing/span.ts
@@ -442,6 +442,8 @@ export class Span implements SpanInterface {
 
   /**
    * @inheritDoc
+   *
+   * @deprecated Use `spanToJSON(span).status === 'ok'` instead.
    */
   public isSuccess(): boolean {
     return this.status === 'ok';
diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts
index 1e6ade7d4dec..194e767df565 100644
--- a/packages/tracing/test/span.test.ts
+++ b/packages/tracing/test/span.test.ts
@@ -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');
     });
   });
 
diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts
index 5bbe6408ea30..939d1fb2038e 100644
--- a/packages/types/src/span.ts
+++ b/packages/types/src/span.ts
@@ -320,6 +320,8 @@ export interface Span extends SpanContext {
 
   /**
    * Determines whether span was successful (HTTP200)
+   *
+   * @deprecated Use `spanToJSON(span).status === 'ok'` instead.
    */
   isSuccess(): boolean;