diff --git a/packages/apm/src/hubextensions.ts b/packages/apm/src/hubextensions.ts index 9d1f3fb4847d..b558d1f3cd56 100644 --- a/packages/apm/src/hubextensions.ts +++ b/packages/apm/src/hubextensions.ts @@ -1,4 +1,4 @@ -import { getMainCarrier, Hub } from '@sentry/hub'; +import { getCurrentHub, getMainCarrier, Hub } from '@sentry/hub'; import { SpanContext, TransactionContext } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -20,44 +20,49 @@ function traceHeaders(this: Hub): { [key: string]: string } { } /** - * This functions 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 crete a new `Span`. - * - * @param context Properties with which the span should be created + * {@see Hub.startTransaction} */ -function startSpan(this: Hub, context: SpanContext | TransactionContext): Transaction | Span { - // This is our safeguard so people always get a Transaction in return. - // We set `_isTransaction: true` in {@link Sentry.startTransaction} to have a runtime check - // if the user really wanted to create a Transaction. - if ((context as TransactionContext)._isTransaction && !(context as TransactionContext).name) { - logger.warn('You are trying to start a Transaction but forgot to provide a `name` property.'); - logger.warn('Will fall back to , use `transaction.setName()` to change it.'); - (context as TransactionContext).name = ''; +function startTransaction(this: Hub, context: TransactionContext): Transaction { + const transaction = new Transaction(context, this); + + const client = this.getClient(); + // Roll the dice for sampling transaction, all child spans inherit the sampling decision. + if (transaction.sampled === undefined) { + const sampleRate = (client && client.getOptions().tracesSampleRate) || 0; + // if true = we want to have the transaction + // if false = we don't want to have it + // Math.random (inclusive of 0, but not 1) + transaction.sampled = Math.random() < sampleRate; } - if ((context as TransactionContext).name) { - // We are dealing with a Transaction - const transaction = new Transaction(context as TransactionContext, this); + // We only want to create a span list if we sampled the transaction + // If sampled == false, we will discard the span anyway, so we can save memory by not storing child spans + if (transaction.sampled) { + const experimentsOptions = (client && client.getOptions()._experiments) || {}; + transaction.initSpanRecorder(experimentsOptions.maxSpans as number); + } - const client = this.getClient(); - // We only roll the dice on sampling for root spans of transactions because all child spans inherit this state - if (transaction.sampled === undefined) { - const sampleRate = (client && client.getOptions().tracesSampleRate) || 0; - // if true = we want to have the transaction - // if false = we don't want to have it - // Math.random (inclusive of 0, but not 1) - transaction.sampled = Math.random() < sampleRate; - } + return transaction; +} - // We only want to create a span list if we sampled the transaction - // If sampled == false, we will discard the span anyway, so we can save memory by not storing child spans - if (transaction.sampled) { - const experimentsOptions = (client && client.getOptions()._experiments) || {}; - transaction.initSpanRecorder(experimentsOptions.maxSpans as number); - } +/** + * {@see Hub.startSpan} + */ +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. + */ + 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; + } - return transaction; + // 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 getCurrentHub().startTransaction(context as TransactionContext); } const scope = this.getScope(); @@ -80,6 +85,9 @@ export function addExtensionMethods(): void { const carrier = getMainCarrier(); if (carrier.__SENTRY__) { carrier.__SENTRY__.extensions = carrier.__SENTRY__.extensions || {}; + if (!carrier.__SENTRY__.extensions.startTransaction) { + carrier.__SENTRY__.extensions.startTransaction = startTransaction; + } if (!carrier.__SENTRY__.extensions.startSpan) { carrier.__SENTRY__.extensions.startSpan = startSpan; } diff --git a/packages/apm/src/span.ts b/packages/apm/src/span.ts index efa280b0218b..eb6c49d42bab 100644 --- a/packages/apm/src/span.ts +++ b/packages/apm/src/span.ts @@ -77,7 +77,7 @@ export class Span implements SpanInterface, SpanContext { public spanRecorder?: SpanRecorder; /** - * You should never call the custructor manually, always use `hub.startSpan()`. + * You should never call the constructor manually, always use `hub.startSpan()`. * @internal * @hideconstructor * @hidden diff --git a/packages/apm/src/transaction.ts b/packages/apm/src/transaction.ts index fb3cdc63589f..9366fca34613 100644 --- a/packages/apm/src/transaction.ts +++ b/packages/apm/src/transaction.ts @@ -46,7 +46,8 @@ export class Transaction extends SpanClass { private readonly _trimEnd?: boolean; /** - * This constructor should never be called manually. Those instrumenting tracing should use `Stentry.startTransaction()`, and internal methods should use `hub.startSpan()`. + * This constructor should never be called manually. Those instrumenting tracing should use + * `Sentry.startTransaction()`, and internal methods should use `hub.startTransaction()`. * @internal * @hideconstructor * @hidden @@ -92,6 +93,11 @@ export class Transaction extends SpanClass { return undefined; } + if (!this.name) { + logger.warn('Transaction has no name, falling back to ``.'); + this.name = ''; + } + super.finish(endTimestamp); if (this.sampled !== true) { diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index 32f82e8c25a6..907a6d6f9df9 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -368,10 +368,17 @@ export class Hub implements HubInterface { /** * @inheritDoc */ - public startSpan(context: SpanContext | TransactionContext): Transaction | Span { + public startSpan(context: SpanContext): Span { return this._callExtensionMethod('startSpan', context); } + /** + * @inheritDoc + */ + public startTransaction(context: TransactionContext): Transaction { + return this._callExtensionMethod('startTransaction', context); + } + /** * @inheritDoc */ diff --git a/packages/minimal/src/index.ts b/packages/minimal/src/index.ts index 6372d46d16d3..b9aae8b81149 100644 --- a/packages/minimal/src/index.ts +++ b/packages/minimal/src/index.ts @@ -169,14 +169,12 @@ export function _callOnClient(method: string, ...args: any[]): void { } /** - * This starts a Transaction and is considered the entry point to do manual tracing. You can add child spans - * to a transactions. After that more children can be added to created spans to buld a tree structure. - * This function returns a Transaction and you need to keep track of the instance yourself. When you call `.finsh()` on - * a transaction it will be sent to Sentry. + * 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. */ -export function startTransaction(transactionContext: TransactionContext): Transaction { - return callOnHub('startSpan', { - ...transactionContext, - _isTransaction: true, - }); +export function startTransaction(context: TransactionContext): Transaction { + return callOnHub('startTransaction', { ...context }); } diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts index 1db098d89c46..8fef4ff47d0a 100644 --- a/packages/types/src/hub.ts +++ b/packages/types/src/hub.ts @@ -174,10 +174,26 @@ export interface Hub { traceHeaders(): { [key: string]: string }; /** - * This functions starts either a Span or a Transaction (depending on the argument passed). - * If there is a Span on the Scope we use the `trace_id` for all other created Transactions / Spans as a reference. + * 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`. + */ + startSpan(context: SpanContext): Span; + + /** + * 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. * - * @param context Properties with which the Transaction/Span should be created + * 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. */ - startSpan(context: SpanContext | TransactionContext): Transaction | Span; + startTransaction(context: TransactionContext): Transaction; } diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index 3f0b91da3bf8..9dbdfb3e523d 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -12,13 +12,6 @@ export interface TransactionContext extends SpanContext { * transaction after a given "idle time" and we don't want this "idle time" to be part of the transaction. */ trimEnd?: boolean; - - /** - * This flag is internally used by {@link Sentry.startTransaction} and set there to determine this is really a - * Transaction. Since there is no type safety in JS we set it in the top level function to true to check later - * if the user really wanted to create a Transaction and we can log a warning if they forgot to set the name property. - */ - _isTransaction?: boolean; } /**