ref: Remove getSpan and use scope.getTransaction

HazAT · HazAT · commit 9a9a3600d985 · 2020-06-15T20:41:14.000+02:00
diff --git a/packages/apm/src/hubextensions.ts b/packages/apm/src/hubextensions.ts
@@ -19,19 +19,6 @@ function traceHeaders(this: Hub): { [key: string]: string } {
   return {};
 }
 
-/**
- * {@see Hub.getSpan}
- */
-function getSpan(this: Hub, callback: (span: Span) => void): void {
-  const scope = this.getScope();
-  if (scope) {
-    const span = scope.getSpan();
-    if (span) {
-      callback(span);
-    }
-  }
-}
-
 /**
  * {@see Hub.startTransaction}
  */
@@ -109,9 +96,6 @@ export function addExtensionMethods(): void {
     if (!carrier.__SENTRY__.extensions.startSpan) {
       carrier.__SENTRY__.extensions.startSpan = startSpan;
     }
-    if (!carrier.__SENTRY__.extensions.getSpan) {
-      carrier.__SENTRY__.extensions.getSpan = getSpan;
-    }
     if (!carrier.__SENTRY__.extensions.traceHeaders) {
       carrier.__SENTRY__.extensions.traceHeaders = traceHeaders;
     }
diff --git a/packages/apm/src/span.ts b/packages/apm/src/span.ts
@@ -156,14 +156,14 @@ export class Span implements SpanInterface, SpanContext {
   /**
    * @inheritDoc
    */
-  public getTransaction(): Transaction {
+  public getTransaction(callback: (transaction: Transaction) => void): void {
     const recorder = this.spanRecorder;
-    if (!recorder) {
+    if (!recorder || !recorder.spans[0]) {
       logger.warn('This Span has no reference to a Transaction. Returning an instance to itself.');
       logger.warn('It means that the Transaction has been sampled or the Span did not originate from a Transaction.');
-      return (this as unknown) as Transaction;
+      return;
     }
-    return recorder.spans[0] as Transaction;
+    callback(recorder.spans[0] as Transaction);
   }
 
   /**
diff --git a/packages/apm/test/hub.test.ts b/packages/apm/test/hub.test.ts
@@ -18,16 +18,20 @@ describe('Hub', () => {
       hub.configureScope(scope => {
         scope.setSpan(transaction);
       });
-      hub.getSpan(t => {
-        expect(t).toBe(transaction);
+      hub.configureScope(s => {
+        s.getTransaction(t => {
+          expect(t).toBe(transaction);
+        });
       });
     });
 
     test('not invoke', () => {
       const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
       const transaction = hub.startTransaction({ name: 'foo' });
-      hub.getSpan(_ => {
-        expect(true).toBe(false);
+      hub.configureScope(s => {
+        s.getTransaction(_ => {
+          expect(true).toBe(false);
+        });
       });
       transaction.finish();
     });
diff --git a/packages/apm/test/span.test.ts b/packages/apm/test/span.test.ts
@@ -366,19 +366,6 @@ describe('Span', () => {
     });
   });
 
-  test('getTransaction on Span from Transaction', () => {
-    const transaction = hub.startTransaction({ name: 'test' });
-    const childSpan = transaction.startChild();
-    childSpan.finish();
-    transaction.finish();
-    expect(childSpan.getTransaction()).toBe(transaction);
-  });
-
-  test('getTransaction on new Span()', () => {
-    const span = new Span({});
-    expect(span.getTransaction()).toBe(span);
-  });
-
   describe('getTraceContext', () => {
     test('should have status attribute undefined if no status tag is available', () => {
       const span = new Span({});
diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts
@@ -380,13 +380,6 @@ export class Hub implements HubInterface {
     return this._callExtensionMethod('startTransaction', context);
   }
 
-  /**
-   * @inheritDoc
-   */
-  public getSpan(callback: (span: Span) => void): void {
-    this._callExtensionMethod<void>('getSpan', callback);
-  }
-
   /**
    * @inheritDoc
    */
diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts
@@ -8,6 +8,7 @@ import {
   ScopeContext,
   Severity,
   Span,
+  Transaction,
   User,
 } from '@sentry/types';
 import { getGlobalObject, isPlainObject, isThenable, SyncPromise, timestampWithMs } from '@sentry/utils';
@@ -217,6 +218,18 @@ export class Scope implements ScopeInterface {
     return this._span;
   }
 
+  /**
+   * @inheritDoc
+   */
+  public getTransaction(callback: (transaction: Transaction) => void): void {
+    const span = this.getSpan() as Span & { spanRecorder: { spans: Span[] } };
+    if (span) {
+      if (span.spanRecorder && span.spanRecorder.spans[0]) {
+        callback(span.spanRecorder.spans[0] as Transaction);
+      }
+    }
+  }
+
   /**
    * Inherit values from the parent scope.
    * @param scope to clone.
diff --git a/packages/minimal/src/index.ts b/packages/minimal/src/index.ts
@@ -1,14 +1,5 @@
 import { getCurrentHub, Hub, Scope } from '@sentry/hub';
-import {
-  Breadcrumb,
-  CaptureContext,
-  Event,
-  Severity,
-  Span,
-  Transaction,
-  TransactionContext,
-  User,
-} from '@sentry/types';
+import { Breadcrumb, CaptureContext, Event, Severity, Transaction, TransactionContext, User } from '@sentry/types';
 
 /**
  * This calls a function on the current hub.
@@ -205,11 +196,3 @@ export function _callOnClient(method: string, ...args: any[]): void {
 export function startTransaction(context: TransactionContext): Transaction {
   return callOnHub('startTransaction', { ...context });
 }
-
-/**
- * Callback that receives a Span if there is one on the Scope.
- * @param callback Will only be invoked in case there is a Span on the Scope
- */
-export function getSpan(callback: (span: Span) => void): void {
-  callOnHub<void>('getSpan', callback);
-}
diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -79,17 +79,14 @@ function createHandlerWrapper(
       }
 
       let span: Span | undefined;
-      let transaction: Transaction | undefined;
 
       const scope = getCurrentHub().getScope();
-      if (scope) {
-        transaction = scope.getSpan() as Transaction;
-      }
-
-      if (tracingEnabled && transaction) {
-        span = transaction.startChild({
-          description: `${typeof options === 'string' || !options.method ? 'GET' : options.method} ${requestUrl}`,
-          op: 'request',
+      if (scope && tracingEnabled) {
+        scope.getTransaction(transaction => {
+          span = transaction.startChild({
+            description: `${typeof options === 'string' || !options.method ? 'GET' : options.method} ${requestUrl}`,
+            op: 'request',
+          });
         });
       }
 
diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts
@@ -199,10 +199,4 @@ export interface Hub {
    * @param context Properties of the new `Transaction`.
    */
   startTransaction(context: TransactionContext): Transaction;
-
-  /**
-   * Callback that receives a Span if there is one on the Scope.
-   * @param callback Will only be invoked in case there is a Span on the Scope
-   */
-  getSpan(callback: (span: Span) => void): void;
 }
diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts
@@ -2,6 +2,7 @@ import { Breadcrumb } from './breadcrumb';
 import { EventProcessor } from './eventprocessor';
 import { Severity } from './severity';
 import { Span } from './span';
+import { Transaction } from './transaction';
 import { User } from './user';
 
 /** JSDocs */
@@ -89,6 +90,17 @@ export interface Scope {
    */
   setSpan(span?: Span): this;
 
+  /**
+   * Returns the current set span if there is one
+   */
+  getSpan(): Span | undefined;
+
+  /**
+   * Callback to retrieve an ongoing Transaction in case there is one.
+   * @param callback Will only be invoked in case there is an active transaction
+   */
+  getTransaction(callback: (transaction: Transaction) => void): void;
+
   /**
    * Updates the scope with provided data. Can work in three variations:
    * - plain object containing updatable attributes
diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts
@@ -136,9 +136,10 @@ export interface Span extends SpanContext {
   ): Span;
 
   /**
-   * Retruns the reference to the root Span (Transaction).
+   * Callback to retrieve the root Span (Transaction) in case there is one.
+   * @param callback Will only be invoked in case there is an active transaction
    */
-  getTransaction(): Transaction;
+  getTransaction(callback: (transaction: Transaction) => void): void;
 
   /**
    * Determines whether span was successful (HTTP200)
