From 1d34b12697fe5d934cfd84775af869f76693d7cb Mon Sep 17 00:00:00 2001 From: datbth Date: Tue, 25 Jan 2022 17:24:22 +0700 Subject: [PATCH 1/3] changed(tracing): always clear timeout when new activity starts within idle timeout --- packages/tracing/src/idletransaction.ts | 5 ++++- packages/tracing/test/idletransaction.test.ts | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/tracing/src/idletransaction.ts b/packages/tracing/src/idletransaction.ts index 7e876bb18660..0b27b88cb129 100644 --- a/packages/tracing/src/idletransaction.ts +++ b/packages/tracing/src/idletransaction.ts @@ -221,7 +221,10 @@ export class IdleTransaction extends Transaction { // Remember timestampWithMs is in seconds, timeout is in ms const end = timestampWithMs() + timeout / 1000; - setTimeout(() => { + if (this._initTimeout) { + clearTimeout(this._initTimeout); + } + this._initTimeout = setTimeout(() => { if (!this._finished) { this.setTag(FINISH_REASON_TAG, IDLE_TRANSACTION_FINISH_REASONS[1]); this.finish(end); diff --git a/packages/tracing/test/idletransaction.test.ts b/packages/tracing/test/idletransaction.test.ts index b60fec726c54..c923b2e9bd1a 100644 --- a/packages/tracing/test/idletransaction.test.ts +++ b/packages/tracing/test/idletransaction.test.ts @@ -188,6 +188,8 @@ describe('IdleTransaction', () => { it('does not finish if a activity is started', () => { const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234 }, hub, DEFAULT_IDLE_TIMEOUT); transaction.initSpanRecorder(10); + const span = transaction.startChild({}); + span.finish(); transaction.startChild({}); jest.advanceTimersByTime(DEFAULT_IDLE_TIMEOUT); From d8e8cd5c21f92172d1ca2e37445ae4c9b54f5772 Mon Sep 17 00:00:00 2001 From: datbth Date: Tue, 25 Jan 2022 17:43:53 +0700 Subject: [PATCH 2/3] chore(tracing): rename variables --- packages/tracing/src/idletransaction.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/tracing/src/idletransaction.ts b/packages/tracing/src/idletransaction.ts index 0b27b88cb129..45f830f25ec9 100644 --- a/packages/tracing/src/idletransaction.ts +++ b/packages/tracing/src/idletransaction.ts @@ -71,7 +71,7 @@ export class IdleTransaction extends Transaction { * If a transaction is created and no activities are added, we want to make sure that * it times out properly. This is cleared and not used when activities are added. */ - private _initTimeout: ReturnType | undefined; + private _idleTimeout: ReturnType | undefined; public constructor( transactionContext: TransactionContext, @@ -80,7 +80,7 @@ export class IdleTransaction extends Transaction { * The time to wait in ms until the idle transaction will be finished. * @default 1000 */ - private readonly _idleTimeout: number = DEFAULT_IDLE_TIMEOUT, + private readonly _idleTimeoutDuration: number = DEFAULT_IDLE_TIMEOUT, // Whether or not the transaction should put itself on the scope when it starts and pop itself off when it ends private readonly _onScope: boolean = false, ) { @@ -96,11 +96,11 @@ export class IdleTransaction extends Transaction { _idleHub.configureScope(scope => scope.setSpan(this)); } - this._initTimeout = setTimeout(() => { + this._idleTimeout = setTimeout(() => { if (!this._finished) { this.finish(); } - }, this._idleTimeout); + }, this._idleTimeoutDuration); } /** {@inheritDoc} */ @@ -194,9 +194,9 @@ export class IdleTransaction extends Transaction { * @param spanId The span id that represents the activity */ private _pushActivity(spanId: string): void { - if (this._initTimeout) { - clearTimeout(this._initTimeout); - this._initTimeout = undefined; + if (this._idleTimeout) { + clearTimeout(this._idleTimeout); + this._idleTimeout = undefined; } logger.log(`[Tracing] pushActivity: ${spanId}`); this.activities[spanId] = true; @@ -216,15 +216,15 @@ export class IdleTransaction extends Transaction { } if (Object.keys(this.activities).length === 0) { - const timeout = this._idleTimeout; + const timeout = this._idleTimeoutDuration; // 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; - if (this._initTimeout) { - clearTimeout(this._initTimeout); + if (this._idleTimeout) { + clearTimeout(this._idleTimeout); } - this._initTimeout = setTimeout(() => { + this._idleTimeout = setTimeout(() => { if (!this._finished) { this.setTag(FINISH_REASON_TAG, IDLE_TRANSACTION_FINISH_REASONS[1]); this.finish(end); From b9e9fe90d8c1725b5f62a9d3a2ea2d0f15d4a452 Mon Sep 17 00:00:00 2001 From: datbth Date: Tue, 25 Jan 2022 17:51:45 +0700 Subject: [PATCH 3/3] test(tracing): improve test --- packages/tracing/test/idletransaction.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/tracing/test/idletransaction.test.ts b/packages/tracing/test/idletransaction.test.ts index c923b2e9bd1a..54d63b973ad0 100644 --- a/packages/tracing/test/idletransaction.test.ts +++ b/packages/tracing/test/idletransaction.test.ts @@ -190,10 +190,14 @@ describe('IdleTransaction', () => { transaction.initSpanRecorder(10); const span = transaction.startChild({}); span.finish(); - transaction.startChild({}); + const span2 = transaction.startChild({}); jest.advanceTimersByTime(DEFAULT_IDLE_TIMEOUT); expect(transaction.endTimestamp).toBeUndefined(); + + span2.finish(); + jest.advanceTimersByTime(DEFAULT_IDLE_TIMEOUT + DEFAULT_IDLE_TIMEOUT); + expect(transaction.endTimestamp).toBeDefined(); }); });