feat: Persist logging flag in session

src/common/session/session-entity.js

@@ -12,6 +12,7 @@ import { handle } from '../event-emitter/handle'
 import { SUPPORTABILITY_METRIC_CHANNEL } from '../../features/metrics/constants'
 import { FEATURE_NAMES } from '../../loaders/features/features'
 import { windowAddEventListener } from '../event-listener/event-listener-opts'
+import { LOGGING_MODE } from '../../features/logging/constants'
 
 // this is what can be stored in local storage (not enforced but probably should be)
 // these values should sync between local storage and the parent class props
@@ -24,6 +25,7 @@ const model = {
   sessionReplaySentFirstChunk: false,
   sessionTraceMode: MODE.OFF,
   traceHarvestStarted: false,
+  loggingMode: LOGGING_MODE.OFF,
   serverTimeDiff: null, // set by TimeKeeper; "undefined" value will not be stringified and stored but "null" will
   custom: {}
 }

src/features/logging/aggregate/index.js

@@ -10,21 +10,45 @@
 import { isValidLogLevel } from '../shared/utils'
 import { applyFnToProps } from '../../../common/util/traverse'
 import { MAX_PAYLOAD_SIZE } from '../../../common/constants/agent-constants'
-import { FEATURE_TO_ENDPOINT } from '../../../loaders/features/features'
+import { FEATURE_NAMES, FEATURE_TO_ENDPOINT } from '../../../loaders/features/features'
+import { SESSION_EVENT_TYPES, SESSION_EVENTS } from '../../../common/session/constants'
+import { ABORT_REASONS } from '../../session_replay/constants'
 
 export class Aggregate extends AggregateBase {
   static featureName = FEATURE_NAME
   constructor (agentRef) {
     super(agentRef, FEATURE_NAME)
     this.harvestTimeSeconds = agentRef.init.logging.harvestTimeSeconds
 
+    // The SessionEntity class can emit a message indicating the session was cleared and reset (expiry, inactivity). This feature must abort and never resume if that occurs.
+    this.ee.on(SESSION_EVENTS.RESET, () => {
+      this.abort(ABORT_REASONS.RESET)
+    })
+
+    this.ee.on(SESSION_EVENTS.UPDATE, (type, data) => {
+      if (this.blocked || type !== SESSION_EVENT_TYPES.CROSS_TAB) return
+      if (this.mode !== LOGGING_MODE.OFF && data.loggingMode === LOGGING_MODE.OFF) this.abort(ABORT_REASONS.CROSS_TAB)
+      else this.mode = data.loggingMode
+    })
+
     this.waitForFlags(['log']).then(([loggingMode]) => {
-      if (!loggingMode) {
+      const session = this.agentRef.runtime.session ?? {}
+      if (this.loggingMode === LOGGING_MODE.OFF || (session.isNew && loggingMode === LOGGING_MODE.OFF)) {
         this.blocked = true
         this.deregisterDrain()
         return
       }
-      this.loggingMode = loggingMode
+      if (session.isNew || !this.isSessionTrackingEnabled) {
+        this.loggingMode = loggingMode
+
+        if (this.isSessionTrackingEnabled) {
+          this.syncWithSessionManager({
+            loggingMode: this.loggingMode
+          })
+        }
+      } else {
+        this.loggingMode = session.state.loggingMode
+      }
       this.scheduler = new HarvestScheduler(FEATURE_TO_ENDPOINT[this.featureName], {
         onFinished: (result) => this.postHarvestCleanup(result.sent && result.retry),
         retryDelay: this.harvestTimeSeconds,
@@ -123,4 +147,14 @@
   queryStringsBuilder () {
     return { browser_monitoring_key: this.agentRef.info.licenseKey }
   }
+
+  /** Abort the feature, once aborted it will not resume */
+  abort (reason = {}) {
+    handle(SUPPORTABILITY_METRIC_CHANNEL, [`Logging/Abort/${reason.sm}`], undefined, FEATURE_NAMES.logging, this.ee)
+    this.blocked = true
+    this.loggingMode = LOGGING_MODE.OFF
+    this.syncWithSessionManager({ loggingMode: this.loggingMode })
+    this.events.clear()
+    this.deregisterDrain()
+  }
 }

src/features/session_replay/aggregate/index.js

@@ -73,7 +73,7 @@
     this.ee.on(SESSION_EVENTS.UPDATE, (type, data) => {
       if (!this.recorder || !this.initialized || this.blocked || type !== SESSION_EVENT_TYPES.CROSS_TAB) return
       if (this.mode !== MODE.OFF && data.sessionReplayMode === MODE.OFF) this.abort(ABORT_REASONS.CROSS_TAB)
-      this.mode = data.sessionReplay
+      this.mode = data.sessionReplayMode
     })
 
     // Bespoke logic for blobs endpoint.
@@ -147,16 +147,14 @@
       this.scheduler.startTimer(this.harvestTimeSeconds)
       this.syncWithSessionManager({ sessionReplayMode: this.mode })
     } else {
-      this.initializeRecording(false, true, true)
+      this.initializeRecording(false, true)
     }
   }
 
   /**
-   * Evaluate entitlements and sampling before starting feature mechanics, importing and configuring recording library, and setting storage state
-   * @param {boolean} entitlements - the true/false state of the "sr" flag from RUM response
-   * @param {boolean} errorSample - the true/false state of the error sampling decision
-   * @param {boolean} fullSample - the true/false state of the full sampling decision
-   * @param {boolean} ignoreSession - whether to force the method to ignore the session state and use just the sample flags
+   * Evaluate entitlements (which already accounts for sampling) before starting feature mechanics, importing and configuring recording library, and setting storage state
+   * @param {boolean} srMode - the true/false state of the "sr" flag from RUM response
+   * @param {boolean} ignoreSession - whether to force the method to ignore the session state and use just the "sr" flag
    * @returns {void}
    */
   async initializeRecording (srMode, ignoreSession) {
@@ -394,8 +392,4 @@
     this.ee.emit('REPLAY_ABORTED')
     while (this.recorder?.getEvents().events.length) this.recorder?.clearBuffer?.()
   }
-
-  syncWithSessionManager (state = {}) {
-    this.agentRef.runtime.session.write(state)
-  }
 }

src/features/utils/aggregate-base.js

@@ -7,6 +7,7 @@ import { activatedFeatures } from '../../common/util/feature-flags'
 import { Obfuscator } from '../../common/util/obfuscate'
 import { EventBuffer } from './event-buffer'
 import { FEATURE_NAMES } from '../../loaders/features/features'
+import { canEnableSessionTracking } from './feature-gates'
 
 export class AggregateBase extends FeatureBase {
   constructor (agentRef, featureName) {
@@ -18,6 +19,7 @@ export class AggregateBase extends FeatureBase {
     else if (![FEATURE_NAMES.pageViewEvent, FEATURE_NAMES.sessionTrace].includes(this.featureName)) this.events = new EventBuffer()
     this.checkConfiguration(agentRef)
     this.obfuscator = agentRef.runtime.obfuscator
+    this.isSessionTrackingEnabled = canEnableSessionTracking(this.agentIdentifier) && this.agentRef.runtime.session
   }
 
   /**
@@ -117,4 +119,10 @@ export class AggregateBase extends FeatureBase {
       existingAgent.runtime.obfuscator = new Obfuscator(this.agentIdentifier)
     }
   }
+
+  syncWithSessionManager (state = {}) {
+    if (this.isSessionTrackingEnabled) {
+      this.agentRef.runtime.session.write(state)
+    }
+  }
 }

tests/components/session-entity.test.js

@@ -2,6 +2,7 @@ import { PREFIX } from '../../src/common/session/constants'
 import { SessionEntity } from '../../src/common/session/session-entity'
 import { LocalMemory, model } from './session-helpers'
 import * as runtimeModule from '../../src/common/constants/runtime'
+import { buildExpectedSessionState } from '../specs/util/helpers'
 
 jest.useFakeTimers()
 
@@ -33,28 +34,13 @@ describe('constructor', () => {
       inactiveTimer: expect.any(Object),
       isNew: expect.any(Boolean),
       storage: expect.any(Object),
-      state: expect.objectContaining({
-        value: expect.any(String),
-        expiresAt: expect.any(Number),
-        inactiveAt: expect.any(Number),
-        sessionReplayMode: expect.any(Number),
-        sessionReplaySentFirstChunk: expect.any(Boolean),
-        sessionTraceMode: expect.any(Number)
-      })
+      state: expect.objectContaining(buildExpectedSessionState())
     })
   })
 
   test('can use sane defaults', () => {
     const session = new SessionEntity({ agentIdentifier, key, storage })
-    expect(session.state).toEqual(expect.objectContaining({
-      value: expect.any(String),
-      expiresAt: expect.any(Number),
-      inactiveAt: expect.any(Number),
-      updatedAt: expect.any(Number),
-      sessionReplayMode: expect.any(Number),
-      sessionReplaySentFirstChunk: expect.any(Boolean),
-      sessionTraceMode: expect.any(Number)
-    }))
+    expect(session.state).toEqual(expect.objectContaining(buildExpectedSessionState()))
   })
 
   test('expiresAt is the correct future timestamp - new session', () => {
@@ -110,47 +96,23 @@ describe('constructor', () => {
     // missing required fields
     const storage = new LocalMemory({ [`${PREFIX}_${key}`]: { invalid_fields: true } })
     const session = new SessionEntity({ agentIdentifier, key, storage })
-    expect(session.state).toEqual(expect.objectContaining({
-      value: expect.any(String),
-      expiresAt: expect.any(Number),
-      inactiveAt: expect.any(Number),
-      updatedAt: expect.any(Number),
-      sessionReplayMode: expect.any(Number),
-      sessionReplaySentFirstChunk: expect.any(Boolean),
-      sessionTraceMode: expect.any(Number)
-    }))
+    expect(session.state).toEqual(expect.objectContaining(buildExpectedSessionState()))
   })
 
   test('expired expiresAt value in storage sets new defaults', () => {
     const now = Date.now()
     jest.setSystemTime(now)
     const storage = new LocalMemory({ [`${PREFIX}_${key}`]: { value, expiresAt: now - 100, inactiveAt: Infinity } })
     const session = new SessionEntity({ agentIdentifier, key, storage })
-    expect(session.state).toEqual(expect.objectContaining({
-      value: expect.any(String),
-      expiresAt: expect.any(Number),
-      inactiveAt: expect.any(Number),
-      updatedAt: expect.any(Number),
-      sessionReplayMode: expect.any(Number),
-      sessionReplaySentFirstChunk: expect.any(Boolean),
-      sessionTraceMode: expect.any(Number)
-    }))
+    expect(session.state).toEqual(expect.objectContaining(buildExpectedSessionState()))
   })
 
   test('expired inactiveAt value in storage sets new defaults', () => {
     const now = Date.now()
     jest.setSystemTime(now)
     const storage = new LocalMemory({ [`${PREFIX}_${key}`]: { value, inactiveAt: now - 100, expiresAt: Infinity } })
     const session = new SessionEntity({ agentIdentifier, key, storage })
-    expect(session.state).toEqual(expect.objectContaining({
-      value: expect.any(String),
-      expiresAt: expect.any(Number),
-      inactiveAt: expect.any(Number),
-      updatedAt: expect.any(Number),
-      sessionReplayMode: expect.any(Number),
-      sessionReplaySentFirstChunk: expect.any(Boolean),
-      sessionTraceMode: expect.any(Number)
-    }))
+    expect(session.state).toEqual(expect.objectContaining(buildExpectedSessionState()))
   })
 })
 
@@ -212,14 +174,7 @@ describe('read()', () => {
     const newSession = new SessionEntity({ agentIdentifier, key, storage, expiresMs: 10 })
     expect(newSession.isNew).toBeTruthy()
 
-    expect(newSession.read()).toEqual(expect.objectContaining({
-      value: expect.any(String),
-      expiresAt: expect.any(Number),
-      inactiveAt: expect.any(Number),
-      sessionReplayMode: expect.any(Number),
-      sessionReplaySentFirstChunk: expect.any(Boolean),
-      sessionTraceMode: expect.any(Number)
-    }))
+    expect(newSession.read()).toEqual(expect.objectContaining(buildExpectedSessionState()))
   })
 
   test('"pre-existing" sessions get data from read()', () => {

tests/components/session-helpers.js

@@ -38,6 +38,7 @@ export const model = {
   sessionReplaySentFirstChunk: false,
   sessionTraceMode: 0,
   traceHarvestStarted: false,
+  loggingMode: 0,
   serverTimeDiff: null,
   custom: {}
 }

tests/specs/harvesting/disable-harvesting.e2e.js

@@ -1,5 +1,6 @@
 import { testBlobTraceRequest, testErrorsRequest, testEventsRequest, testInsRequest, testLogsRequest, testMetricsRequest, testRumRequest } from '../../../tools/testing-server/utils/expect-tests'
 import { rumFlags } from '../../../tools/testing-server/constants'
+import { LOGGING_MODE } from '../../../src/features/logging/constants'
 
 describe('disable harvesting', () => {
   it('should disable harvesting metrics and errors when err entitlement is 0', async () => {
@@ -61,10 +62,13 @@ describe('disable harvesting', () => {
   })
 
   it('should disable harvesting console logs when log entitlement is 0', async () => {
+    // logging mode will only take effect on a fresh session
+    await browser.destroyAgentSession()
+
     const logsCapture = await browser.testHandle.createNetworkCaptures('bamServer', { test: testLogsRequest })
     await browser.testHandle.scheduleReply('bamServer', {
       test: testRumRequest,
-      body: JSON.stringify(rumFlags({ log: 0 }))
+      body: JSON.stringify(rumFlags({ log: LOGGING_MODE.OFF }))
     })
 
     const [logsHarvests] = await Promise.all([

tests/specs/harvesting/index.e2e.js

@@ -1,6 +1,6 @@
 import { faker } from '@faker-js/faker'
 import { testExpectedTrace } from '../util/helpers'
-import { testAjaxEventsRequest, testAjaxTimeSlicesRequest, testBlobTraceRequest, testErrorsRequest, testInsRequest, testInteractionEventsRequest, testRumRequest, testTimingEventsRequest } from '../../../tools/testing-server/utils/expect-tests'
+import { testAjaxEventsRequest, testAjaxTimeSlicesRequest, testBlobTraceRequest, testErrorsRequest, testInsRequest, testInteractionEventsRequest, testLogsRequest, testRumRequest, testTimingEventsRequest } from '../../../tools/testing-server/utils/expect-tests'
 
 describe('harvesting', () => {
   let rumCapture
@@ -11,17 +11,19 @@ describe('harvesting', () => {
   let insightsCapture
   let interactionEventsCapture
   let errorMetricsCapture
+  let loggingEventsCapture
 
   beforeEach(async () => {
-    [rumCapture, timingEventsCapture, ajaxEventsCapture, ajaxMetricsCapture, traceCapture, insightsCapture, interactionEventsCapture, errorMetricsCapture] = await browser.testHandle.createNetworkCaptures('bamServer', [
+    [rumCapture, timingEventsCapture, ajaxEventsCapture, ajaxMetricsCapture, traceCapture, insightsCapture, interactionEventsCapture, errorMetricsCapture, loggingEventsCapture] = await browser.testHandle.createNetworkCaptures('bamServer', [
       { test: testRumRequest },
       { test: testTimingEventsRequest },
       { test: testAjaxEventsRequest },
       { test: testAjaxTimeSlicesRequest },
       { test: testBlobTraceRequest },
       { test: testInsRequest },
       { test: testInteractionEventsRequest },
-      { test: testErrorsRequest }
+      { test: testErrorsRequest },
+      { test: testLogsRequest }
     ])
   })
 
@@ -315,6 +317,28 @@ describe('harvesting', () => {
     interactionEventsHarvests.forEach(harvest => expect(harvest.request.query.s).toEqual('0'))
   })
 
+  it('should not contain session data in the logs request when cookies_enabled is false', async () => {
+    const testURL = await browser.testHandle.assetURL('obfuscate-pii.html', {
+      init: {
+        privacy: {
+          cookies_enabled: false
+        }
+      }
+    })
+
+    const [
+      loggingEventsHarvests
+    ] = await Promise.all([
+      loggingEventsCapture.waitForResult({ totalCount: 1 }),
+      browser.url(testURL)
+        .then(() => browser.waitForAgentLoad())
+    ])
+
+    loggingEventsHarvests.forEach(harvest => {
+      expect(harvest.request.query.session).toBeUndefined()
+    })
+  })
+
   it('should not harvest features when there is no data', async () => {
     const [
       insightsHarvests,

tests/specs/logging/harvesting.e2e.js

@@ -11,12 +11,18 @@ describe('logging harvesting', () => {
     })
   })
 
+  afterEach(async () => {
+    // logging mode is sticky to the session, so we need to reset before the next test
+    await browser.destroyAgentSession()
+  })
+
   const mockRumResponse = async (logLevel) => {
     await browser.testHandle.scheduleReply('bamServer', {
       test: testRumRequest,
       body: JSON.stringify(rumFlags({ log: logLevel }))
     })
   }
+
   const checkPayload = (actualPayload, expectedPayload) => {
     expect(actualPayload).toContainAllKeys(['common', 'logs'])
     expect(actualPayload.common).toEqual(expectedPayload.common)
@@ -70,8 +76,8 @@ describe('logging harvesting', () => {
         it(`should harvest expected logs - ${type} pre load - logging mode: ${mode}`, async () => {
           await mockRumResponse(logLevel)
           const [[{ request: { body } }]] = await Promise.all([
-            logsCapture.waitForResult({ totalCount: 1 }),
-            browser.url(await browser.testHandle.assetURL(`logs-${type}-pre-load.html`))
+            logsCapture.waitForResult({ totalCount: 1, timeout: 15000 }),
+            await browser.url(await browser.testHandle.assetURL(`logs-${type}-pre-load.html`))
           ])
 
           const actualPayload = JSON.parse(body)
@@ -81,7 +87,7 @@ describe('logging harvesting', () => {
         it(`should harvest expected logs - ${type} post load - logging mode: ${mode}`, async () => {
           await mockRumResponse(logLevel)
           const [[{ request: { body } }]] = await Promise.all([
-            logsCapture.waitForResult({ totalCount: 1 }),
+            logsCapture.waitForResult({ totalCount: 1, timeout: 15000 }),
             browser.url(await browser.testHandle.assetURL(`logs-${type}-post-load.html`))
           ])
           const actualPayload = JSON.parse(body)