chore: emit transaction discard event

packages/rum-core/src/common/constants.js

@@ -110,6 +110,7 @@ const TRANSACTION_END = 'transaction:end'
 const CONFIG_CHANGE = 'config:change'
 const QUEUE_FLUSH = 'queue:flush'
 const QUEUE_ADD_TRANSACTION = 'queue:add_transaction'
+const TRANSACTION_DISCARD = 'transaction:discard'
 
 /**
  * Events types that are used to toggle auto instrumentations
@@ -203,6 +204,7 @@ export {
   CONFIG_CHANGE,
   QUEUE_FLUSH,
   QUEUE_ADD_TRANSACTION,
+  TRANSACTION_DISCARD,
   XMLHTTPREQUEST,
   FETCH,
   HISTORY,

packages/rum-core/src/common/observers/page-visibility.js

@@ -23,7 +23,11 @@
  *
  */
 
-import { QUEUE_ADD_TRANSACTION, QUEUE_FLUSH } from '../constants'
+import {
+  QUEUE_ADD_TRANSACTION,
+  QUEUE_FLUSH,
+  TRANSACTION_DISCARD
+} from '../constants'
 import { state } from '../../state'
 import { now } from '../utils'
 import { reportInp } from '../../performance-monitoring/metrics/inp/report'
@@ -98,15 +102,33 @@ function onPageHidden(configService, transactionService) {
 function endManagedTransaction(configService, transactionService) {
   const tr = transactionService.getCurrentTransaction()
   if (tr) {
-    const unobserve = configService.observeEvent(QUEUE_ADD_TRANSACTION, () => {
-      configService.dispatchEvent(QUEUE_FLUSH)
-      state.lastHiddenStart = now()
+    // Make sure that we still update lastHiddenStart if the managed transaction
+    // ends up being discarded
+    const unobserveDiscard = configService.observeEvent(
+      TRANSACTION_DISCARD,
+      () => {
+        state.lastHiddenStart = now()
 
-      // unsubscribe listener to execute it only once.
-      // otherwise in SPAs we might be finding situations where we would be executing
-      // this logic as many times as we observed to this event
-      unobserve()
-    })
+        // unsubscribe listeners to execute it only once.
+        // otherwise in SPAs we might be finding situations where we would be executing
+        // this logic as many times as we observed to this event
+        unobserveDiscard()
+        unobserveQueueAdd()
+      }
+    )
+    const unobserveQueueAdd = configService.observeEvent(
+      QUEUE_ADD_TRANSACTION,
+      () => {
+        configService.dispatchEvent(QUEUE_FLUSH)
+        state.lastHiddenStart = now()
+
+        // unsubscribe listeners to execute it only once.
+        // otherwise in SPAs we might be finding situations where we would be executing
+        // this logic as many times as we observed to this event
+        unobserveQueueAdd()
+        unobserveDiscard()
+      }
+    )
 
     tr.end()
   } else {

packages/rum-core/src/performance-monitoring/performance-monitoring.js

@@ -47,7 +47,8 @@ import {
   OUTCOME_FAILURE,
   OUTCOME_SUCCESS,
   OUTCOME_UNKNOWN,
-  QUEUE_ADD_TRANSACTION
+  QUEUE_ADD_TRANSACTION,
+  TRANSACTION_DISCARD
 } from '../common/constants'
 import {
   truncateModel,
@@ -423,5 +424,7 @@ export default class PerformanceMonitoring {
     if (filtered) {
       return this.createTransactionDataModel(transaction)
     }
+
+    this._configService.dispatchEvent(TRANSACTION_DISCARD)
   }
 }

packages/rum-core/src/performance-monitoring/transaction-service.js

@@ -45,6 +45,7 @@ import {
   NAME_UNKNOWN,
   TRANSACTION_START,
   TRANSACTION_END,
+  TRANSACTION_DISCARD,
   TEMPORARY_TYPE,
   TRANSACTION_TYPE_ORDER,
   LARGEST_CONTENTFUL_PAINT,
@@ -227,7 +228,7 @@ class TransactionService {
       if (__DEV__) {
         this._logger.debug(`startTransaction(${tr.id}, ${tr.name}, ${tr.type})`)
       }
-      this._config.events.send(TRANSACTION_START, [tr])
+      this._config.dispatchEvent(TRANSACTION_START, [tr])
     }
 
     return tr
@@ -257,6 +258,7 @@ class TransactionService {
               `transaction(${tr.id}, ${name}, ${type}) was discarded! The page was hidden during the transaction!`
             )
           }
+          this._config.dispatchEvent(TRANSACTION_DISCARD)
           return
         }
 
@@ -266,6 +268,7 @@ class TransactionService {
               `transaction(${tr.id}, ${name}, ${type}) is ignored`
             )
           }
+          this._config.dispatchEvent(TRANSACTION_DISCARD)
           return
         }
 
