From f9ed8855b3928f26ff6907da7551e2e3f33785ee Mon Sep 17 00:00:00 2001
From: Lukas Stracke <lukas.stracke@sentry.io>
Date: Wed, 17 Jan 2024 18:16:50 +0100
Subject: [PATCH] feat(core): Deprecate `Span.isSuccess()` in favor of reading
 span status (#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.
---
 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 b5b6d4da060b..df21637dc6a4 100644
--- a/MIGRATION.md
+++ b/MIGRATION.md
@@ -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.
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 82f93ff6a38e..6938ef00e47a 100644
--- a/packages/core/src/tracing/span.ts
+++ b/packages/core/src/tracing/span.ts
@@ -472,6 +472,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 873c139e41c9..1cf33c7c435f 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 cc11df4cab83..993dcd66b2a1 100644
--- a/packages/types/src/span.ts
+++ b/packages/types/src/span.ts
@@ -329,6 +329,8 @@ export interface Span extends SpanContext {
 
   /**
    * Determines whether span was successful (HTTP200)
+   *
+   * @deprecated Use `spanToJSON(span).status === 'ok'` instead.
    */
   isSuccess(): boolean;