feat: use page visibilityState for browser responsiveness check (elas…

…tic#813)

* feat: use page visibilityState for browser responsiveness check

* chore: merge state and env

merge bootstrap files

* chore: address review

packages/rum-core/src/bootstrap.js

@@ -25,13 +25,13 @@
 
 import { isPlatformSupported, isBrowser } from './common/utils'
 import { patchAll } from './common/patching'
-import { bootstrapMetrics } from './performance-monitoring/metrics'
+import { state } from './state'
 
 let enabled = false
 export function bootstrap() {
   if (isPlatformSupported()) {
     patchAll()
-    bootstrapMetrics()
+    bootstrapPerf()
     enabled = true
   } else if (isBrowser) {
     /**
@@ -43,3 +43,19 @@ export function bootstrap() {
 
   return enabled
 }
+
+export function bootstrapPerf() {
+  if (document.visibilityState === 'hidden') {
+    state.lastHiddenStart = 0
+  }
+
+  window.addEventListener(
+    'visibilitychange',
+    () => {
+      if (document.visibilityState === 'hidden') {
+        state.lastHiddenStart = performance.now()
+      }
+    },
+    { capture: true }
+  )
+}

packages/rum-core/src/common/apm-server.js

@@ -36,7 +36,7 @@ import {
   compressTransaction,
   compressError
 } from './compress'
-import { __DEV__ } from '../env'
+import { __DEV__ } from '../state'
 
 /**
  * Throttling interval defaults to 60 seconds

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

@@ -129,11 +129,6 @@ const FIRST_CONTENTFUL_PAINT = 'first-contentful-paint'
 const LARGEST_CONTENTFUL_PAINT = 'largest-contentful-paint'
 const FIRST_INPUT = 'first-input'
 
-/**
- * Managed transaction configs
- */
-const BROWSER_RESPONSIVENESS_INTERVAL = 500
-
 /**
  * Event types sent to APM Server on the queue
  */
@@ -194,7 +189,6 @@ export {
   TEMPORARY_TYPE,
   USER_INTERACTION,
   TRANSACTION_TYPE_ORDER,
-  BROWSER_RESPONSIVENESS_INTERVAL,
   ERRORS,
   TRANSACTIONS,
   CONFIG_SERVICE,

packages/rum-core/src/common/service-factory.js

@@ -32,7 +32,7 @@ import {
   LOGGING_SERVICE,
   APM_SERVER
 } from './constants'
-import { __DEV__ } from '../env'
+import { __DEV__ } from '../state'
 
 const serviceCreators = {
   [CONFIG_SERVICE]: () => new ConfigService(),

packages/rum-core/src/opentracing/tracer.js

@@ -32,7 +32,7 @@ import {
 } from 'opentracing/lib/constants'
 import { Span as NoopSpan } from 'opentracing/lib/span'
 import { getTimeOrigin, find } from '../common/utils'
-import { __DEV__ } from '../env'
+import { __DEV__ } from '../state'
 import Span from './span'
 
 class Tracer extends otTracer {

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

@@ -105,7 +105,7 @@ function createLongTaskSpans(longtasks) {
 export function createFirstInputDelaySpan(fidEntries) {
   let firstInput = fidEntries[0]
 
-  if (firstInput && !metrics.wasHidden) {
+  if (firstInput) {
     const { startTime, processingStart } = firstInput
 
     const span = new Span('First Input Delay', FIRST_INPUT, { startTime })
@@ -292,17 +292,3 @@ export class PerfEntryRecorder {
     this.po.disconnect()
   }
 }
-
-export function bootstrapMetrics() {
-  metrics.wasHidden = document.visibilityState === 'hidden'
-
-  window.addEventListener(
-    'visibilitychange',
-    () => {
-      if (document.visibilityState === 'hidden') {
-        metrics.wasHidden = true
-      }
-    },
-    { capture: true, once: true }
-  )
-}

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

@@ -43,21 +43,18 @@ import {
   XMLHTTPREQUEST,
   EVENT_TARGET,
   HTTP_REQUEST_TYPE,
-  USER_INTERACTION,
-  PAGE_LOAD,
-  BROWSER_RESPONSIVENESS_INTERVAL
+  USER_INTERACTION
 } from '../common/constants'
 import {
   truncateModel,
   SPAN_MODEL,
   TRANSACTION_MODEL
 } from '../common/truncate'
-import { __DEV__ } from '../env'
+import { __DEV__ } from '../state'
 
 /**
  * Parameters used for Managed Transactions
  */
-const BROWSER_RESPONSIVENESS_BUFFER = 3
 const SIMILAR_SPAN_TO_TRANSACTION_RATIO = 0.05
 const TRANSACTION_DURATION_THRESHOLD = 60000
 
@@ -141,14 +138,6 @@ export function adjustTransactionSpans(transaction) {
   return transaction
 }
 
-export function checkBrowserResponsiveness(transaction, interval, buffer) {
-  const counter = transaction.browserResponsivenessCounter
-  const duration = transaction.duration()
-  const expectedCount = Math.floor(duration / interval)
-
-  return counter + buffer >= expectedCount
-}
-
 export default class PerformanceMonitoring {
   constructor(apmServer, configService, loggingService, transactionService) {
     this._apmServer = apmServer
@@ -378,30 +367,6 @@ export default class PerformanceMonitoring {
         }
         return false
       }
-
-      /**
-       * TODO: Refactor the type check with better logic
-       */
-      if (tr.type !== PAGE_LOAD) {
-        const wasBrowserResponsive = checkBrowserResponsiveness(
-          tr,
-          BROWSER_RESPONSIVENESS_INTERVAL,
-          BROWSER_RESPONSIVENESS_BUFFER
-        )
-
-        if (!wasBrowserResponsive) {
-          if (__DEV__) {
-            this._logginService.debug(
-              `transaction(${tr.id}, ${tr.name}) was discarded! Browser was not responsive enough during the transaction.`,
-              ' duration:',
-              duration,
-              ' browserResponsivenessCounter:',
-              tr.browserResponsivenessCounter
-            )
-          }
-          return false
-        }
-      }
     }
     return true
   }

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

@@ -38,7 +38,6 @@ import {
   NAME_UNKNOWN,
   TRANSACTION_START,
   TRANSACTION_END,
-  BROWSER_RESPONSIVENESS_INTERVAL,
   TEMPORARY_TYPE,
   TRANSACTION_TYPE_ORDER,
   LARGEST_CONTENTFUL_PAINT,
@@ -48,7 +47,7 @@ import {
   FIRST_INPUT
 } from '../common/constants'
 import { addTransactionContext } from '../common/context'
-import { __DEV__ } from '../env'
+import { __DEV__, state } from '../state'
 
 class TransactionService {
   constructor(logger, config) {
@@ -91,28 +90,6 @@ class TransactionService {
     this.currentTransaction = value
   }
 
-  ensureRespInterval(checkBrowserResponsiveness) {
-    const clearRespInterval = () => {
-      clearInterval(this.respIntervalId)
-      this.respIntervalId = undefined
-    }
-
-    if (checkBrowserResponsiveness) {
-      if (typeof this.respIntervalId === 'undefined') {
-        this.respIntervalId = setInterval(() => {
-          let tr = this.getCurrentTransaction()
-          if (tr) {
-            tr.browserResponsivenessCounter++
-          } else {
-            clearRespInterval()
-          }
-        }, BROWSER_RESPONSIVENESS_INTERVAL)
-      }
-    } else if (typeof this.respIntervalId !== 'undefined') {
-      clearRespInterval()
-    }
-  }
-
   createOptions(options) {
     const config = this._config.config
     let presetOptions = { transactionSampleRate: config.transactionSampleRate }
@@ -176,14 +153,12 @@ class TransactionService {
       tr = this.ensureCurrentTransaction(name, type, perfOptions)
     }
 
-    let checkBrowserResponsiveness = true
     if (tr.type === PAGE_LOAD) {
       if (!isRedefined) {
         this.recorder.start(LARGEST_CONTENTFUL_PAINT)
         this.recorder.start(PAINT)
         this.recorder.start(FIRST_INPUT)
       }
-      checkBrowserResponsiveness = false
       if (perfOptions.pageLoadTraceId) {
         tr.traceId = perfOptions.pageLoadTraceId
       }
@@ -212,8 +187,6 @@ class TransactionService {
       tr.captureTimings = true
     }
 
-    this.ensureRespInterval(checkBrowserResponsiveness)
-
     return tr
   }
 
@@ -262,6 +235,16 @@ class TransactionService {
     return Promise.resolve().then(
       () => {
         const { name, type } = tr
+        let { lastHiddenStart } = state
+
+        if (lastHiddenStart >= tr._start) {
+          if (__DEV__) {
+            this._logger.debug(
+              `transaction(${tr.id}, ${tr.name}, ${tr.type}) was discarded! The page was hidden during the transaction!`
+            )
+          }
+          return
+        }
 
         if (this.shouldIgnoreTransaction(name) || type === TEMPORARY_TYPE) {
           if (__DEV__) {

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

@@ -53,7 +53,6 @@ class Transaction extends SpanBase {
     this.breakdownTimings = []
 
     this.sampled = Math.random() <= this.options.transactionSampleRate
-    this.browserResponsivenessCounter = 0
   }
 
   addMarks(obj) {

packages/rum-core/src/env.js → packages/rum-core/src/state.js

@@ -26,3 +26,5 @@
 const __DEV__ = process.env.NODE_ENV !== 'production'
 
 export { __DEV__ }
+
+export const state = {}

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

@@ -28,7 +28,6 @@ import Transaction from '../../src/performance-monitoring/transaction'
 import Span from '../../src/performance-monitoring/span'
 import {
   groupSmallContinuouslySimilarSpans,
-  checkBrowserResponsiveness,
   adjustTransactionSpans
 } from '../../src/performance-monitoring/performance-monitoring'
 import { getGlobalConfig } from '../../../../dev-utils/test-config'
@@ -121,31 +120,6 @@ describe('PerformanceMonitoring', function() {
     )
   })
 
-  it('should filter transactions based on browser responsiveness', function() {
-    spyOn(logger, 'debug').and.callThrough()
-    expect(logger.debug).not.toHaveBeenCalled()
-    var tr = new Transaction('transaction', 'transaction', {
-      id: 212,
-      transactionSampleRate: 1,
-      managed: true,
-      startTime: 1
-    })
-    var span = tr.startSpan('test span', 'test span type')
-    span.end()
-    tr.end(3501)
-
-    tr.browserResponsivenessCounter = 3
-    var wasBrowserResponsive = performanceMonitoring.filterTransaction(tr)
-    expect(wasBrowserResponsive).toBe(false)
-    expect(logger.debug).toHaveBeenCalledWith(
-      'transaction(212, transaction) was discarded! Browser was not responsive enough during the transaction.',
-      ' duration:',
-      3500,
-      ' browserResponsivenessCounter:',
-      3
-    )
-  })
-
   it('should initialize and add transaction to the queue', async () => {
     performanceMonitoring.init()
     spyOn(apmServer, 'addTransaction')
@@ -816,32 +790,6 @@ describe('PerformanceMonitoring', function() {
       expect(grouped[1].name).toBe('another-name')
     })
 
-    it('should calculate browser responsiveness', function() {
-      const tr = new Transaction('transaction', 'transaction', {
-        startTime: 1
-      })
-      tr.end(400)
-
-      tr.browserResponsivenessCounter = 0
-      var resp = checkBrowserResponsiveness(tr, 500, 2)
-      expect(resp).toBe(true)
-
-      tr._end = 1001
-      tr.browserResponsivenessCounter = 2
-      resp = checkBrowserResponsiveness(tr, 500, 2)
-      expect(resp).toBe(true)
-
-      tr._end = 1601
-      tr.browserResponsivenessCounter = 2
-      resp = checkBrowserResponsiveness(tr, 500, 2)
-      expect(resp).toBe(true)
-
-      tr._end = 3001
-      tr.browserResponsivenessCounter = 3
-      resp = checkBrowserResponsiveness(tr, 500, 2)
-      expect(resp).toBe(false)
-    })
-
     it('should reset spans for unsampled transactions', function() {
       const tr = new Transaction('unsampled', 'test', {
         transactionSampleRate: 0,