From d8b7c32b6eb339182717dc3ad517bcfe85fb050a Mon Sep 17 00:00:00 2001 From: David Newell Date: Wed, 11 Oct 2023 13:44:54 +0100 Subject: [PATCH] fix: account for negative timestamps from rrweb (#17849) --- .../src/worker/ingestion/process-event.ts | 18 +++++++-- .../tests/main/process-event.test.ts | 40 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/plugin-server/src/worker/ingestion/process-event.ts b/plugin-server/src/worker/ingestion/process-event.ts index f378c0e6c1770..1517b3ebebac3 100644 --- a/plugin-server/src/worker/ingestion/process-event.ts +++ b/plugin-server/src/worker/ingestion/process-event.ts @@ -397,6 +397,19 @@ export const gatherConsoleLogEvents = ( return consoleLogEntries } +export const getTimestampsFrom = (events: RRWebEvent[]): ClickHouseTimestamp[] => + events + // from millis expects a number and handles unexpected input gracefully so we have to do some filtering + // since we're accepting input over the API and have seen very unexpected values in the past + // we want to be very careful here before converting to a DateTime + // TODO we don't really want to support timestamps of 1, + // but we don't currently filter out based on date of RRWebEvents being too far in the past + .filter((e) => (e?.timestamp || -1) > 0) + .map((e) => DateTime.fromMillis(e.timestamp)) + .filter((e) => e.isValid) + .map((e) => castTimestampOrNow(e, TimestampFormat.ClickHouse)) + .sort() + export const createSessionReplayEvent = ( uuid: string, team_id: number, @@ -404,10 +417,7 @@ export const createSessionReplayEvent = ( session_id: string, events: RRWebEvent[] ) => { - const timestamps = events - .filter((e) => !!e?.timestamp) - .map((e) => castTimestampOrNow(DateTime.fromMillis(e.timestamp), TimestampFormat.ClickHouse)) - .sort() + const timestamps = getTimestampsFrom(events) // but every event where chunk index = 0 must have an eventsSummary if (events.length === 0 || timestamps.length === 0) { diff --git a/plugin-server/tests/main/process-event.test.ts b/plugin-server/tests/main/process-event.test.ts index c222007d2cce6..786acbbd7698e 100644 --- a/plugin-server/tests/main/process-event.test.ts +++ b/plugin-server/tests/main/process-event.test.ts @@ -36,6 +36,7 @@ import { createSessionReplayEvent, EventsProcessor, gatherConsoleLogEvents, + getTimestampsFrom, SummarizedSessionRecordingEvent, } from '../../src/worker/ingestion/process-event' import { delayUntilEventIngested, resetTestDatabaseClickhouse } from '../helpers/clickhouse' @@ -1404,6 +1405,31 @@ const sessionReplayEventTestCases: { message_count: 1, }, }, + { + snapshotData: { + events_summary: [ + // a negative timestamp is ignored + { timestamp: 1682449093000, type: 3, data: { source: 2 }, windowId: '1' }, + { timestamp: 1682449095000, type: 3, data: { source: 2 }, windowId: '1' }, + { timestamp: -922167545571, type: 3, data: { source: 2 }, windowId: '1' }, + ], + }, + expected: { + click_count: 3, + keypress_count: 0, + mouse_activity_count: 3, + first_url: null, + first_timestamp: '2023-04-25 18:58:13.000', + last_timestamp: '2023-04-25 18:58:15.000', + active_milliseconds: 1, + console_log_count: 0, + console_warn_count: 0, + console_error_count: 0, + size: 217, + event_count: 3, + message_count: 1, + }, + }, { snapshotData: { events_summary: [ @@ -1483,6 +1509,20 @@ test(`snapshot event with no event summary timestamps is ignored`, () => { }).toThrowError() }) +test.each([ + { events: [], expectedTimestamps: [] }, + { events: [{ without: 'timestamp property' } as unknown as RRWebEvent], expectedTimestamps: [] }, + { events: [{ timestamp: undefined } as unknown as RRWebEvent], expectedTimestamps: [] }, + { events: [{ timestamp: null } as unknown as RRWebEvent], expectedTimestamps: [] }, + { events: [{ timestamp: 'what about a string' } as unknown as RRWebEvent], expectedTimestamps: [] }, + // we have seen negative timestamps from clients 🙈 + { events: [{ timestamp: -1 } as unknown as RRWebEvent], expectedTimestamps: [] }, + { events: [{ timestamp: 0 } as unknown as RRWebEvent], expectedTimestamps: [] }, + { events: [{ timestamp: 1 } as unknown as RRWebEvent], expectedTimestamps: ['1970-01-01 00:00:00.001'] }, +])('timestamps from rrweb events', ({ events, expectedTimestamps }) => { + expect(getTimestampsFrom(events)).toEqual(expectedTimestamps) +}) + function consoleMessageFor(payload: any[]) { return { timestamp: 1682449093469,