@@ -337,7 +340,7 @@ class TransactionService {
         const configContext = this._config.get('context')
         addTransactionContext(tr, configContext)
 
-        this._config.events.send(TRANSACTION_END, [tr])
+        this._config.dispatchEvent(TRANSACTION_END, [tr])
         if (__DEV__) {
           this._logger.debug(
             `end transaction(${tr.id}, ${tr.name}, ${tr.type})`,

packages/rum-core/test/common/observers/page-visibility.spec.js

@@ -30,7 +30,8 @@ import {
   QUEUE_FLUSH,
   TRANSACTION_SERVICE,
   CONFIG_SERVICE,
-  PERFORMANCE_MONITORING
+  PERFORMANCE_MONITORING,
+  TEMPORARY_TYPE
 } from '../../../src/common/constants'
 import * as utils from '../../../src/common/utils'
 import {
@@ -40,8 +41,17 @@ import {
 } from '../..'
 import { spyOnFunction, waitFor } from '../../../../../dev-utils/jasmine'
 import * as inpReporter from '../../../src/performance-monitoring/metrics/inp/report'
+import * as inpProcessor from '../../../src/performance-monitoring/metrics/inp/process'
 
 describe('observePageVisibility', () => {
+  // Starts performanceMonitoring to observe how transaction ends
+  function startsPerformanceMonitoring() {
+    const performanceMonitoring = serviceFactory.getService(
+      PERFORMANCE_MONITORING
+    )
+    performanceMonitoring.init()
+  }
+
   let serviceFactory
   let configService
   let transactionService
@@ -93,17 +103,32 @@ describe('observePageVisibility', () => {
   // the observed events by the agent...
   ;['visibilitychange', 'pagehide'].forEach(eventName => {
     describe(`when page becomes hidden due to ${eventName.toUpperCase()} event`, () => {
-      it('should report INP', () => {
-        const reportSpy = spyOnFunction(inpReporter, 'reportInp')
+      it('should report INP', async () => {
+        // Arrange
+        const dispatchEventSpy = spyOn(
+          configService,
+          'dispatchEvent'
+        ).and.callThrough()
+        const reportSpy = spyOnFunction(
+          inpReporter,
+          'reportInp'
+        ).and.callThrough()
+        const calculateInpSpy = spyOnFunction(inpProcessor, 'calculateInp')
+        calculateInpSpy.and.returnValue(100) // makes sure the inp transaction is created
         unobservePageVisibility = observePageVisibility(
           configService,
           transactionService
         )
+        startsPerformanceMonitoring()
 
+        // Act
         hidePageSynthetically(eventName)
+        await waitFor(() => dispatchEventSpy.calls.any())
 
+        // Assert
         expect(reportSpy).toHaveBeenCalledTimes(1)
         expect(reportSpy).toHaveBeenCalledWith(transactionService)
+        expect(dispatchEventSpy).toHaveBeenCalledWith(QUEUE_FLUSH)
       })
 
       describe('with every transaction already ended', () => {
@@ -135,14 +160,6 @@ describe('observePageVisibility', () => {
       })
 
       describe('with a transaction that has not ended yet', () => {
-        // Starts performanceMonitoring to observe how transaction ends
-        function startsPerformanceMonitoring() {
-          const performanceMonitoring = serviceFactory.getService(
-            PERFORMANCE_MONITORING
-          )
-          performanceMonitoring.init()
-        }
-
         function createTransaction() {
           const transaction = transactionService.startTransaction(
             'test-tr',
@@ -173,84 +190,65 @@ describe('observePageVisibility', () => {
           expect(endTransactionSpy).toHaveBeenCalledTimes(1)
         })
 
-        it('should dispatch the QUEUE_FLUSH event once the transaction ends and added to the queue', async () => {
-          // Arrange
-          const transaction = createTransaction()
-          const dispatchEventSpy = spyOn(
-            configService,
-            'dispatchEvent'
-          ).and.callThrough()
-          spyOn(transactionService, 'getCurrentTransaction').and.returnValue(
-            transaction
-          )
-          startsPerformanceMonitoring()
-
-          // Act
-          unobservePageVisibility = observePageVisibility(
-            configService,
-            transactionService
-          )
-          hidePageSynthetically(eventName)
-          await waitFor(() => dispatchEventSpy.calls.any())
-
-          // Assert
-          expect(dispatchEventSpy).toHaveBeenCalledWith(QUEUE_FLUSH)
-        })
-
-        it('should remove the QUEUE_ADD_TRANSACTION event listener once the transaction ends and added to the queue', async () => {
-          // Arrange
-          const unobserveSpy = jasmine.createSpy()
-          const observeEvent = configService.observeEvent.bind(configService)
-          spyOn(configService, 'observeEvent').and.callFake((name, fn) => {
-            observeEvent(name, fn)
-            return unobserveSpy
+        describe('event listeners', () => {
+          ;[
+            {
+              name: 'should fire when transaction is added to the queue',
+              createTransaction,
+              customAssertions: ({ dispatchEventSpy }) => {
+                // should dispatch the QUEUE_FLUSH event
+                expect(dispatchEventSpy).toHaveBeenCalledWith(QUEUE_FLUSH)
+              }
+            },
+            {
+              name: 'should fire when transaction is discarded',
+              createTransaction: () => {
+                const tr = createTransaction()
+                // make sure transaction will be discarded
+                tr.type = TEMPORARY_TYPE
+
+                return tr
+              },
+              customAssertions: utils.noop
+            }
+          ].forEach(({ name, createTransaction, customAssertions }) => {
+            it(`${name}`, async () => {
+              // Arrange
+              const anyTime = 1234567834
+              spyOnFunction(utils, 'now').and.returnValue(anyTime)
+              const unobserveSpy = jasmine.createSpy()
+              const observeEvent = configService.observeEvent.bind(
+                configService
+              )
+              spyOn(configService, 'observeEvent').and.callFake((name, fn) => {
+                observeEvent(name, fn)
+                return unobserveSpy
+              })
+              const transaction = createTransaction()
+              const dispatchEventSpy = spyOn(
+                configService,
+                'dispatchEvent'
+              ).and.callThrough()
+              spyOn(
+                transactionService,
+                'getCurrentTransaction'
+              ).and.returnValue(transaction)
+              startsPerformanceMonitoring()
+
+              // Act
+              unobservePageVisibility = observePageVisibility(
+                configService,
+                transactionService
+              )
+              hidePageSynthetically(eventName)
+              await waitFor(() => dispatchEventSpy.calls.any())
+
+              // Assert
+              customAssertions({ dispatchEventSpy })
+              expect(unobserveSpy).toHaveBeenCalledTimes(2)
+              expect(state.lastHiddenStart).toBe(anyTime)
+            })
           })
-          const transaction = createTransaction()
-          const dispatchEventSpy = spyOn(
-            configService,
-            'dispatchEvent'
-          ).and.callThrough()
-          spyOn(transactionService, 'getCurrentTransaction').and.returnValue(
-            transaction
-          )
-          startsPerformanceMonitoring()
-
-          // Act
-          unobservePageVisibility = observePageVisibility(
-            configService,
-            transactionService
-          )
-          hidePageSynthetically(eventName)
-          await waitFor(() => dispatchEventSpy.calls.any())
-
-          // Assert
-          expect(unobserveSpy).toHaveBeenCalledTimes(1)
-        })
-
-        it('should set lastHiddenStart once the transaction ends and added to the queue', async () => {
-          // Arrange
-          const anyTime = 1234567834
-          spyOnFunction(utils, 'now').and.returnValue(anyTime)
-          const transaction = createTransaction()
-          const dispatchEventSpy = spyOn(
-            configService,
-            'dispatchEvent'
-          ).and.callThrough()
-          spyOn(transactionService, 'getCurrentTransaction').and.returnValue(
-            transaction
-          )
-          startsPerformanceMonitoring()
-
-          // Act
-          unobservePageVisibility = observePageVisibility(
-            configService,
-            transactionService
-          )
-          hidePageSynthetically(eventName)
-          await waitFor(() => dispatchEventSpy.calls.any())
-
-          // Assert
-          expect(state.lastHiddenStart).toBe(anyTime)
         })
       })
     })

packages/rum-core/test/performance-monitoring/performance-monitoring.spec.js

@@ -49,7 +49,8 @@ import {
   LOGGING_SERVICE,
   CONFIG_SERVICE,
   APM_SERVER,
-  PERFORMANCE_MONITORING
+  PERFORMANCE_MONITORING,
+  TRANSACTION_DISCARD
 } from '../../src/common/constants'
 import { state } from '../../src/state'
 import patchEventHandler from '../common/patch'
@@ -195,6 +196,18 @@ describe('PerformanceMonitoring', function () {
     expect(payload.spans[0].duration).toBe(parseInt(span._end - span._start))
   })
 
+  it('should notify when a transaction has been filtered out', function () {
+    spyOn(configService, 'dispatchEvent')
+    var tr = new Transaction('transaction-no-duration', 'transaction-type')
+    tr.end()
+
+    var payload = performanceMonitoring.createTransactionPayload(tr)
+    expect(payload).toBeUndefined()
+    expect(configService.dispatchEvent).toHaveBeenCalledWith(
+      TRANSACTION_DISCARD
+    )
+  })
+
   it('should sendPageLoadMetrics', function (done) {
     const unMock = mockGetEntriesByType()
     const transactionService = serviceFactory.getService(TRANSACTION_SERVICE)