Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: migrate EventCache.canRecord to SessionManager.shouldSample #504

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

chore: migrate EventCache.canRecord to SessionManager.shouldSample #504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84bb410
chore: consolidate sampling logic in SessionManager
williazz Feb 7, 2024
9567a93
rename verbose method
williazz Feb 8, 2024
f54ee2e
fix off by one error
williazz Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 5 additions & 16 deletions src/event-cache/EventCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
};
Expand All @@ -118,7 +116,6 @@ export class EventCache {
if (this.isCurrentUrlAllowed()) {
return this.sessionManager.getSession();
}
return undefined;
};

/**
Expand Down Expand Up @@ -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.
*
Expand Down
135 changes: 24 additions & 111 deletions src/event-cache/__tests__/EventCache.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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)
}))
}));

Expand All @@ -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 () => {
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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;
});
});
26 changes: 17 additions & 9 deletions src/sessions/SessionManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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();
Expand Down
Loading
Loading