ref: Use startTransaction where appropriate (#2644)

Follow up on #2626.

Now that startTransaction exists, replace usages of startSpan where the intention is to create a transaction.

Additionally, documentation updates and some refactoring to minimize type castings and linter flag usage.

Author: rhcarvalho

packages/apm/src/hubextensions.ts

@@ -51,18 +51,23 @@ function startTransaction(this: Hub, context: TransactionContext): Transaction {
 function startSpan(this: Hub, context: SpanContext): Transaction | Span {
   /**
    * @deprecated
-   * This is here to make sure we don't break users that relied on calling startSpan to create a transaction
-   * with the transaction poperty set.
+   * TODO: consider removing this in a future release.
+   *
+   * This is for backwards compatibility with releases before startTransaction
+   * existed, to allow for a smoother transition.
    */
-  if ((context as any).transaction !== undefined) {
-    logger.warn(`Use \`Sentry.startTransaction({name: ${(context as any).transaction}})\` to start a Transaction.`);
-    (context as TransactionContext).name = (context as any).transaction as string;
-  }
-
-  // We have the check of not undefined since we defined it's ok to start a transaction with an empty name
-  // tslint:disable-next-line: strict-type-predicates
-  if ((context as TransactionContext).name !== undefined) {
-    return this.startTransaction(context as TransactionContext);
+  {
+    // The `TransactionContext.name` field used to be called `transaction`.
+    const transactionContext = context as Partial<TransactionContext & { transaction: string }>;
+    if (transactionContext.transaction !== undefined) {
+      transactionContext.name = transactionContext.transaction;
+    }
+    // Check for not undefined since we defined it's ok to start a transaction
+    // with an empty name.
+    if (transactionContext.name !== undefined) {
+      logger.warn('Deprecated: Use startTransaction to start transactions and Transaction.startChild to start spans.');
+      return this.startTransaction(transactionContext as TransactionContext);
+    }
   }
 
   const scope = this.getScope();

packages/apm/src/integrations/tracing.ts

@@ -431,7 +431,7 @@ export class Tracing implements Integration {
       return undefined;
     }
 
-    Tracing._activeTransaction = hub.startSpan({
+    Tracing._activeTransaction = hub.startTransaction({
       trimEnd: true,
       ...transactionContext,
     }) as Transaction;

packages/apm/test/hub.test.ts

@@ -1,6 +1,5 @@
 import { BrowserClient } from '@sentry/browser';
 import { Hub, Scope } from '@sentry/hub';
-import { Span, Transaction } from '@sentry/types';
 
 import { addExtensionMethods } from '../src/hubextensions';
 
@@ -21,21 +20,18 @@ describe('Hub', () => {
       });
       test('set tracesSampleRate 0 on transaction', () => {
         const hub = new Hub(new BrowserClient({ tracesSampleRate: 0 }));
-        // @ts-ignore
-        const transaction = hub.startSpan({ name: 'foo' }) as any;
+        const transaction = hub.startTransaction({ name: 'foo' });
         expect(transaction.sampled).toBe(false);
       });
       test('set tracesSampleRate 1 on transaction', () => {
         const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
-        // @ts-ignore
-        const transaction = hub.startSpan({ name: 'foo' }) as any;
+        const transaction = hub.startTransaction({ name: 'foo' });
         expect(transaction.sampled).toBeTruthy();
       });
       test('set tracesSampleRate should be propergated to children', () => {
         const hub = new Hub(new BrowserClient({ tracesSampleRate: 0 }));
-        // @ts-ignore
-        const transaction = hub.startSpan({ name: 'foo' }) as any;
-        const child = transaction.startChild({ op: 1 });
+        const transaction = hub.startTransaction({ name: 'foo' });
+        const child = transaction.startChild({ op: 'test' });
         expect(child.sampled).toBeFalsy();
       });
     });
@@ -49,8 +45,7 @@ describe('Hub', () => {
 
       test('simple standalone Transaction', () => {
         const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
-        // @ts-ignore
-        const transaction = hub.startSpan({ name: 'transaction' }) as Transaction;
+        const transaction = hub.startTransaction({ name: 'transaction' });
         expect(transaction.spanId).toBeTruthy();
         // tslint:disable-next-line: no-unbound-method
         expect(transaction.setName).toBeTruthy();
@@ -71,8 +66,7 @@ describe('Hub', () => {
       test('create a child if there is a Span already on the scope', () => {
         const myScope = new Scope();
         const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }), myScope);
-        // @ts-ignore
-        const transaction = hub.startSpan({ name: 'transaction' }) as Transaction;
+        const transaction = hub.startTransaction({ name: 'transaction' });
         hub.configureScope(scope => {
           scope.setSpan(transaction);
         });

packages/minimal/src/index.ts

@@ -177,11 +177,21 @@ export function _callOnClient(method: string, ...args: any[]): void {
 }
 
 /**
- * Starts a Transaction. This is the entry point to do manual tracing. You can
- * add child spans to transactions. Spans themselves can have children, building
- * a tree structure. This function returns a Transaction and you need to keep
- * track of the instance yourself. When you call `.finish()` on the transaction
- * it will be sent to Sentry.
+ * Starts a new `Transaction` and returns it. This is the entry point to manual
+ * tracing instrumentation.
+ *
+ * A tree structure can be built by adding child spans to the transaction, and
+ * child spans to other spans. To start a new child span within the transaction
+ * or any span, call the respective `.startChild()` method.
+ *
+ * Every child span must be finished before the transaction is finished,
+ * otherwise the unfinished spans are discarded.
+ *
+ * The transaction must be finished with a call to its `.finish()` method, at
+ * which point the transaction with all its finished child spans will be sent to
+ * Sentry.
+ *
+ * @param context Properties of the new `Transaction`.
  */
 export function startTransaction(context: TransactionContext): Transaction {
   return callOnHub('startTransaction', { ...context });

packages/node/test/manual/apm-transaction/main.js

@@ -24,8 +24,8 @@ class Tracing {
         ? Span.fromTraceparent(req.headers['sentry-trace'], {
             transaction,
           })
-        : Sentry.getCurrentHub().startSpan({
-            transaction,
+        : Sentry.startTransaction({
+            name: transaction,
           });
 
       Sentry.getCurrentHub().configureScope(scope => {

packages/types/src/hub.ts

@@ -174,14 +174,15 @@ export interface Hub {
   traceHeaders(): { [key: string]: string };
 
   /**
-   * This function starts a span. If there is already a `Span` on the Scope,
-   * the created Span with the SpanContext will have a reference to it and become it's child.
-   * Otherwise it'll create a new `Span`.
+   * Starts a new `Span` and returns it. If there is a `Span` on the `Scope`,
+   * the new `Span` will be a child of the existing `Span`.
+   *
+   * @param context Properties of the new `Span`.
    */
   startSpan(context: SpanContext): Span;
 
   /**
-   * Starts a new transaction and returns it. This is the entry point to manual
+   * Starts a new `Transaction` and returns it. This is the entry point to manual
    * tracing instrumentation.
    *
    * A tree structure can be built by adding child spans to the transaction, and
@@ -194,6 +195,8 @@ export interface Hub {
    * The transaction must be finished with a call to its `.finish()` method, at
    * which point the transaction with all its finished child spans will be sent to
    * Sentry.
+   *
+   * @param context Properties of the new `Transaction`.
    */
   startTransaction(context: TransactionContext): Transaction;
 }