feat: Add scope.getTransaction, rename scope.setTransaction -> setTra…

…nsactionName (#2668) * feat: Add Sentry.getTransaction * fix: Linter * ref: Introduce Sentry.getSpan * ref: Remove getSpan and use scope.getTransaction * fix: Linter * ref: Rename setTransaction * Update CHANGELOG.md Co-authored-by: Abhijeet Prasad <AbhiPrasad@users.noreply.github.com> * ref: CodeReview * fix: Node * ref: CR * ref: Remove unused code * ref: Code Review * ref: Revert tests Co-authored-by: Abhijeet Prasad <AbhiPrasad@users.noreply.github.com>
getsentry · Jun 22, 2020 · eb4846e · eb4846e
1 parent 7f74693
commit eb4846e
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,7 +9,9 @@
 - [core] feat: Export `makeMain` (#2665)
 - [core] fix: Call `bindClient` when creating new `Hub` to make integrations work automatically (#2665)
 - [gatsby] feat: Add @sentry/gatsby package (#2652)
-- [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls`
+- [tracing] feat: Add `scope.getTransaction` to return a Transaction if it exists (#2668)
+- [tracing] ref: Deprecate `scope.setTransaction` in favor of `scope.setTransactionName` (#2668)
+- [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls` (#2671)
 - [react] ref: Refactor Profiler to account for update and render (#2677)
 - [apm] feat: Add ability to get span from activity using `getActivitySpan` (#2677)
 - [apm] fix: Check if `performance.mark` exists before calling it (#2680)

diff --git a/packages/apm/test/hub.test.ts b/packages/apm/test/hub.test.ts
@@ -11,6 +11,28 @@ describe('Hub', () => {
     jest.useRealTimers();
   });
 
+  describe('getTransaction', () => {
+    test('simple invoke', () => {
+      const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
+      const transaction = hub.startTransaction({ name: 'foo' });
+      hub.configureScope(scope => {
+        scope.setSpan(transaction);
+      });
+      hub.configureScope(s => {
+        expect(s.getTransaction()).toBe(transaction);
+      });
+    });
+
+    test('not invoke', () => {
+      const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
+      const transaction = hub.startTransaction({ name: 'foo' });
+      hub.configureScope(s => {
+        expect(s.getTransaction()).toBeUndefined();
+      });
+      transaction.finish();
+    });
+  });
+
   describe('spans', () => {
     describe('sampling', () => {
       test('set tracesSampleRate 0 on span', () => {

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';
@@ -47,8 +48,8 @@ export class Scope implements ScopeInterface {
   /** Severity */
   protected _level?: Severity;
 
-  /** Transaction */
-  protected _transaction?: string;
+  /** Transaction Name */
+  protected _transactionName?: string;
 
   /** Span */
   protected _span?: Span;
@@ -185,12 +186,20 @@ export class Scope implements ScopeInterface {
   /**
    * @inheritDoc
    */
-  public setTransaction(transaction?: string): this {
-    this._transaction = transaction;
+  public setTransactionName(name?: string): this {
+    this._transactionName = name;
     this._notifyScopeListeners();
     return this;
   }
 
+  /**
+   * Can be removed in major version.
+   * @deprecated in favor of {@link this.setTransactionName}
+   */
+  public setTransaction(name?: string): this {
+    return this.setTransactionName(name);
+  }
+
   /**
    * @inheritDoc
    */
@@ -210,13 +219,23 @@ export class Scope implements ScopeInterface {
   }
 
   /**
-   * Internal getter for Span, used in Hub.
-   * @hidden
+   * @inheritDoc
    */
   public getSpan(): Span | undefined {
     return this._span;
   }
 
+  /**
+   * @inheritDoc
+   */
+  public getTransaction(): Transaction | undefined {
+    const span = this.getSpan() as Span & { spanRecorder: { spans: Span[] } };
+    if (span && span.spanRecorder && span.spanRecorder.spans[0]) {
+      return span.spanRecorder.spans[0] as Transaction;
+    }
+    return undefined;
+  }
+
   /**
    * Inherit values from the parent scope.
    * @param scope to clone.
@@ -231,7 +250,7 @@ export class Scope implements ScopeInterface {
       newScope._user = scope._user;
       newScope._level = scope._level;
       newScope._span = scope._span;
-      newScope._transaction = scope._transaction;
+      newScope._transactionName = scope._transactionName;
       newScope._fingerprint = scope._fingerprint;
       newScope._eventProcessors = [...scope._eventProcessors];
     }
@@ -294,7 +313,7 @@ export class Scope implements ScopeInterface {
     this._user = {};
     this._contexts = {};
     this._level = undefined;
-    this._transaction = undefined;
+    this._transactionName = undefined;
     this._fingerprint = undefined;
     this._span = undefined;
     this._notifyScopeListeners();
@@ -374,8 +393,8 @@ export class Scope implements ScopeInterface {
     if (this._level) {
       event.level = this._level;
     }
-    if (this._transaction) {
-      event.transaction = this._transaction;
+    if (this._transactionName) {
+      event.transaction = this._transactionName;
     }
     // We want to set the trace context for normal events only if there isn't already
     // a trace context on the event. There is a product feature in place where we link

diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts
@@ -73,17 +73,17 @@ describe('Scope', () => {
       expect((scope as any)._level).toEqual(Severity.Critical);
     });
 
-    test('setTransaction', () => {
+    test('setTransactionName', () => {
       const scope = new Scope();
-      scope.setTransaction('/abc');
-      expect((scope as any)._transaction).toEqual('/abc');
+      scope.setTransactionName('/abc');
+      expect((scope as any)._transactionName).toEqual('/abc');
     });
 
-    test('setTransaction with no value unsets it', () => {
+    test('setTransactionName with no value unsets it', () => {
       const scope = new Scope();
-      scope.setTransaction('/abc');
-      scope.setTransaction();
-      expect((scope as any)._transaction).toBeUndefined();
+      scope.setTransactionName('/abc');
+      scope.setTransactionName();
+      expect((scope as any)._transactionName).toBeUndefined();
     });
 
     test('setContext', () => {
@@ -157,7 +157,7 @@ describe('Scope', () => {
       scope.setUser({ id: '1' });
       scope.setFingerprint(['abcd']);
       scope.setLevel(Severity.Warning);
-      scope.setTransaction('/abc');
+      scope.setTransactionName('/abc');
       scope.addBreadcrumb({ message: 'test' }, 100);
       scope.setContext('os', { id: '1' });
       const event: Event = {};
@@ -256,7 +256,7 @@ describe('Scope', () => {
     test('scope transaction should have priority over event transaction', () => {
       expect.assertions(1);
       const scope = new Scope();
-      scope.setTransaction('/abc');
+      scope.setTransactionName('/abc');
       const event: Event = {};
       event.transaction = '/cdf';
       return scope.applyToEvent(event).then(processedEvent => {

diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -82,15 +82,14 @@ function createHandlerWrapper(
       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) {
+        transaction = scope.getTransaction();
+        if (transaction) {
+          span = transaction.startChild({
+            description: `${typeof options === 'string' || !options.method ? 'GET' : options.method} ${requestUrl}`,
+            op: 'request',
+          });
+        }
       }
 
       return originalHandler

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 */
@@ -71,10 +72,9 @@ export interface Scope {
   setLevel(level: Severity): this;
 
   /**
-   * Sets the transaction on the scope for future events.
-   * @param transaction string This will be converted in a tag in Sentry
+   * Sets the transaction name on the scope for future events.
    */
-  setTransaction(transaction?: string): this;
+  setTransactionName(name?: string): this;
 
   /**
    * Sets context data with the given name.
@@ -89,6 +89,16 @@ export interface Scope {
    */
   setSpan(span?: Span): this;
 
+  /**
+   * Returns the `Span` if there is one
+   */
+  getSpan(): Span | undefined;
+
+  /**
+   * Returns the `Transaction` if there is one
+   */
+  getTransaction(): Transaction | undefined;
+
   /**
    * Updates the scope with provided data. Can work in three variations:
    * - plain object containing updatable attributes