Skip to content

Commit

Permalink
ref: Use startTransaction where appropriate (#2644)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rhcarvalho committed Jun 8, 2020
1 parent fc53353 commit 5b6ea3d
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 35 deletions.
27 changes: 16 additions & 11 deletions packages/apm/src/hubextensions.ts
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/apm/src/integrations/tracing.ts
Expand Up @@ -431,7 +431,7 @@ export class Tracing implements Integration {
return undefined;
}

Tracing._activeTransaction = hub.startSpan({
Tracing._activeTransaction = hub.startTransaction({
trimEnd: true,
...transactionContext,
}) as Transaction;
Expand Down
18 changes: 6 additions & 12 deletions 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';

Expand All @@ -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();
});
});
Expand All @@ -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();
Expand All @@ -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);
});
Expand Down
20 changes: 15 additions & 5 deletions packages/minimal/src/index.ts
Expand Up @@ -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 });
Expand Down
4 changes: 2 additions & 2 deletions packages/node/test/manual/apm-transaction/main.js
Expand Up @@ -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 => {
Expand Down
11 changes: 7 additions & 4 deletions packages/types/src/hub.ts
Expand Up @@ -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
Expand All @@ -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;
}

0 comments on commit 5b6ea3d

Please sign in to comment.