From e0f9ca0c1b59b99314502052d39d9c99c455cc99 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 28 Feb 2022 13:22:47 -0500 Subject: [PATCH] make sure all finish reasons are defined --- .../tracing/src/browser/browsertracing.ts | 2 +- packages/tracing/src/constants.ts | 3 ++- packages/tracing/src/idletransaction.ts | 26 +++++++++++++------ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 7a530559b799..d5f23f9395c0 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -29,7 +29,7 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { idleTimeout: number; /** - * The max transaction duration for a transaction. If a transaction duration hits the `finalTimeout` value, it + * The max duration for a transaction. If a transaction duration hits the `finalTimeout` value, it * will be finished. * * Time is in ms. diff --git a/packages/tracing/src/constants.ts b/packages/tracing/src/constants.ts index 2d02f3099571..801e1d315e3a 100644 --- a/packages/tracing/src/constants.ts +++ b/packages/tracing/src/constants.ts @@ -1,10 +1,11 @@ // Store finish reasons in tuple to save on bundle size // Readonly type should enforce that this is not mutated. -export const FINISH_REASON_TAG = 'finishReason'; +export const FINISH_REASON_TAG = 'finishReason' as const; export const IDLE_TRANSACTION_FINISH_REASONS = [ 'heartbeatFailed', 'idleTimeout', 'documentHidden', 'finalTimeout', + 'externalFinish', ] as const; diff --git a/packages/tracing/src/idletransaction.ts b/packages/tracing/src/idletransaction.ts index 4e29feb56d8f..9130644732b3 100644 --- a/packages/tracing/src/idletransaction.ts +++ b/packages/tracing/src/idletransaction.ts @@ -77,6 +77,14 @@ export class IdleTransaction extends Transaction { */ private _idleTimeoutID: ReturnType | undefined; + /** + * + * Defaults to `externalFinish`, which means transaction.finish() was called outside of the idle transaction (for example, + * by a navigation transaction ending the previous pageload/navigation in some routing instrumentation). + * @default 'externalFinish' + */ + private _finishReason: typeof IDLE_TRANSACTION_FINISH_REASONS[number] = IDLE_TRANSACTION_FINISH_REASONS[4]; + public constructor( transactionContext: TransactionContext, private readonly _idleHub?: Hub, @@ -114,7 +122,7 @@ export class IdleTransaction extends Transaction { this._startIdleTimeout(); global.setTimeout(() => { if (!this._finished) { - this.setTag(FINISH_REASON_TAG, IDLE_TRANSACTION_FINISH_REASONS[3]); + this._finishReason = IDLE_TRANSACTION_FINISH_REASONS[3]; /* 'finalTimeout' */ this.finish(); } }, this._finalTimeout); @@ -125,6 +133,8 @@ export class IdleTransaction extends Transaction { this._finished = true; this.activities = {}; + this.setTag(FINISH_REASON_TAG, this._finishReason); + if (this.spanRecorder) { logger.log('[Tracing] finishing IdleTransaction', new Date(endTimestamp * 1000).toISOString(), this.op); @@ -207,7 +217,7 @@ export class IdleTransaction extends Transaction { } /** - * Creates an idletimeout + * Cancels the existing idletimeout, if there is one */ private _cancelIdleTimeout(): void { if (this._idleTimeoutID) { @@ -219,12 +229,12 @@ export class IdleTransaction extends Transaction { /** * Creates an idletimeout */ - private _startIdleTimeout(end?: Parameters[0]): void { + private _startIdleTimeout(endTimestamp?: Parameters[0]): void { this._cancelIdleTimeout(); this._idleTimeoutID = global.setTimeout(() => { if (!this._finished && Object.keys(this.activities).length === 0) { - this.setTag(FINISH_REASON_TAG, IDLE_TRANSACTION_FINISH_REASONS[1]); - this.finish(end); + this._finishReason = IDLE_TRANSACTION_FINISH_REASONS[1]; /* 'idleTimeout' */ + this.finish(endTimestamp); } }, this._idleTimeout); } @@ -256,8 +266,8 @@ export class IdleTransaction extends Transaction { const timeout = this._idleTimeout; // We need to add the timeout here to have the real endtimestamp of the transaction // Remember timestampWithMs is in seconds, timeout is in ms - const end = timestampWithMs() + timeout / 1000; - this._startIdleTimeout(end); + const endTimestamp = timestampWithMs() + timeout / 1000; + this._startIdleTimeout(endTimestamp); } } @@ -284,7 +294,7 @@ export class IdleTransaction extends Transaction { if (this._heartbeatCounter >= 3) { logger.log(`[Tracing] Transaction finished because of no change for 3 heart beats`); this.setStatus('deadline_exceeded'); - this.setTag(FINISH_REASON_TAG, IDLE_TRANSACTION_FINISH_REASONS[0]); + this._finishReason = IDLE_TRANSACTION_FINISH_REASONS[0]; /* 'heartbeatFailed' */ this.finish(); } else { this._pingHeartbeat();