diff --git a/src/event-cache/EventCache.ts b/src/event-cache/EventCache.ts index 6228bce9..fef776e9 100644 --- a/src/event-cache/EventCache.ts +++ b/src/event-cache/EventCache.ts @@ -99,13 +99,11 @@ export class EventCache { if (!this.enabled) { return; } - + this.sessionManager.getSession(); // refresh session if needed if (this.isCurrentUrlAllowed()) { - const session: Session = this.sessionManager.getSession(); - this.sessionManager.incrementSessionEventCount(); - - if (this.canRecord(session)) { + if (this.sessionManager.shouldSample()) { this.addRecordToCache(type, eventData); + this.sessionManager.countEvent(); } } }; @@ -118,7 +116,6 @@ export class EventCache { if (this.isCurrentUrlAllowed()) { return this.sessionManager.getSession(); } - return undefined; }; /** @@ -190,21 +187,13 @@ export class EventCache { return; } - this.sessionManager.incrementSessionEventCount(); + this.sessionManager.countEvent(); - if (this.canRecord(session)) { + if (this.sessionManager.shouldSample()) { this.addRecordToCache(type, eventData); } }; - private canRecord = (session: Session): boolean => { - return ( - session.record && - (session.eventCount <= this.config.sessionEventLimit || - this.config.sessionEventLimit <= 0) - ); - }; - /** * Add an event to the cache. * diff --git a/src/event-cache/__tests__/EventCache.test.ts b/src/event-cache/__tests__/EventCache.test.ts index bc80b845..75fdad03 100644 --- a/src/event-cache/__tests__/EventCache.test.ts +++ b/src/event-cache/__tests__/EventCache.test.ts @@ -1,7 +1,6 @@ import { EventCache } from '../EventCache'; import { advanceTo } from 'jest-date-mock'; import * as Utils from '../../test-utils/test-utils'; -import { SessionManager } from '../../sessions/SessionManager'; import { RumEvent } from '../../dispatch/dataplane'; import { DEFAULT_CONFIG, mockFetch } from '../../test-utils/test-utils'; import { INSTALL_MODULE, INSTALL_SCRIPT } from '../../utils/constants'; @@ -16,18 +15,20 @@ const getSession = jest.fn(() => ({ })); const getUserId = jest.fn(() => 'b'); const getAttributes = jest.fn(); -const incrementSessionEventCount = jest.fn(); +const countEvent = jest.fn(); const addSessionAttributes = jest.fn(); let samplingDecision = true; +let shouldSample = true; const isSampled = jest.fn().mockImplementation(() => samplingDecision); jest.mock('../../sessions/SessionManager', () => ({ SessionManager: jest.fn().mockImplementation(() => ({ getSession, getUserId, getAttributes, - incrementSessionEventCount, + countEvent, addSessionAttributes, - isSampled + isSampled, + shouldSample: jest.fn().mockImplementation(() => shouldSample) })) })); @@ -41,7 +42,7 @@ describe('EventCache tests', () => { beforeEach(() => { getSession.mockClear(); getUserId.mockClear(); - incrementSessionEventCount.mockClear(); + countEvent.mockClear(); }); test('record does nothing when cache is disabled', async () => { @@ -446,92 +447,6 @@ describe('EventCache tests', () => { expect(eventCache.isSessionSampled()).toBeTruthy(); }); - test('when session.record is false then event is not recorded', async () => { - // Init - const getSession = jest.fn(() => ({ sessionId: 'a', record: true })); - const getUserId = jest.fn(() => 'b'); - const incrementSessionEventCount = jest.fn(); - (SessionManager as any).mockImplementation(() => ({ - getSession, - getUserId, - incrementSessionEventCount - })); - - const EVENT1_SCHEMA = 'com.amazon.rum.event1'; - const eventCache: EventCache = Utils.createDefaultEventCache(); - - // Run - eventCache.getEventBatch(); - eventCache.recordEvent(EVENT1_SCHEMA, {}); - - // Assert - expect(eventCache.hasEvents()).toBeFalsy(); - }); - - test('when session.record is true then event is recorded', async () => { - // Init - const getSession = jest.fn(() => ({ - sessionId: 'a', - record: true, - eventCount: 1 - })); - const getUserId = jest.fn(() => 'b'); - const incrementSessionEventCount = jest.fn(); - (SessionManager as any).mockImplementation(() => ({ - getSession, - getUserId, - getAttributes, - incrementSessionEventCount - })); - - const EVENT1_SCHEMA = 'com.amazon.rum.event1'; - const eventCache: EventCache = Utils.createDefaultEventCache(); - - // Run - eventCache.getEventBatch(); - eventCache.recordEvent(EVENT1_SCHEMA, {}); - - // Assert - expect(eventCache.hasEvents()).toBeTruthy(); - }); - - test('when event limit is reached then recordEvent does not record events', async () => { - // Init - let eventCount = 0; - const getSession = jest.fn().mockImplementation(() => { - eventCount++; - return { - sessionId: 'a', - record: true, - eventCount - }; - }); - const getUserId = jest.fn(() => 'b'); - const incrementSessionEventCount = jest.fn(); - (SessionManager as any).mockImplementation(() => ({ - getSession, - getUserId, - getAttributes, - incrementSessionEventCount - })); - const EVENT1_SCHEMA = 'com.amazon.rum.event1'; - const config = { - ...DEFAULT_CONFIG, - ...{ - sessionEventLimit: 1 - } - }; - const eventCache: EventCache = Utils.createEventCache(config); - - // Run - eventCache.recordEvent(EVENT1_SCHEMA, {}); - eventCache.recordEvent(EVENT1_SCHEMA, {}); - eventCache.recordEvent(EVENT1_SCHEMA, {}); - - // Assert - expect(eventCache.getEventBatch().length).toEqual(1); - }); - test('when event is recorded then events subscribers are notified with parsed rum event', async () => { // Init const EVENT1_SCHEMA = 'com.amazon.rum.event1'; @@ -586,31 +501,29 @@ describe('EventCache tests', () => { expect(bus.dispatch).not.toHaveBeenCalled(); // eslint-disable-line }); - test('when event limit is zero then recordEvent records all events', async () => { + test('when session should not sample, then no events are recorded', async () => { // Init - const eventCount = 0; - const getSession = jest.fn(() => ({ sessionId: 'a', record: true })); - const getUserId = jest.fn(() => 'b'); - const incrementSessionEventCount = jest.fn(); - (SessionManager as any).mockImplementation(() => ({ - getSession, - getUserId, - getAttributes, - incrementSessionEventCount - })); + shouldSample = false; const EVENT1_SCHEMA = 'com.amazon.rum.event1'; - const config = { - ...DEFAULT_CONFIG, - ...{ - sessionEventLimit: 0 - } - }; - const eventCache: EventCache = Utils.createEventCache(config); + const eventCache: EventCache = Utils.createEventCache({ + ...DEFAULT_CONFIG + }); // Run eventCache.recordEvent(EVENT1_SCHEMA, {}); + expect(eventCache.getEventBatch()).toEqual([]); - // Assert - expect(eventCache.getEventBatch().length).toEqual(1); + shouldSample = true; + eventCache.recordEvent(EVENT1_SCHEMA, {}); + eventCache.recordEvent(EVENT1_SCHEMA, {}); + expect(eventCache.getEventBatch()).toHaveLength(2); + + shouldSample = false; + eventCache.recordEvent(EVENT1_SCHEMA, {}); + eventCache.recordEvent(EVENT1_SCHEMA, {}); + expect(eventCache.getEventBatch()).toEqual([]); + + // Restore + shouldSample = true; }); }); diff --git a/src/sessions/SessionManager.ts b/src/sessions/SessionManager.ts index ac33d712..82041d51 100644 --- a/src/sessions/SessionManager.ts +++ b/src/sessions/SessionManager.ts @@ -112,17 +112,14 @@ export class SessionManager { * Returns the session ID. If no session ID exists, one will be created. */ public getSession(): Session { - if (this.session.sessionId === NIL_UUID) { - // The session does not exist. Create a new one. - // If it is created before the page view is recorded, the session start event metadata will - // not have page attributes as the page does not exist yet. - this.createSession(); - } else if ( - this.session.sessionId !== NIL_UUID && + if ( + this.session.sessionId === NIL_UUID || new Date() > this.sessionExpiry ) { - // The session has expired. Create a new one. this.createSession(); + // The session does not exist or has expired.. Create a new one. + // If it is created before the page view is recorded, the session start event metadata will + // not have page attributes as the page does not exist yet. } return this.session; } @@ -149,11 +146,22 @@ export class SessionManager { return NIL_UUID; } - public incrementSessionEventCount() { + public countEvent() { this.session.eventCount++; this.renewSession(); } + public shouldSample(): boolean { + if (!this.isSampled()) { + return false; + } + + return ( + this.session.eventCount < this.config.sessionEventLimit || + this.config.sessionEventLimit <= 0 + ); + } + private initializeUser() { let userId = ''; this.userExpiry = new Date(); diff --git a/src/sessions/__tests__/SessionManager.test.ts b/src/sessions/__tests__/SessionManager.test.ts index d502816f..c78b2a22 100644 --- a/src/sessions/__tests__/SessionManager.test.ts +++ b/src/sessions/__tests__/SessionManager.test.ts @@ -22,7 +22,6 @@ import { DEFAULT_CONFIG, mockFetch } from '../../test-utils/test-utils'; -import { advanceTo } from 'jest-date-mock'; global.fetch = mockFetch; const NAVIGATION = 'navigation'; @@ -379,7 +378,7 @@ describe('SessionManager tests', () => { expect(userIdFromTracker).toEqual(NIL_UUID); }); - test('when the sessionId cookie expires then a new sessionId is created', async () => { + test('when the sessionId cookie expires then a new session is created', async () => { // Init const sessionManager = defaultSessionManager({ ...DEFAULT_CONFIG, @@ -387,11 +386,16 @@ describe('SessionManager tests', () => { }); const sessionOne = sessionManager.getSession(); + sessionManager.countEvent(); + sessionManager.countEvent(); + sessionManager.countEvent(); + await new Promise((resolve) => setTimeout(resolve, 10)); const sessionTwo = sessionManager.getSession(); // Assert expect(sessionOne.sessionId).not.toEqual(sessionTwo.sessionId); + expect(sessionTwo.eventCount).toEqual(0); }); test('When the sessionId cookie does not expire, sessionId remains the same', async () => { @@ -459,7 +463,7 @@ describe('SessionManager tests', () => { expect(mockRecord).toHaveBeenCalledTimes(0); }); - test('when sessionSampleRate is one then session.record is true', async () => { + test('when sessionSampleRate is one then should sample', async () => { // Init const sessionManager = defaultSessionManager({ ...DEFAULT_CONFIG, @@ -470,10 +474,10 @@ describe('SessionManager tests', () => { // Assert expect(session.record).toBeTruthy(); - expect(sessionManager.isSampled()).toBeTruthy(); + expect(sessionManager.isSampled()).toBe(true); }); - test('when sessionSampleRate is zero then session.record is false', async () => { + test('when sessionSampleRate is zero then should not sample', async () => { // Init const sessionManager = defaultSessionManager({ ...DEFAULT_CONFIG, @@ -484,7 +488,40 @@ describe('SessionManager tests', () => { // Assert expect(session.record).toBeFalsy(); - expect(sessionManager.isSampled()).toBeFalsy(); + expect(sessionManager.isSampled()).toBe(false); + expect(sessionManager.shouldSample()).toBe(false); + }); + + test('when sessionEventLimit is reached then should not sample ', async () => { + const sessionManager = defaultSessionManager({ + ...DEFAULT_CONFIG, + sessionSampleRate: 1, + allowCookies: true, + sessionEventLimit: 2 + }); + expect(sessionManager.shouldSample()).toBe(true); + sessionManager.countEvent(); + expect(sessionManager.shouldSample()).toBe(true); + sessionManager.countEvent(); + expect(sessionManager.shouldSample()).toBe(false); + sessionManager.countEvent(); + expect(sessionManager.shouldSample()).toBe(false); + }); + + test('when sessionEventLimit is zero then always should sample ', async () => { + const sessionManager = defaultSessionManager({ + ...DEFAULT_CONFIG, + sessionSampleRate: 1, + allowCookies: true, + sessionEventLimit: 0 + }); + expect(sessionManager.shouldSample()).toBe(true); + sessionManager.countEvent(); + expect(sessionManager.shouldSample()).toBe(true); + sessionManager.countEvent(); + expect(sessionManager.shouldSample()).toBe(true); + sessionManager.countEvent(); + expect(sessionManager.shouldSample()).toBe(true); }); test('when sessionId is nil then create new session with same sampling decision', async () => { @@ -588,7 +625,7 @@ describe('SessionManager tests', () => { expect(session.eventCount).toEqual(0); }); - test('when cookies are allowed then incrementSessionEventCount increments session.eventCount in cookie', async () => { + test('when cookies are allowed then countEvent increments session.eventCount in cookie', async () => { // Init const config = { ...DEFAULT_CONFIG, @@ -604,14 +641,14 @@ describe('SessionManager tests', () => { const sessionManager = defaultSessionManager(config); sessionManager.getSession(); - sessionManager.incrementSessionEventCount(); + sessionManager.countEvent(); const session = JSON.parse(atob(getCookie(SESSION_COOKIE_NAME))); // Assert expect(session.eventCount).toEqual(2); }); - test('when cookies are not allowed then incrementSessionEventCount increments session.eventCount in member', async () => { + test('when cookies are not allowed then countEvent increments session.eventCount in member', async () => { // Init const sessionManager = defaultSessionManager({ ...DEFAULT_CONFIG, @@ -619,7 +656,7 @@ describe('SessionManager tests', () => { }); sessionManager.getSession(); - sessionManager.incrementSessionEventCount(); + sessionManager.countEvent(); const session = sessionManager.getSession(); // Assert @@ -727,7 +764,7 @@ describe('SessionManager tests', () => { sessionManager.getSession(); const userIdFromCookie1 = getCookie(USER_COOKIE_NAME); config.allowCookies = true; - sessionManager.incrementSessionEventCount(); + sessionManager.countEvent(); const userIdFromCookie2 = getCookie(USER_COOKIE_NAME); // Assert