From f7c3ca202a55a25a51683f33fe48ea97407f739c Mon Sep 17 00:00:00 2001 From: Alberto Delgado Roda Date: Thu, 21 Dec 2023 20:11:18 +0100 Subject: [PATCH] chore: coordinate pagehidden multitransaction --- .../src/common/observers/page-visibility.js | 23 ++++++++++++++++--- .../metrics/inp/report.js | 2 +- .../transaction-service.js | 3 +-- .../src/performance-monitoring/transaction.js | 11 --------- .../common/observers/page-visibility.spec.js | 2 +- .../metrics/inp/report.spec.js | 2 +- .../transaction-service.spec.js | 12 ---------- 7 files changed, 24 insertions(+), 31 deletions(-) diff --git a/packages/rum-core/src/common/observers/page-visibility.js b/packages/rum-core/src/common/observers/page-visibility.js index f613824ef..95c71023b 100644 --- a/packages/rum-core/src/common/observers/page-visibility.js +++ b/packages/rum-core/src/common/observers/page-visibility.js @@ -76,9 +76,26 @@ export function observePageVisibility(configService, transactionService) { * @param transactionService */ function onPageHidden(configService, transactionService) { - reportInp(transactionService) + const inpTr = reportInp(transactionService) + // we don't want to flush the queue for every transaction that is ended when page becomes hidden + // and given the async nature of the ending process of transactions + // will need to coordinate the flushing process + if (inpTr) { + const unobserve = configService.observeEvent(QUEUE_ADD_TRANSACTION, () => { + // At this point the INP transaction is in the queue + // so let's end the managed transaction if any and flush + endManagedTransaction(configService, transactionService) + unobserve() + }) + } else { + // In the absence of INP transaction + // let's just end the managed transaction if any and flush + endManagedTransaction(configService, transactionService) + } +} - // Ends an ongoing transaction if any and flushes the events queue +// Ends an ongoing managed transaction if any and flushes the events queue +function endManagedTransaction(configService, transactionService) { const tr = transactionService.getCurrentTransaction() if (tr) { const unobserve = configService.observeEvent(QUEUE_ADD_TRANSACTION, () => { @@ -91,7 +108,7 @@ function onPageHidden(configService, transactionService) { unobserve() }) - tr.endPageHidden() + tr.end() } else { configService.dispatchEvent(QUEUE_FLUSH) state.lastHiddenStart = now() diff --git a/packages/rum-core/src/performance-monitoring/metrics/inp/report.js b/packages/rum-core/src/performance-monitoring/metrics/inp/report.js index bfc154d23..6108b991c 100644 --- a/packages/rum-core/src/performance-monitoring/metrics/inp/report.js +++ b/packages/rum-core/src/performance-monitoring/metrics/inp/report.js @@ -57,7 +57,7 @@ export function reportInp(transactionService) { // make sure that transaction's duration is > 0 // we do the +1 because INP can be also reported as 0 const endTime = startTime + inp + 1 - inpTr.endPageHidden(endTime) + inpTr.end(endTime) // restart INP tracking information // since users can enter and leave the page multiple times diff --git a/packages/rum-core/src/performance-monitoring/transaction-service.js b/packages/rum-core/src/performance-monitoring/transaction-service.js index fa02b565b..e9586d726 100644 --- a/packages/rum-core/src/performance-monitoring/transaction-service.js +++ b/packages/rum-core/src/performance-monitoring/transaction-service.js @@ -251,8 +251,7 @@ class TransactionService { () => { const { name, type } = tr let { lastHiddenStart } = state - - if (lastHiddenStart >= tr._start && !tr.endedPageHidden) { + if (lastHiddenStart >= tr._start) { if (__DEV__) { this._logger.debug( `transaction(${tr.id}, ${name}, ${type}) was discarded! The page was hidden during the transaction!` diff --git a/packages/rum-core/src/performance-monitoring/transaction.js b/packages/rum-core/src/performance-monitoring/transaction.js index 864681b55..c4d56e780 100644 --- a/packages/rum-core/src/performance-monitoring/transaction.js +++ b/packages/rum-core/src/performance-monitoring/transaction.js @@ -54,8 +54,6 @@ class Transaction extends SpanBase { this.sampleRate = this.options.transactionSampleRate this.sampled = Math.random() <= this.sampleRate - - this.endedPageHidden = false } addMarks(obj) { @@ -198,15 +196,6 @@ class Transaction extends SpanBase { isManaged() { return !!this.options.managed } - - /* - Makes sure that a transaction ended while the page was becoming hidden - it's not discarded for that very same reason. - */ - endPageHidden(endTime) { - this.endedPageHidden = true - this.end(endTime) - } } export default Transaction diff --git a/packages/rum-core/test/common/observers/page-visibility.spec.js b/packages/rum-core/test/common/observers/page-visibility.spec.js index 2edb8fd9a..9cc733625 100644 --- a/packages/rum-core/test/common/observers/page-visibility.spec.js +++ b/packages/rum-core/test/common/observers/page-visibility.spec.js @@ -159,7 +159,7 @@ describe('observePageVisibility', () => { it('should end the transaction', () => { const transaction = createTransaction() - const endTransactionSpy = spyOn(transaction, 'endPageHidden') + const endTransactionSpy = spyOn(transaction, 'end') spyOn(transactionService, 'getCurrentTransaction').and.returnValue( transaction ) diff --git a/packages/rum-core/test/performance-monitoring/metrics/inp/report.spec.js b/packages/rum-core/test/performance-monitoring/metrics/inp/report.spec.js index e4504d3b8..57c30fff7 100644 --- a/packages/rum-core/test/performance-monitoring/metrics/inp/report.spec.js +++ b/packages/rum-core/test/performance-monitoring/metrics/inp/report.spec.js @@ -59,7 +59,7 @@ if (utils.isPerfTypeSupported(EVENT)) { it('should create page-exit transaction', () => { const hardNavigatedPage = performance.getEntriesByType('navigation')[0] const restoreINPSpy = spyOnFunction(inpProcessor, 'restoreINPState') - const endSpy = spyOnFunction(Transaction.prototype, 'endPageHidden') + const endSpy = spyOnFunction(Transaction.prototype, 'end') endSpy.and.callThrough() // To make sure we can calculate the transaction duration const tr = reportInp(transactionService) diff --git a/packages/rum-core/test/performance-monitoring/transaction-service.spec.js b/packages/rum-core/test/performance-monitoring/transaction-service.spec.js index c74c2a616..96bbf59d3 100644 --- a/packages/rum-core/test/performance-monitoring/transaction-service.spec.js +++ b/packages/rum-core/test/performance-monitoring/transaction-service.spec.js @@ -741,18 +741,6 @@ describe('TransactionService', function () { state.lastHiddenStart = lastHiddenStart }) - it('should not discard transaction ended when page was becoming hidden', async () => { - let { lastHiddenStart } = state - state.lastHiddenStart = performance.now() + 1000 - let tr = transactionService.startTransaction('test-name', 'test-type') - await tr.endPageHidden() - expect(logger.debug).not.toHaveBeenCalledWith( - `transaction(${tr.id}, ${tr.name}, ${tr.type}) was discarded! The page was hidden during the transaction!` - ) - - state.lastHiddenStart = lastHiddenStart - }) - it('should set session information on transaction', () => { config.setConfig({ session: true }) const tr = new Transaction('test', 'test')