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.

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

feat: add sessionLimitOverride always record by type #505

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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(type)) {
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
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;
});
});
2 changes: 2 additions & 0 deletions src/orchestration/Orchestration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { PageViewPlugin } from '../plugins/event-plugins/PageViewPlugin';
import { PageAttributes } from '../sessions/PageManager';
import { INSTALL_MODULE } from '../utils/constants';
import EventBus, { Topic } from '../event-bus/EventBus';
import { CountByType } from '../sessions/SessionManager';

const DEFAULT_REGION = 'us-west-2';
const DEFAULT_ENDPOINT = `https://dataplane.rum.${DEFAULT_REGION}.amazonaws.com`;
Expand Down Expand Up @@ -142,6 +143,7 @@ export interface Config {
routeChangeComplete: number;
routeChangeTimeout: number;
sessionEventLimit: number;
sessionEventLimitOverride?: CountByType;
sessionLengthSeconds: number;
sessionSampleRate: number;
/**
Expand Down
38 changes: 29 additions & 9 deletions src/sessions/SessionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ export type RecordSessionInitEvent = (
eventData: object
) => void;

/**
* Represents a mapping of RUM event types to their respective session count
*/
export type CountByType = {
[key: string]: number;
};

export type Session = {
sessionId: string;
record: boolean;
Expand Down Expand Up @@ -112,17 +119,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 +153,27 @@ export class SessionManager {
return NIL_UUID;
}

public incrementSessionEventCount() {
public countEvent() {
this.session.eventCount++;
this.renewSession();
}

public shouldSample(type?: string): boolean {
if (!this.isSampled()) {
return false;
}

// Handle events with always record configuration
if (type && this.config.sessionEventLimitOverride?.[type] === 0) {
return true;
}

return (
this.session.eventCount < this.config.sessionEventLimit ||
this.config.sessionEventLimit <= 0
);
}

private initializeUser() {
let userId = '';
this.userExpiry = new Date();
Expand Down
Loading
Loading