diff --git a/CHANGELOG.md b/CHANGELOG.md index ff3fae005444..697a88ecbb52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ - [core] fix: sent_at for envelope headers to use same clock #2597 - [apm] fix: Improve bundle size by moving span status to @sentry/apm #2589 - [apm] feat: No longer discard transactions instead mark them deadline exceeded #2588 +- [apm] feat: Introduce `Sentry.startTransaction` and `Transaction.startChild` #2600 +- [apm] feat: Transactions no longer go through `beforeSend` #2600 - [browser] fix: Emit Sentry Request breadcrumbs from inside the client (#2615) ## 5.15.5 diff --git a/packages/apm/src/hubextensions.ts b/packages/apm/src/hubextensions.ts index 8f128610113c..9d1f3fb4847d 100644 --- a/packages/apm/src/hubextensions.ts +++ b/packages/apm/src/hubextensions.ts @@ -1,13 +1,13 @@ import { getMainCarrier, Hub } from '@sentry/hub'; -import { SpanContext } from '@sentry/types'; +import { SpanContext, TransactionContext } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { Span } from './span'; +import { Transaction } from './transaction'; /** Returns all trace headers that are currently on the top scope. */ -function traceHeaders(): { [key: string]: string } { - // @ts-ignore - const that = this as Hub; - const scope = that.getScope(); +function traceHeaders(this: Hub): { [key: string]: string } { + const scope = this.getScope(); if (scope) { const span = scope.getSpan(); if (span) { @@ -24,45 +24,53 @@ function traceHeaders(): { [key: string]: string } { * 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 spanContext Properties with which the span should be created + * @param context Properties with which the span should be created */ -function startSpan(spanContext?: SpanContext): Span { - // @ts-ignore - const hub = this as Hub; - const scope = hub.getScope(); - const client = hub.getClient(); - let span; +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 = ''; + } - // This flag determines if we already added the span as a child to the span that currently lives on the scope - // If we do not have this, we will add it later on twice to the span recorder and therefore have too many spans - let addedAsChild = false; + if ((context as TransactionContext).name) { + // We are dealing with a Transaction + const transaction = new Transaction(context as TransactionContext, this); - if (scope) { - const parentSpan = scope.getSpan() as Span; - if (parentSpan) { - span = parentSpan.child(spanContext); - addedAsChild = true; + 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; } - } - if (!span) { - span = new Span(spanContext, hub); - } + // 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); + } - // We only roll the dice on sampling for "root" spans (transactions) because the childs inherit this state - if (span.sampled === undefined && span.isRootSpan()) { - const sampleRate = (client && client.getOptions().tracesSampleRate) || 0; - span.sampled = Math.random() < sampleRate; + return transaction; } - // We only want to create a span list if we sampled the transaction - // in case we will discard the span anyway because sampled == false, we safe memory and do not store child spans - if (span.sampled && !addedAsChild) { - const experimentsOptions = (client && client.getOptions()._experiments) || {}; - span.initSpanRecorder(experimentsOptions.maxSpans as number); + const scope = this.getScope(); + if (scope) { + // If there is a Span on the Scope we start a child and return that instead + const parentSpan = scope.getSpan(); + if (parentSpan) { + return parentSpan.startChild(context); + } } - return span; + // Otherwise we return a new Span + return new Span(context); } /** diff --git a/packages/apm/src/index.ts b/packages/apm/src/index.ts index 3e021fb5d210..9f59e3515f40 100644 --- a/packages/apm/src/index.ts +++ b/packages/apm/src/index.ts @@ -3,6 +3,7 @@ import * as ApmIntegrations from './integrations'; export { ApmIntegrations as Integrations }; export { Span, TRACEPARENT_REGEXP } from './span'; +export { Transaction } from './transaction'; export { SpanStatus } from './spanstatus'; diff --git a/packages/apm/src/integrations/express.ts b/packages/apm/src/integrations/express.ts index 1caa3f681390..113d41423dcd 100644 --- a/packages/apm/src/integrations/express.ts +++ b/packages/apm/src/integrations/express.ts @@ -1,8 +1,16 @@ -import { EventProcessor, Hub, Integration } from '@sentry/types'; +import { Integration, Transaction } from '@sentry/types'; import { logger } from '@sentry/utils'; // tslint:disable-next-line:no-implicit-dependencies import { Application, ErrorRequestHandler, NextFunction, Request, RequestHandler, Response } from 'express'; +/** + * Internal helper for `__sentry_transaction` + * @hidden + */ +interface SentryTracingResponse { + __sentry_transaction?: Transaction; +} + /** * Express integration * @@ -35,12 +43,12 @@ export class Express implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + public setupOnce(): void { if (!this._app) { logger.error('ExpressIntegration is missing an Express instance'); return; } - instrumentMiddlewares(this._app, getCurrentHub); + instrumentMiddlewares(this._app); } } @@ -56,40 +64,66 @@ export class Express implements Integration { * // error handler * app.use(function (err, req, res, next) { ... }) */ -function wrap(fn: Function, getCurrentHub: () => Hub): RequestHandler | ErrorRequestHandler { +function wrap(fn: Function): RequestHandler | ErrorRequestHandler { const arrity = fn.length; switch (arrity) { case 2: { - return function(this: NodeJS.Global, _req: Request, res: Response): any { - const span = getCurrentHub().startSpan({ - description: fn.name, - op: 'middleware', - }); - res.once('finish', () => span.finish()); + return function(this: NodeJS.Global, _req: Request, res: Response & SentryTracingResponse): any { + const transaction = res.__sentry_transaction; + if (transaction) { + const span = transaction.startChild({ + description: fn.name, + op: 'middleware', + }); + res.once('finish', () => { + span.finish(); + }); + } return fn.apply(this, arguments); }; } case 3: { - return function(this: NodeJS.Global, req: Request, res: Response, next: NextFunction): any { - const span = getCurrentHub().startSpan({ - description: fn.name, - op: 'middleware', - }); + return function( + this: NodeJS.Global, + req: Request, + res: Response & SentryTracingResponse, + next: NextFunction, + ): any { + const transaction = res.__sentry_transaction; + const span = + transaction && + transaction.startChild({ + description: fn.name, + op: 'middleware', + }); fn.call(this, req, res, function(this: NodeJS.Global): any { - span.finish(); + if (span) { + span.finish(); + } return next.apply(this, arguments); }); }; } case 4: { - return function(this: NodeJS.Global, err: any, req: Request, res: Response, next: NextFunction): any { - const span = getCurrentHub().startSpan({ - description: fn.name, - op: 'middleware', - }); + return function( + this: NodeJS.Global, + err: any, + req: Request, + res: Response & SentryTracingResponse, + next: NextFunction, + ): any { + const transaction = res.__sentry_transaction; + const span = + transaction && + transaction.startChild({ + description: fn.name, + op: 'middleware', + }); fn.call(this, err, req, res, function(this: NodeJS.Global): any { - span.finish(); + if (span) { + span.finish(); + } return next.apply(this, arguments); }); }; @@ -110,16 +144,16 @@ function wrap(fn: Function, getCurrentHub: () => Hub): RequestHandler | ErrorReq * app.use([], , ...) * app.use([], ...[]) */ -function wrapUseArgs(args: IArguments, getCurrentHub: () => Hub): unknown[] { +function wrapUseArgs(args: IArguments): unknown[] { return Array.from(args).map((arg: unknown) => { if (typeof arg === 'function') { - return wrap(arg, getCurrentHub); + return wrap(arg); } if (Array.isArray(arg)) { return arg.map((a: unknown) => { if (typeof a === 'function') { - return wrap(a, getCurrentHub); + return wrap(a); } return a; }); @@ -132,10 +166,10 @@ function wrapUseArgs(args: IArguments, getCurrentHub: () => Hub): unknown[] { /** * Patches original app.use to utilize our tracing functionality */ -function instrumentMiddlewares(app: Application, getCurrentHub: () => Hub): Application { +function instrumentMiddlewares(app: Application): Application { const originalAppUse = app.use; app.use = function(): any { - return originalAppUse.apply(this, wrapUseArgs(arguments, getCurrentHub)); + return originalAppUse.apply(this, wrapUseArgs(arguments)); }; return app; } diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index f0948f1b3eb9..61a34f969aa8 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -1,5 +1,5 @@ -import { Hub, Scope } from '@sentry/hub'; -import { Event, EventProcessor, Integration, Severity, Span, SpanContext } from '@sentry/types'; +import { Hub } from '@sentry/hub'; +import { Event, EventProcessor, Integration, Severity, Span, SpanContext, TransactionContext } from '@sentry/types'; import { addInstrumentationHandler, getGlobalObject, @@ -11,6 +11,7 @@ import { import { Span as SpanClass } from '../span'; import { SpanStatus } from '../spanstatus'; +import { Transaction } from '../transaction'; /** * Options for Tracing integration @@ -70,14 +71,13 @@ interface TracingOptions { maxTransactionDuration: number; /** - * Flag to discard all spans that occur in background. This includes transactions. Browser background tab timing is - * not suited towards doing precise measurements of operations. That's why this option discards any active transaction - * and also doesn't add any spans that happen in the background. Background spans/transaction can mess up your + * Flag Transactions where tabs moved to background with "cancelled". Browser background tab timing is + * not suited towards doing precise measurements of operations. Background transaction can mess up your * statistics in non deterministic ways that's why we by default recommend leaving this opition enabled. * * Default: true */ - discardBackgroundSpans: boolean; + markBackgroundTransactions: boolean; /** * This is only if you want to debug in prod. @@ -132,7 +132,7 @@ export class Tracing implements Integration { */ private static _getCurrentHub?: () => Hub; - private static _activeTransaction?: Span; + private static _activeTransaction?: Transaction; private static _currentIndex: number = 1; @@ -164,8 +164,8 @@ export class Tracing implements Integration { spanDebugTimingInfo: false, writeAsBreadcrumbs: false, }, - discardBackgroundSpans: true, idleTimeout: 500, + markBackgroundTransactions: true, maxTransactionDuration: 600, shouldCreateSpanForRequest(url: string): boolean { const origins = (_options && _options.tracingOrigins) || defaultTracingOrigins; @@ -205,7 +205,8 @@ export class Tracing implements Integration { // Starting pageload transaction if (global.location && global.location.href) { // Use `${global.location.href}` as transaction name - Tracing.startIdleTransaction(global.location.href, { + Tracing.startIdleTransaction({ + name: global.location.href, op: 'pageload', }); } @@ -236,7 +237,7 @@ export class Tracing implements Integration { event.timestamp - event.start_timestamp < 0); if (Tracing.options.maxTransactionDuration !== 0 && event.type === 'transaction' && isOutdatedTransaction) { - Tracing._log('[Tracing] Discarded transaction since it maxed out maxTransactionDuration'); + Tracing._log(`[Tracing] Transaction: ${SpanStatus.Cancelled} since it maxed out maxTransactionDuration`); if (event.contexts && event.contexts.trace) { event.contexts.trace = { ...event.contexts.trace, @@ -279,7 +280,9 @@ export class Tracing implements Integration { if (Tracing._heartbeatCounter >= 3) { if (Tracing._activeTransaction) { Tracing._log( - "[Tracing] Heartbeat safeguard kicked in, finishing transaction since activities content hasn't changed for 3 beats", + `[Tracing] Transaction: ${ + SpanStatus.Cancelled + } -> Heartbeat safeguard kicked in since content hasn't changed for 3 beats`, ); Tracing._activeTransaction.setStatus(SpanStatus.DeadlineExceeded); Tracing._activeTransaction.setTag('heartbeat', 'failed'); @@ -295,10 +298,10 @@ export class Tracing implements Integration { * Discards active transactions if tab moves to background */ private _setupBackgroundTabDetection(): void { - if (Tracing.options.discardBackgroundSpans && global.document) { + if (Tracing.options && Tracing.options.markBackgroundTransactions && global.document) { document.addEventListener('visibilitychange', () => { if (document.hidden && Tracing._activeTransaction) { - Tracing._log('[Tracing] Discarded active transaction incl. activities since tab moved to the background'); + Tracing._log(`[Tracing] Transaction: ${SpanStatus.Cancelled} -> since tab moved to the background`); Tracing._activeTransaction.setStatus(SpanStatus.Cancelled); Tracing._activeTransaction.setTag('visibilitychange', 'document.hidden'); Tracing.finishIdleTransaction(); @@ -374,7 +377,7 @@ export class Tracing implements Integration { /** * If an error or unhandled promise occurs, we mark the active transaction as failed */ - Tracing._log(`[Tracing] Global error occured, setting status in transaction: ${SpanStatus.InternalError}`); + Tracing._log(`[Tracing] Transaction: ${SpanStatus.InternalError} -> Global error occured`); Tracing._activeTransaction.setStatus(SpanStatus.InternalError); } } @@ -404,19 +407,19 @@ export class Tracing implements Integration { return; } } - logger.log(args); + logger.log(...args); } /** * Starts a Transaction waiting for activity idle to finish */ - public static startIdleTransaction(name: string, spanContext?: SpanContext): Span | undefined { + public static startIdleTransaction(transactionContext: TransactionContext): Transaction | undefined { // If we already have an active transaction it means one of two things // a) The user did rapid navigation changes and didn't wait until the transaction was finished // b) A activity wasn't popped correctly and therefore the transaction is stalling Tracing.finishIdleTransaction(); - Tracing._log('[Tracing] startIdleTransaction, name:', name); + Tracing._log('[Tracing] startIdleTransaction, name:', transactionContext.name); const _getCurrentHub = Tracing._getCurrentHub; if (!_getCurrentHub) { @@ -429,16 +432,9 @@ export class Tracing implements Integration { } Tracing._activeTransaction = hub.startSpan({ - ...spanContext, - transaction: name, - }); - - // We set the transaction on the scope so if there are any other spans started outside of this integration - // we also add them to this transaction. - // Once the idle transaction is finished, we make sure to remove it again. - hub.configureScope((scope: Scope) => { - scope.setSpan(Tracing._activeTransaction); - }); + trimEnd: true, + ...transactionContext, + }) as Transaction; // The reason we do this here is because of cached responses // If we start and transaction without an activity it would never finish since there is no activity @@ -454,11 +450,11 @@ export class Tracing implements Integration { * Finshes the current active transaction */ public static finishIdleTransaction(): void { - const active = Tracing._activeTransaction as SpanClass; + const active = Tracing._activeTransaction; if (active) { Tracing._addPerformanceEntries(active); - Tracing._log('[Tracing] finishIdleTransaction', active.transaction); - active.finish(/*trimEnd*/ true); + Tracing._log('[Tracing] finishIdleTransaction', active.name); + active.finish(); Tracing._resetActiveTransaction(); } } @@ -482,29 +478,29 @@ export class Tracing implements Integration { // tslint:disable-next-line: completed-docs function addPerformanceNavigationTiming(parent: SpanClass, entry: { [key: string]: number }, event: string): void { - const span = parent.child({ + const span = parent.startChild({ description: event, op: 'browser', }); span.startTimestamp = timeOrigin + Tracing._msToSec(entry[`${event}Start`]); - span.timestamp = timeOrigin + Tracing._msToSec(entry[`${event}End`]); + span.endTimestamp = timeOrigin + Tracing._msToSec(entry[`${event}End`]); } // tslint:disable-next-line: completed-docs function addRequest(parent: SpanClass, entry: { [key: string]: number }): void { - const request = parent.child({ + const request = parent.startChild({ description: 'request', op: 'browser', }); request.startTimestamp = timeOrigin + Tracing._msToSec(entry.requestStart); - request.timestamp = timeOrigin + Tracing._msToSec(entry.responseEnd); + request.endTimestamp = timeOrigin + Tracing._msToSec(entry.responseEnd); - const response = parent.child({ + const response = parent.startChild({ description: 'response', op: 'browser', }); response.startTimestamp = timeOrigin + Tracing._msToSec(entry.responseStart); - response.timestamp = timeOrigin + Tracing._msToSec(entry.responseEnd); + response.endTimestamp = timeOrigin + Tracing._msToSec(entry.responseEnd); } let entryScriptSrc: string | undefined; @@ -549,12 +545,12 @@ export class Tracing implements Integration { case 'mark': case 'paint': case 'measure': - const mark = transactionSpan.child({ + const mark = transactionSpan.startChild({ description: entry.name, op: entry.entryType, }); mark.startTimestamp = timeOrigin + startTime; - mark.timestamp = mark.startTimestamp + duration; + mark.endTimestamp = mark.startTimestamp + duration; if (tracingInitMarkStartTime === undefined && entry.name === 'sentry-tracing-init') { tracingInitMarkStartTime = mark.startTimestamp; } @@ -567,20 +563,20 @@ export class Tracing implements Integration { transactionSpan.spanRecorder.spans.map((finishedSpan: SpanClass) => { if (finishedSpan.description && finishedSpan.description.indexOf(resourceName) !== -1) { finishedSpan.startTimestamp = timeOrigin + startTime; - finishedSpan.timestamp = finishedSpan.startTimestamp + duration; + finishedSpan.endTimestamp = finishedSpan.startTimestamp + duration; } }); } } else { - const resource = transactionSpan.child({ + const resource = transactionSpan.startChild({ description: `${entry.initiatorType} ${resourceName}`, op: `resource`, }); resource.startTimestamp = timeOrigin + startTime; - resource.timestamp = resource.startTimestamp + duration; + resource.endTimestamp = resource.startTimestamp + duration; // We remember the entry script end time to calculate the difference to the first init mark if (entryScriptStartEndTime === undefined && (entryScriptSrc || '').includes(resourceName)) { - entryScriptStartEndTime = resource.timestamp; + entryScriptStartEndTime = resource.endTimestamp; } } break; @@ -590,12 +586,12 @@ export class Tracing implements Integration { }); if (entryScriptStartEndTime !== undefined && tracingInitMarkStartTime !== undefined) { - const evaluation = transactionSpan.child({ + const evaluation = transactionSpan.startChild({ description: 'evaluation', op: `script`, }); evaluation.startTimestamp = entryScriptStartEndTime; - evaluation.timestamp = tracingInitMarkStartTime; + evaluation.endTimestamp = tracingInitMarkStartTime; } Tracing._performanceCursor = Math.max(performance.getEntries().length - 1, 0); @@ -614,6 +610,13 @@ export class Tracing implements Integration { } } + /** + * Returns the current active idle transaction if there is one + */ + public static getTransaction(): Transaction | undefined { + return Tracing._activeTransaction; + } + /** * Converts from milliseconds to seconds * @param time time in ms @@ -673,7 +676,7 @@ export class Tracing implements Integration { if (spanContext && _getCurrentHub) { const hub = _getCurrentHub(); if (hub) { - const span = activeTransaction.child(spanContext); + const span = activeTransaction.startChild(spanContext); Tracing._activities[Tracing._currentIndex] = { name, span, @@ -858,7 +861,8 @@ function fetchCallback(handlerData: { [key: string]: any }): void { */ function historyCallback(_: { [key: string]: any }): void { if (Tracing.options.startTransactionOnLocationChange && global && global.location) { - Tracing.startIdleTransaction(global.location.href, { + Tracing.startIdleTransaction({ + name: global.location.href, op: 'navigation', }); } diff --git a/packages/apm/src/span.ts b/packages/apm/src/span.ts index 4b6d2fe0f4e8..113580edcdb1 100644 --- a/packages/apm/src/span.ts +++ b/packages/apm/src/span.ts @@ -1,12 +1,9 @@ -// tslint:disable:max-classes-per-file - -import { getCurrentHub, Hub } from '@sentry/hub'; import { Span as SpanInterface, SpanContext } from '@sentry/types'; -import { dropUndefinedKeys, isInstanceOf, logger, timestampWithMs, uuid4 } from '@sentry/utils'; +import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils'; import { SpanStatus } from './spanstatus'; +import { SpanRecorder } from './transaction'; -// TODO: Should this be exported? export const TRACEPARENT_REGEXP = new RegExp( '^[ \\t]*' + // whitespace '([0-9a-f]{32})?' + // trace_id @@ -15,60 +12,29 @@ export const TRACEPARENT_REGEXP = new RegExp( '[ \\t]*$', // whitespace ); -/** - * Keeps track of finished spans for a given transaction - */ -class SpanRecorder { - private readonly _maxlen: number; - public spans: Span[] = []; - - public constructor(maxlen: number = 1000) { - this._maxlen = maxlen; - } - - /** - * This is just so that we don't run out of memory while recording a lot - * of spans. At some point we just stop and flush out the start of the - * trace tree (i.e.the first n spans with the smallest - * start_timestamp). - */ - public add(span: Span): void { - if (this.spans.length > this._maxlen) { - span.spanRecorder = undefined; - } else { - this.spans.push(span); - } - } -} - /** * Span contains all data about a span */ export class Span implements SpanInterface, SpanContext { - /** - * The reference to the current hub. - */ - private readonly _hub: Hub = (getCurrentHub() as unknown) as Hub; - /** * @inheritDoc */ - private readonly _traceId: string = uuid4(); + public traceId: string = uuid4(); /** * @inheritDoc */ - private readonly _spanId: string = uuid4().substring(16); + public spanId: string = uuid4().substring(16); /** * @inheritDoc */ - private readonly _parentSpanId?: string; + public parentSpanId?: string; /** * Internal keeper of the status */ - private _status?: SpanStatus | string; + public status?: SpanStatus | string; /** * @inheritDoc @@ -83,12 +49,7 @@ export class Span implements SpanInterface, SpanContext { /** * Timestamp in seconds when the span ended. */ - public timestamp?: number; - - /** - * @inheritDoc - */ - public transaction?: string; + public endTimestamp?: number; /** * @inheritDoc @@ -121,31 +82,23 @@ export class Span implements SpanInterface, SpanContext { * @hideconstructor * @hidden */ - public constructor(spanContext?: SpanContext, hub?: Hub) { - if (isInstanceOf(hub, Hub)) { - this._hub = hub as Hub; - } - + public constructor(spanContext?: SpanContext) { if (!spanContext) { return this; } - if (spanContext.traceId) { - this._traceId = spanContext.traceId; + this.traceId = spanContext.traceId; } if (spanContext.spanId) { - this._spanId = spanContext.spanId; + this.spanId = spanContext.spanId; } if (spanContext.parentSpanId) { - this._parentSpanId = spanContext.parentSpanId; + this.parentSpanId = spanContext.parentSpanId; } // We want to include booleans as well here if ('sampled' in spanContext) { this.sampled = spanContext.sampled; } - if (spanContext.transaction) { - this.transaction = spanContext.transaction; - } if (spanContext.op) { this.op = spanContext.op; } @@ -159,32 +112,37 @@ export class Span implements SpanInterface, SpanContext { this.tags = spanContext.tags; } if (spanContext.status) { - this._status = spanContext.status; + this.status = spanContext.status; + } + if (spanContext.startTimestamp) { + this.startTimestamp = spanContext.startTimestamp; + } + if (spanContext.endTimestamp) { + this.endTimestamp = spanContext.endTimestamp; } } /** - * Attaches SpanRecorder to the span itself - * @param maxlen maximum number of spans that can be recorded + * @inheritDoc + * @deprecated */ - public initSpanRecorder(maxlen: number = 1000): void { - if (!this.spanRecorder) { - this.spanRecorder = new SpanRecorder(maxlen); - } - this.spanRecorder.add(this); + public child( + spanContext?: Pick>, + ): Span { + return this.startChild(spanContext); } /** * @inheritDoc */ - public child( + public startChild( spanContext?: Pick>, ): Span { const span = new Span({ ...spanContext, - parentSpanId: this._spanId, + parentSpanId: this.spanId, sampled: this.sampled, - traceId: this._traceId, + traceId: this.traceId, }); span.spanRecorder = this.spanRecorder; @@ -195,13 +153,6 @@ export class Span implements SpanInterface, SpanContext { return span; } - /** - * @inheritDoc - */ - public isRootSpan(): boolean { - return this._parentSpanId === undefined; - } - /** * Continues a trace from a string (usually the header). * @param traceparent Traceparent string @@ -248,7 +199,7 @@ export class Span implements SpanInterface, SpanContext { * @inheritDoc */ public setStatus(value: SpanStatus): this { - this._status = value; + this.status = value; return this; } @@ -268,63 +219,14 @@ export class Span implements SpanInterface, SpanContext { * @inheritDoc */ public isSuccess(): boolean { - return this._status === SpanStatus.Ok; + return this.status === SpanStatus.Ok; } /** - * Sets the finish timestamp on the current span. - * @param trimEnd If true, sets the end timestamp of the transaction to the highest timestamp of child spans, trimming - * the duration of the transaction span. This is useful to discard extra time in the transaction span that is not - * accounted for in child spans, like what happens in the idle transaction Tracing integration, where we finish the - * transaction after a given "idle time" and we don't want this "idle time" to be part of the transaction. + * @inheritDoc */ - public finish(trimEnd: boolean = false): string | undefined { - // This transaction is already finished, so we should not flush it again. - if (this.timestamp !== undefined) { - return undefined; - } - - this.timestamp = timestampWithMs(); - - // We will not send any child spans - if (!this.isRootSpan()) { - return undefined; - } - - // This happens if a span was initiated outside of `hub.startSpan` - // Also if the span was sampled (sampled = false) in `hub.startSpan` already - if (this.spanRecorder === undefined) { - return undefined; - } - - if (this.sampled !== true) { - // At this point if `sampled !== true` we want to discard the transaction. - logger.warn('Discarding transaction Span because it was span.sampled !== true'); - return undefined; - } - - const finishedSpans = this.spanRecorder ? this.spanRecorder.spans.filter(s => s !== this && s.timestamp) : []; - - if (trimEnd && finishedSpans.length > 0) { - this.timestamp = finishedSpans.reduce((prev: Span, current: Span) => { - if (prev.timestamp && current.timestamp) { - return prev.timestamp > current.timestamp ? prev : current; - } - return prev; - }).timestamp; - } - - return this._hub.captureEvent({ - contexts: { - trace: this.getTraceContext(), - }, - spans: finishedSpans, - start_timestamp: this.startTimestamp, - tags: this.tags, - timestamp: this.timestamp, - transaction: this.transaction, - type: 'transaction', - }); + public finish(endTimestamp?: number): void { + this.endTimestamp = typeof endTimestamp === 'number' ? endTimestamp : timestampWithMs(); } /** @@ -335,7 +237,7 @@ export class Span implements SpanInterface, SpanContext { if (this.sampled !== undefined) { sampledString = this.sampled ? '-1' : '-0'; } - return `${this._traceId}-${this._spanId}${sampledString}`; + return `${this.traceId}-${this.spanId}${sampledString}`; } /** @@ -355,11 +257,11 @@ export class Span implements SpanInterface, SpanContext { data: Object.keys(this.data).length > 0 ? this.data : undefined, description: this.description, op: this.op, - parent_span_id: this._parentSpanId, - span_id: this._spanId, - status: this._status, + parent_span_id: this.parentSpanId, + span_id: this.spanId, + status: this.status, tags: Object.keys(this.tags).length > 0 ? this.tags : undefined, - trace_id: this._traceId, + trace_id: this.traceId, }); } @@ -377,20 +279,18 @@ export class Span implements SpanInterface, SpanContext { tags?: { [key: string]: string }; timestamp?: number; trace_id: string; - transaction?: string; } { return dropUndefinedKeys({ data: Object.keys(this.data).length > 0 ? this.data : undefined, description: this.description, op: this.op, - parent_span_id: this._parentSpanId, + parent_span_id: this.parentSpanId, sampled: this.sampled, - span_id: this._spanId, + span_id: this.spanId, start_timestamp: this.startTimestamp, tags: Object.keys(this.tags).length > 0 ? this.tags : undefined, - timestamp: this.timestamp, - trace_id: this._traceId, - transaction: this.transaction, + timestamp: this.endTimestamp, + trace_id: this.traceId, }); } } diff --git a/packages/apm/src/transaction.ts b/packages/apm/src/transaction.ts new file mode 100644 index 000000000000..fb3cdc63589f --- /dev/null +++ b/packages/apm/src/transaction.ts @@ -0,0 +1,126 @@ +// tslint:disable:max-classes-per-file +import { getCurrentHub, Hub } from '@sentry/hub'; +import { TransactionContext } from '@sentry/types'; +import { isInstanceOf, logger } from '@sentry/utils'; + +import { Span as SpanClass } from './span'; + +/** + * Keeps track of finished spans for a given transaction + * @internal + * @hideconstructor + * @hidden + */ +export class SpanRecorder { + private readonly _maxlen: number; + public spans: SpanClass[] = []; + + public constructor(maxlen: number = 1000) { + this._maxlen = maxlen; + } + + /** + * This is just so that we don't run out of memory while recording a lot + * of spans. At some point we just stop and flush out the start of the + * trace tree (i.e.the first n spans with the smallest + * start_timestamp). + */ + public add(span: SpanClass): void { + if (this.spans.length > this._maxlen) { + span.spanRecorder = undefined; + } else { + this.spans.push(span); + } + } +} + +/** JSDoc */ +export class Transaction extends SpanClass { + /** + * The reference to the current hub. + */ + private readonly _hub: Hub = (getCurrentHub() as unknown) as Hub; + + public name?: string; + + 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()`. + * @internal + * @hideconstructor + * @hidden + */ + public constructor(transactionContext: TransactionContext, hub?: Hub) { + super(transactionContext); + + if (isInstanceOf(hub, Hub)) { + this._hub = hub as Hub; + } + + if (transactionContext.name) { + this.name = transactionContext.name; + } + + this._trimEnd = transactionContext.trimEnd; + } + + /** + * JSDoc + */ + public setName(name: string): void { + this.name = name; + } + + /** + * Attaches SpanRecorder to the span itself + * @param maxlen maximum number of spans that can be recorded + */ + public initSpanRecorder(maxlen: number = 1000): void { + if (!this.spanRecorder) { + this.spanRecorder = new SpanRecorder(maxlen); + } + this.spanRecorder.add(this); + } + + /** + * @inheritDoc + */ + public finish(endTimestamp?: number): string | undefined { + // This transaction is already finished, so we should not flush it again. + if (this.endTimestamp !== undefined) { + return undefined; + } + + super.finish(endTimestamp); + + if (this.sampled !== true) { + // At this point if `sampled !== true` we want to discard the transaction. + logger.warn('Discarding transaction because it was not chosen to be sampled.'); + return undefined; + } + + const finishedSpans = this.spanRecorder ? this.spanRecorder.spans.filter(s => s !== this && s.endTimestamp) : []; + + if (this._trimEnd && finishedSpans.length > 0) { + this.endTimestamp = finishedSpans.reduce((prev: SpanClass, current: SpanClass) => { + if (prev.endTimestamp && current.endTimestamp) { + return prev.endTimestamp > current.endTimestamp ? prev : current; + } + return prev; + }).endTimestamp; + } + + return this._hub.captureEvent({ + contexts: { + trace: this.getTraceContext(), + }, + spans: finishedSpans, + start_timestamp: this.startTimestamp, + tags: this.tags, + timestamp: this.endTimestamp, + transaction: this.name, + type: 'transaction', + }); + } +} diff --git a/packages/apm/test/hub.test.ts b/packages/apm/test/hub.test.ts index cd8f864ab9ee..b067ea77d46e 100644 --- a/packages/apm/test/hub.test.ts +++ b/packages/apm/test/hub.test.ts @@ -1,5 +1,6 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, Scope } from '@sentry/hub'; +import { Span, Transaction } from '@sentry/types'; import { addExtensionMethods } from '../src/hubextensions'; @@ -13,46 +14,71 @@ describe('Hub', () => { describe('spans', () => { describe('sampling', () => { - test('set tracesSampleRate 0 root span', () => { + test('set tracesSampleRate 0 on span', () => { const hub = new Hub(new BrowserClient({ tracesSampleRate: 0 })); - const span = hub.startSpan() as any; - expect(span.sampled).toBe(false); + const span = hub.startSpan({}) as any; + expect(span.sampled).toBeUndefined(); }); test('set tracesSampleRate 0 on transaction', () => { const hub = new Hub(new BrowserClient({ tracesSampleRate: 0 })); - const span = hub.startSpan({ transaction: 'foo' }) as any; - expect(span.sampled).toBe(false); + const transaction = hub.startSpan({ name: 'foo' }) as any; + expect(transaction.sampled).toBe(false); }); test('set tracesSampleRate 1 on transaction', () => { const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 })); - const span = hub.startSpan({ transaction: 'foo' }) as any; - expect(span.sampled).toBeTruthy(); + const transaction = hub.startSpan({ name: 'foo' }) as any; + expect(transaction.sampled).toBeTruthy(); }); test('set tracesSampleRate should be propergated to children', () => { const hub = new Hub(new BrowserClient({ tracesSampleRate: 0 })); - const span = hub.startSpan() as any; - const child = span.child({ op: 1 }); + const transaction = hub.startSpan({ name: 'foo' }) as any; + const child = transaction.startChild({ op: 1 }); expect(child.sampled).toBeFalsy(); }); }); - describe('start', () => { - test('simple', () => { + describe('startSpan', () => { + test('simple standalone Span', () => { const hub = new Hub(new BrowserClient()); - const span = hub.startSpan() as any; - expect(span._spanId).toBeTruthy(); + const span = hub.startSpan({}) as any; + expect(span.spanId).toBeTruthy(); }); - test('inherits from parent span', () => { + test('simple standalone Transaction', () => { + const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 })); + const transaction = hub.startSpan({ name: 'transaction' }) as Transaction; + expect(transaction.spanId).toBeTruthy(); + // tslint:disable-next-line: no-unbound-method + expect(transaction.setName).toBeTruthy(); + }); + + test('Transaction inherits trace_id from span on scope', () => { const myScope = new Scope(); const hub = new Hub(new BrowserClient(), myScope); const parentSpan = hub.startSpan({}) as any; - expect(parentSpan._parentId).toBeFalsy(); hub.configureScope(scope => { scope.setSpan(parentSpan); }); - const span = hub.startSpan({}) as any; - expect(span._parentSpanId).toBeTruthy(); + const span = hub.startSpan({ name: 'test' }) as any; + expect(span.trace_id).toEqual(parentSpan.trace_id); + }); + + 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); + const transaction = hub.startSpan({ name: 'transaction' }) as Transaction; + hub.configureScope(scope => { + scope.setSpan(transaction); + }); + const span = hub.startSpan({}) as Span; + expect(span.traceId).toEqual(transaction.traceId); + expect(span.parentSpanId).toEqual(transaction.spanId); + hub.configureScope(scope => { + scope.setSpan(span); + }); + const span2 = hub.startSpan({}) as Span; + expect(span2.traceId).toEqual(span.traceId); + expect(span2.parentSpanId).toEqual(span.spanId); }); }); }); diff --git a/packages/apm/test/span.test.ts b/packages/apm/test/span.test.ts index 188ff8d6b691..2bfa32a8ebe1 100644 --- a/packages/apm/test/span.test.ts +++ b/packages/apm/test/span.test.ts @@ -1,7 +1,7 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, Scope } from '@sentry/hub'; -import { Span, SpanStatus, TRACEPARENT_REGEXP } from '../src'; +import { Span, SpanStatus, TRACEPARENT_REGEXP, Transaction } from '../src'; describe('Span', () => { let hub: Hub; @@ -11,20 +11,37 @@ describe('Span', () => { hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }), myScope); }); - describe('newSpan', () => { + describe('new Span', () => { test('simple', () => { const span = new Span({ sampled: true }); - const span2 = span.child(); - expect((span2 as any)._parentSpanId).toBe((span as any)._spanId); - expect((span2 as any)._traceId).toBe((span as any)._traceId); + const span2 = span.startChild(); + expect((span2 as any).parentSpanId).toBe((span as any).spanId); + expect((span2 as any).traceId).toBe((span as any).traceId); expect((span2 as any).sampled).toBe((span as any).sampled); }); + }); + + describe('new Transaction', () => { + test('simple', () => { + const transaction = new Transaction({ name: 'test', sampled: true }); + const span2 = transaction.startChild(); + expect((span2 as any).parentSpanId).toBe((transaction as any).spanId); + expect((span2 as any).traceId).toBe((transaction as any).traceId); + expect((span2 as any).sampled).toBe((transaction as any).sampled); + }); test('gets currentHub', () => { - const span = new Span({}); - const span2 = span.child(); - expect((span as any)._hub).toBeInstanceOf(Hub); - expect((span2 as any)._hub).toBeInstanceOf(Hub); + const transaction = new Transaction({ name: 'test' }); + expect((transaction as any)._hub).toBeInstanceOf(Hub); + }); + + test('inherit span list', () => { + const transaction = new Transaction({ name: 'test', sampled: true }); + const span2 = transaction.startChild(); + const span3 = span2.startChild(); + span3.finish(); + expect(transaction.spanRecorder).toBe(span2.spanRecorder); + expect(transaction.spanRecorder).toBe(span3.spanRecorder); }); }); @@ -74,32 +91,6 @@ describe('Span', () => { }); }); - describe('newSpan', () => { - test('simple', () => { - const span = new Span({ sampled: true }); - const span2 = span.child(); - expect((span2 as any)._parentSpanId).toBe((span as any)._spanId); - expect((span2 as any)._traceId).toBe((span as any)._traceId); - expect((span2 as any).sampled).toBe((span as any).sampled); - }); - - test('gets currentHub', () => { - const span = new Span({}); - const span2 = span.child(); - expect((span as any)._hub).toBeInstanceOf(Hub); - expect((span2 as any)._hub).toBeInstanceOf(Hub); - }); - - test('inherit span list', () => { - const span = new Span({ sampled: true }); - const span2 = span.child(); - const span3 = span.child(); - span3.finish(); - expect(span.spanRecorder).toBe(span2.spanRecorder); - expect(span.spanRecorder).toBe(span3.spanRecorder); - }); - }); - describe('toTraceparent', () => { test('simple', () => { expect(new Span().toTraceparent()).toMatch(TRACEPARENT_REGEXP); @@ -113,9 +104,9 @@ describe('Span', () => { test('no sample', () => { const from = Span.fromTraceparent('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb') as any; - expect(from._parentSpanId).toEqual('bbbbbbbbbbbbbbbb'); - expect(from._traceId).toEqual('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); - expect(from._spanId).not.toEqual('bbbbbbbbbbbbbbbb'); + expect(from.parentSpanId).toEqual('bbbbbbbbbbbbbbbb'); + expect(from.traceId).toEqual('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); + expect(from.spanId).not.toEqual('bbbbbbbbbbbbbbbb'); expect(from.sampled).toBeUndefined(); }); test('sample true', () => { @@ -130,13 +121,13 @@ describe('Span', () => { test('just sample rate', () => { const from = Span.fromTraceparent('0') as any; - expect(from._traceId).toHaveLength(32); - expect(from._spanId).toHaveLength(16); + expect(from.traceId).toHaveLength(32); + expect(from.spanId).toHaveLength(16); expect(from.sampled).toBeFalsy(); const from2 = Span.fromTraceparent('1') as any; - expect(from2._traceId).toHaveLength(32); - expect(from2._spanId).toHaveLength(16); + expect(from2.traceId).toHaveLength(32); + expect(from2.spanId).toHaveLength(16); expect(from2.sampled).toBeTruthy(); }); @@ -156,7 +147,7 @@ describe('Span', () => { test('with parent', () => { const spanA = new Span({ traceId: 'a', spanId: 'b' }) as any; - const spanB = new Span({ traceId: 'c', spanId: 'd', sampled: false, parentSpanId: spanA._spanId }); + const spanB = new Span({ traceId: 'c', spanId: 'd', sampled: false, parentSpanId: spanA.spanId }); const serialized = JSON.parse(JSON.stringify(spanB)); expect(serialized).toHaveProperty('parent_span_id', 'b'); expect(serialized).toHaveProperty('span_id', 'd'); @@ -166,7 +157,7 @@ describe('Span', () => { test('should drop all `undefined` values', () => { const spanA = new Span({ traceId: 'a', spanId: 'b' }) as any; const spanB = new Span({ - parentSpanId: spanA._spanId, + parentSpanId: spanA.spanId, sampled: false, spanId: 'd', traceId: 'c', @@ -186,80 +177,60 @@ describe('Span', () => { describe('finish', () => { test('simple', () => { const span = new Span({}); - expect(span.timestamp).toBeUndefined(); + expect(span.endTimestamp).toBeUndefined(); span.finish(); - expect(span.timestamp).toBeGreaterThan(1); + expect(span.endTimestamp).toBeGreaterThan(1); }); describe('hub.startSpan', () => { - test('finish a span', () => { + test('finish a transaction', () => { const spy = jest.spyOn(hub as any, 'captureEvent') as any; - const span = hub.startSpan(); - span.finish(); + const transaction = hub.startSpan({ name: 'test' }); + transaction.finish(); expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0].spans).toHaveLength(0); expect(spy.mock.calls[0][0].timestamp).toBeTruthy(); expect(spy.mock.calls[0][0].start_timestamp).toBeTruthy(); - expect(spy.mock.calls[0][0].contexts.trace).toEqual(span.getTraceContext()); + expect(spy.mock.calls[0][0].contexts.trace).toEqual(transaction.getTraceContext()); }); - test('finish a span with transaction + child span', () => { + test('finish a transaction + child span', () => { const spy = jest.spyOn(hub as any, 'captureEvent') as any; - const parentSpan = hub.startSpan(); - const childSpan = parentSpan.child(); + const transaction = hub.startSpan({ name: 'test' }); + const childSpan = transaction.startChild(); childSpan.finish(); - parentSpan.finish(); + transaction.finish(); expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0].spans).toHaveLength(1); - expect(spy.mock.calls[0][0].contexts.trace).toEqual(parentSpan.getTraceContext()); + expect(spy.mock.calls[0][0].contexts.trace).toEqual(transaction.getTraceContext()); }); test("finish a child span shouldn't trigger captureEvent", () => { const spy = jest.spyOn(hub as any, 'captureEvent') as any; - const parentSpan = hub.startSpan(); - const childSpan = parentSpan.child(); + const transaction = hub.startSpan({ name: 'test' }); + const childSpan = transaction.startChild(); childSpan.finish(); expect(spy).not.toHaveBeenCalled(); }); - test('finish a span with another one on the scope should add the span and not call captureEvent', () => { - const spy = jest.spyOn(hub as any, 'captureEvent') as any; - - const spanOne = hub.startSpan(); - const childSpanOne = spanOne.child(); - childSpanOne.finish(); - - hub.configureScope(scope => { - scope.setSpan(spanOne); - }); - - const spanTwo = hub.startSpan(); - spanTwo.finish(); - - expect(spy).not.toHaveBeenCalled(); - expect((spanOne as any).spanRecorder.spans).toHaveLength(3); - // We only want two finished spans - expect((spanOne as any).spanRecorder.spans.filter((s: Span) => !!s.timestamp)).toHaveLength(2); - }); - test("finish a span with another one on the scope shouldn't override contexts.trace", () => { const spy = jest.spyOn(hub as any, 'captureEvent') as any; - const spanOne = hub.startSpan(); - const childSpanOne = spanOne.child(); + const transaction = hub.startSpan({ name: 'test' }); + const childSpanOne = transaction.startChild(); childSpanOne.finish(); hub.configureScope(scope => { - scope.setSpan(spanOne); + scope.setSpan(childSpanOne); }); - const spanTwo = hub.startSpan(); + const spanTwo = transaction.startChild(); spanTwo.finish(); - spanOne.finish(); + transaction.finish(); expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0].spans).toHaveLength(2); - expect(spy.mock.calls[0][0].contexts.trace).toEqual(spanOne.getTraceContext()); + expect(spy.mock.calls[0][0].contexts.trace).toEqual(transaction.getTraceContext()); }); test('span child limit', () => { @@ -270,33 +241,33 @@ describe('Span', () => { }), ); const spy = jest.spyOn(_hub as any, 'captureEvent') as any; - const span = _hub.startSpan(); + const transaction = _hub.startSpan({ name: 'test' }); for (let i = 0; i < 10; i++) { - const child = span.child(); + const child = transaction.startChild(); child.finish(); } - span.finish(); + transaction.finish(); expect(spy.mock.calls[0][0].spans).toHaveLength(3); }); - test('if we sampled the parent (transaction) we do not want any childs', () => { + test('if we sampled the transaction we do not want any children', () => { const _hub = new Hub( new BrowserClient({ tracesSampleRate: 0, }), ); const spy = jest.spyOn(_hub as any, 'captureEvent') as any; - const span = _hub.startSpan(); + const transaction = _hub.startSpan({ name: 'test' }); for (let i = 0; i < 10; i++) { - const child = span.child(); + const child = transaction.startChild(); child.finish(); } - span.finish(); - expect((span as any).spanRecorder).toBeUndefined(); + transaction.finish(); + expect((transaction as any).spanRecorder).toBeUndefined(); expect(spy).not.toHaveBeenCalled(); }); - test('mixing hub.startSpan + span.child + maxSpans', () => { + test('mixing hub.startSpan(transaction) + span.startChild + maxSpans', () => { const _hub = new Hub( new BrowserClient({ _experiments: { maxSpans: 2 }, @@ -305,21 +276,21 @@ describe('Span', () => { ); const spy = jest.spyOn(_hub as any, 'captureEvent') as any; - const spanOne = _hub.startSpan(); - const childSpanOne = spanOne.child({ op: '1' }); + const transaction = _hub.startSpan({ name: 'test' }); + const childSpanOne = transaction.startChild({ op: '1' }); childSpanOne.finish(); _hub.configureScope(scope => { - scope.setSpan(spanOne); + scope.setSpan(transaction); }); - const spanTwo = _hub.startSpan({ op: '2' }); + const spanTwo = transaction.startChild({ op: '2' }); spanTwo.finish(); - const spanThree = _hub.startSpan({ op: '3' }); + const spanThree = transaction.startChild({ op: '3' }); spanThree.finish(); - spanOne.finish(); + transaction.finish(); expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0].spans).toHaveLength(2); @@ -328,28 +299,28 @@ describe('Span', () => { test('tree structure of spans should be correct when mixing it with span on scope', () => { const spy = jest.spyOn(hub as any, 'captureEvent') as any; - const spanOne = hub.startSpan(); - const childSpanOne = spanOne.child(); + const transaction = hub.startSpan({ name: 'test' }); + const childSpanOne = transaction.startChild(); - const childSpanTwo = childSpanOne.child(); + const childSpanTwo = childSpanOne.startChild(); childSpanTwo.finish(); childSpanOne.finish(); hub.configureScope(scope => { - scope.setSpan(spanOne); + scope.setSpan(transaction); }); - const spanTwo = hub.startSpan(); + const spanTwo = transaction.startChild({}); spanTwo.finish(); - spanOne.finish(); + transaction.finish(); expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0].spans).toHaveLength(3); - expect(spy.mock.calls[0][0].contexts.trace).toEqual(spanOne.getTraceContext()); - expect(childSpanOne.toJSON().parent_span_id).toEqual(spanOne.toJSON().span_id); + expect(spy.mock.calls[0][0].contexts.trace).toEqual(transaction.getTraceContext()); + expect(childSpanOne.toJSON().parent_span_id).toEqual(transaction.toJSON().span_id); expect(childSpanTwo.toJSON().parent_span_id).toEqual(childSpanOne.toJSON().span_id); - expect(spanTwo.toJSON().parent_span_id).toEqual(spanOne.toJSON().span_id); + expect(spanTwo.toJSON().parent_span_id).toEqual(transaction.toJSON().span_id); }); }); }); diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index dc6b03df4b8a..e9ce11d4673f 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -26,6 +26,7 @@ export { getCurrentHub, Hub, Scope, + startTransaction, setContext, setExtra, setExtras, diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 114b75e362ca..b4143317f8c9 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -392,9 +392,11 @@ export abstract class BaseClient implement return SyncPromise.reject('SDK not enabled, will not send event.'); } + const isTransaction = event.type === 'transaction'; // 1.0 === 100% events are sent // 0.0 === 0% events are sent - if (typeof sampleRate === 'number' && Math.random() > sampleRate) { + // Sampling for transaction happens somewhere else + if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) { return SyncPromise.reject('This event has been sampled, will not send event.'); } @@ -409,7 +411,8 @@ export abstract class BaseClient implement let finalEvent: Event | null = prepared; const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; - if (isInternalException || !beforeSend) { + // We skip beforeSend in case of transactions + if (isInternalException || !beforeSend || isTransaction) { this._getBackend().sendEvent(finalEvent); resolve(finalEvent); return; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0dcfc7bdb9e8..5418c728ff54 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -4,6 +4,7 @@ export { captureEvent, captureMessage, configureScope, + startTransaction, setContext, setExtra, setExtras, diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index ddfd4dc0e9cc..32f82e8c25a6 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -10,6 +10,8 @@ import { Severity, Span, SpanContext, + Transaction, + TransactionContext, User, } from '@sentry/types'; import { consoleSandbox, getGlobalObject, isNodeEnv, logger, timestampWithMs, uuid4 } from '@sentry/utils'; @@ -366,8 +368,8 @@ export class Hub implements HubInterface { /** * @inheritDoc */ - public startSpan(spanOrSpanContext?: Span | SpanContext, forceNoChild: boolean = false): Span { - return this._callExtensionMethod('startSpan', spanOrSpanContext, forceNoChild); + public startSpan(context: SpanContext | TransactionContext): Transaction | Span { + return this._callExtensionMethod('startSpan', context); } /** diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 66c2067fa4cf..d64923dbf19e 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -185,9 +185,6 @@ export class Scope implements ScopeInterface { */ public setTransaction(transaction?: string): this { this._transaction = transaction; - if (this._span) { - (this._span as any).transaction = transaction; - } this._notifyScopeListeners(); return this; } @@ -332,9 +329,6 @@ export class Scope implements ScopeInterface { if (this._transaction) { event.transaction = this._transaction; } - if (this._span) { - event.contexts = { trace: this._span.getTraceContext(), ...event.contexts }; - } this._applyFingerprint(event); diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index 73f4a48e7c4f..3628961d0409 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -245,38 +245,6 @@ describe('Scope', () => { }); }); - test('applyToEvent trace context', async () => { - expect.assertions(1); - const scope = new Scope(); - const span = { - fake: 'span', - getTraceContext: () => ({ a: 'b' }), - } as any; - scope.setSpan(span); - const event: Event = {}; - return scope.applyToEvent(event).then(processedEvent => { - expect((processedEvent!.contexts!.trace as any).a).toEqual('b'); - }); - }); - - test('applyToEvent existing trace context in event should be stronger', async () => { - expect.assertions(1); - const scope = new Scope(); - const span = { - fake: 'span', - getTraceContext: () => ({ a: 'b' }), - } as any; - scope.setSpan(span); - const event: Event = { - contexts: { - trace: { a: 'c' }, - }, - }; - return scope.applyToEvent(event).then(processedEvent => { - expect((processedEvent!.contexts!.trace as any).a).toEqual('c'); - }); - }); - test('clear', () => { const scope = new Scope(); scope.setExtra('a', 2); diff --git a/packages/integrations/src/vue.ts b/packages/integrations/src/vue.ts index 889a0e854516..e2ed67739947 100644 --- a/packages/integrations/src/vue.ts +++ b/packages/integrations/src/vue.ts @@ -238,11 +238,16 @@ export class Vue implements Integration { if (tracingIntegration) { // tslint:disable-next-line:no-unsafe-any this._tracingActivity = (tracingIntegration as any).constructor.pushActivity('Vue Application Render'); + // tslint:disable-next-line:no-unsafe-any + const transaction = (tracingIntegration as any).constructor.getTransaction(); + if (transaction) { + // tslint:disable-next-line:no-unsafe-any + this._rootSpan = transaction.startChild({ + description: 'Application Render', + op: 'Vue', + }); + } } - this._rootSpan = getCurrentHub().startSpan({ - description: 'Application Render', - op: 'Vue', - }); }); } }; @@ -269,7 +274,7 @@ export class Vue implements Integration { } else { vm.$once(`hook:${hook}`, () => { if (this._rootSpan) { - spans[op] = this._rootSpan.child({ + spans[op] = this._rootSpan.startChild({ description: `Vue <${name}>`, op, }); @@ -300,9 +305,6 @@ export class Vue implements Integration { } this._rootSpanTimer = setTimeout(() => { - if (this._rootSpan) { - ((this._rootSpan as unknown) as { timestamp: number }).timestamp = timestamp; - } if (this._tracingActivity) { // We do this whole dance with `TRACING_GETTER` to prevent `@sentry/apm` from becoming a peerDependency. // We also need to ask for the `.constructor`, as `pushActivity` and `popActivity` are static, not instance methods. @@ -310,6 +312,9 @@ export class Vue implements Integration { if (tracingIntegration) { // tslint:disable-next-line:no-unsafe-any (tracingIntegration as any).constructor.popActivity(this._tracingActivity); + if (this._rootSpan) { + this._rootSpan.finish(timestamp); + } } } }, this._options.tracingOptions.timeout); diff --git a/packages/minimal/src/index.ts b/packages/minimal/src/index.ts index 6e28ce17fcbe..6372d46d16d3 100644 --- a/packages/minimal/src/index.ts +++ b/packages/minimal/src/index.ts @@ -1,5 +1,5 @@ import { getCurrentHub, Hub, Scope } from '@sentry/hub'; -import { Breadcrumb, Event, Severity, User } from '@sentry/types'; +import { Breadcrumb, Event, Severity, Transaction, TransactionContext, User } from '@sentry/types'; /** * This calls a function on the current hub. @@ -167,3 +167,16 @@ export function withScope(callback: (scope: Scope) => void): void { export function _callOnClient(method: string, ...args: any[]): void { callOnHub('_invokeClient', method, ...args); } + +/** + * 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. + */ +export function startTransaction(transactionContext: TransactionContext): Transaction { + return callOnHub('startSpan', { + ...transactionContext, + _isTransaction: true, + }); +} diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index d5a1a2007fed..baab3f0a1753 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -1,5 +1,5 @@ import { Span } from '@sentry/apm'; -import { captureException, getCurrentHub, withScope } from '@sentry/core'; +import { captureException, getCurrentHub, startTransaction, withScope } from '@sentry/core'; import { Event } from '@sentry/types'; import { forget, isPlainObject, isString, logger, normalize } from '@sentry/utils'; import * as cookie from 'cookie'; @@ -32,16 +32,35 @@ export function tracingHandler(): ( const reqMethod = (req.method || '').toUpperCase(); const reqUrl = req.url; - const hub = getCurrentHub(); - const transaction = hub.startSpan({ + let traceId; + let parentSpanId; + + // If there is a trace header set, we extract the data from it and set the span on the scope + // to be the origin an created transaction set the parent_span_id / trace_id + if (req.headers && isString(req.headers['sentry-trace'])) { + const span = Span.fromTraceparent(req.headers['sentry-trace'] as string); + if (span) { + traceId = span.traceId; + parentSpanId = span.parentSpanId; + } + } + + const transaction = startTransaction({ + name: `${reqMethod} ${reqUrl}`, op: 'http.server', - transaction: `${reqMethod} ${reqUrl}`, + parentSpanId, + traceId, }); - hub.configureScope(scope => { + // We put the transaction on the scope so users can attach children to it + getCurrentHub().configureScope(scope => { scope.setSpan(transaction); }); + // We also set __sentry_transaction on the response so people can grab the transaction there to add + // spans to it later. + (res as any).__sentry_transaction = transaction; + res.once('finish', () => { transaction.setHttpStatus(res.statusCode); transaction.finish(); @@ -369,18 +388,14 @@ export function errorHandler(options?: { ) => void { return function sentryErrorMiddleware( error: MiddlewareError, - req: http.IncomingMessage, + _req: http.IncomingMessage, res: http.ServerResponse, next: (error: MiddlewareError) => void, ): void { const shouldHandleError = (options && options.shouldHandleError) || defaultShouldHandleError; if (shouldHandleError(error)) { - withScope(scope => { - if (req.headers && isString(req.headers['sentry-trace'])) { - const span = Span.fromTraceparent(req.headers['sentry-trace'] as string); - scope.setSpan(span); - } + withScope(_scope => { const eventId = captureException(error); (res as any).sentry = eventId; next(error); diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 63b0a7ccb6ca..7c6f6b932252 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -1,5 +1,5 @@ import { getCurrentHub } from '@sentry/core'; -import { Integration, Span } from '@sentry/types'; +import { Integration, Span, Transaction } from '@sentry/types'; import { fill, parseSemver } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; @@ -78,9 +78,16 @@ function createHandlerWrapper( return originalHandler.apply(this, arguments); } - let span: Span; - if (tracingEnabled) { - span = getCurrentHub().startSpan({ + let span: Span | undefined; + 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', }); diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts index 20e0adadae08..1db098d89c46 100644 --- a/packages/types/src/hub.ts +++ b/packages/types/src/hub.ts @@ -5,6 +5,7 @@ import { Integration, IntegrationClass } from './integration'; import { Scope } from './scope'; import { Severity } from './severity'; import { Span, SpanContext } from './span'; +import { Transaction, TransactionContext } from './transaction'; import { User } from './user'; /** @@ -173,11 +174,10 @@ export interface Hub { traceHeaders(): { [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`. + * 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. * - * @param spanContext Properties with which the span should be created + * @param context Properties with which the Transaction/Span should be created */ - startSpan(spanContext?: SpanContext): Span; + startSpan(context: SpanContext | TransactionContext): Transaction | Span; } diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 6e82841dde4e..857d304d42c4 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -21,6 +21,7 @@ export { Span, SpanContext } from './span'; export { StackFrame } from './stackframe'; export { Stacktrace } from './stacktrace'; export { Status } from './status'; +export { Transaction, TransactionContext } from './transaction'; export { Thread } from './thread'; export { Transport, TransportOptions, TransportClass } from './transport'; export { User } from './user'; diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index afcfb6e8fd47..c3685406d6e5 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -1,123 +1,168 @@ -/** Span holding trace_id, span_id */ -export interface Span { - /** Sets the finish timestamp on the current span and sends it if it was a transaction */ - finish(useLastSpanTimestamp?: boolean): string | undefined; - /** Return a traceparent compatible header string */ - toTraceparent(): string; - /** Convert the object to JSON for w. spans array info only */ - getTraceContext(): { - data?: { [key: string]: any }; - description?: string; - op?: string; - parent_span_id?: string; - span_id: string; - status?: string; - tags?: { [key: string]: string }; - trace_id: string; - }; - /** Convert the object to JSON */ - toJSON(): { - data?: { [key: string]: any }; - description?: string; - op?: string; - parent_span_id?: string; - sampled?: boolean; - span_id: string; - start_timestamp: number; - tags?: { [key: string]: string }; - timestamp?: number; - trace_id: string; - transaction?: string; - }; - +/** Interface holding all properties that can be set on a Span on creation. */ +export interface SpanContext { /** - * Sets the tag attribute on the current span - * @param key Tag key - * @param value Tag value + * Description of the Span. */ - setTag(key: string, value: string): this; + description?: string; /** - * Sets the data attribute on the current span - * @param key Data key - * @param value Data value + * Operation of the Span. */ - setData(key: string, value: any): this; + op?: string; /** - * Sets the status attribute on the current span + * Completion status of the Span. * See: {@sentry/apm SpanStatus} for possible values - * @param status http code used to set the status */ - setStatus(status: string): this; + status?: string; /** - * Sets the status attribute on the current span based on the http code - * @param httpStatus http code used to set the status + * Parent Span ID */ - setHttpStatus(httpStatus: number): this; + parentSpanId?: string; /** - * Creates a new `Span` while setting the current `Span.id` as `parentSpanId`. - * Also the `sampled` decision will be inherited. + * Was this span chosen to be sent as part of the sample? */ - child( - spanContext?: Pick>, - ): Span; + sampled?: boolean; /** - * Determines whether span was successful (HTTP200) + * Span ID */ - isSuccess(): boolean; + spanId?: string; + + /** + * Trace ID + */ + traceId?: string; /** - * Determines if the span is transaction (root) + * Tags of the Span. */ - isRootSpan(): boolean; + tags?: { [key: string]: string }; + + /** + * Data of the Span. + */ + data?: { [key: string]: any }; + + /** + * Timestamp in seconds (epoch time) indicating when the span started. + */ + startTimestamp?: number; + + /** + * Timestamp in seconds (epoch time) indicating when the span ended. + */ + endTimestamp?: number; } -/** Interface holder all properties that can be set on a Span on creation. */ -export interface SpanContext { +/** Span holding trace_id, span_id */ +export interface Span extends SpanContext { /** - * Description of the Span. + * @inheritDoc */ - description?: string; + spanId: string; + /** - * Operation of the Span. + * @inheritDoc */ - op?: string; + traceId: string; + /** - * Completion status of the Span. - * See: {@sentry/apm SpanStatus} for possible values + * @inheritDoc */ - status?: string; + startTimestamp: number; + /** - * Parent Span ID + * @inheritDoc */ - parentSpanId?: string; + tags: { [key: string]: string }; + /** - * Has the sampling decision been made? + * @inheritDoc */ - sampled?: boolean; + data: { [key: string]: any }; + /** - * Span ID + * Sets the finish timestamp on the current span. + * @param endTimestamp Takes an endTimestamp if the end should not be the time when you call this function. */ - spanId?: string; + finish(endTimestamp?: number): void; + /** - * Trace ID + * Sets the tag attribute on the current span + * @param key Tag key + * @param value Tag value */ - traceId?: string; + setTag(key: string, value: string): this; + /** - * Transaction of the Span. + * Sets the data attribute on the current span + * @param key Data key + * @param value Data value */ - transaction?: string; + setData(key: string, value: any): this; + /** - * Tags of the Span. + * Sets the status attribute on the current span + * See: {@sentry/apm SpanStatus} for possible values + * @param status http code used to set the status */ - tags?: { [key: string]: string }; + setStatus(status: string): this; /** - * Data of the Span. + * Sets the status attribute on the current span based on the http code + * @param httpStatus http code used to set the status */ - data?: { [key: string]: any }; + setHttpStatus(httpStatus: number): this; + + /** + * Use {@link startChild} + * @deprecated + */ + child( + spanContext?: Pick>, + ): Span; + + /** + * Creates a new `Span` while setting the current `Span.id` as `parentSpanId`. + * Also the `sampled` decision will be inherited. + */ + startChild( + spanContext?: Pick>, + ): Span; + + /** + * Determines whether span was successful (HTTP200) + */ + isSuccess(): boolean; + + /** Return a traceparent compatible header string */ + toTraceparent(): string; + + /** Convert the object to JSON for w. spans array info only */ + getTraceContext(): { + data?: { [key: string]: any }; + description?: string; + op?: string; + parent_span_id?: string; + span_id: string; + status?: string; + tags?: { [key: string]: string }; + trace_id: string; + }; + /** Convert the object to JSON */ + toJSON(): { + data?: { [key: string]: any }; + description?: string; + op?: string; + parent_span_id?: string; + sampled?: boolean; + span_id: string; + start_timestamp: number; + tags?: { [key: string]: string }; + timestamp?: number; + trace_id: string; + }; } diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts new file mode 100644 index 000000000000..3f0b91da3bf8 --- /dev/null +++ b/packages/types/src/transaction.ts @@ -0,0 +1,57 @@ +import { Span, SpanContext } from './span'; + +/** + * Interface holding Transaction specific properties + */ +export interface TransactionContext extends SpanContext { + name: string; + /** + * If true, sets the end timestamp of the transaction to the highest timestamp of child spans, trimming + * the duration of the transaction. This is useful to discard extra time in the transaction that is not + * accounted for in child spans, like what happens in the idle transaction Tracing integration, where we finish the + * 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; +} + +/** + * Transaction "Class", inherits Span only has `setName` + */ +export interface Transaction extends TransactionContext, Span { + /** + * @inheritDoc + */ + spanId: string; + + /** + * @inheritDoc + */ + traceId: string; + + /** + * @inheritDoc + */ + startTimestamp: number; + + /** + * @inheritDoc + */ + tags: { [key: string]: string }; + + /** + * @inheritDoc + */ + data: { [key: string]: any }; + + /** + * Set the name of the transaction + */ + setName(name: string): void; +}