Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hub.startTransaction take 2 #2644

Merged
merged 3 commits into from Jun 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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);
{
rhcarvalho marked this conversation as resolved.
Show resolved Hide resolved
// 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
rhcarvalho marked this conversation as resolved.
Show resolved Hide resolved
// 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;
}