From 46bb20b9e12f8d3d5002d2587d976f4f6d606de7 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Thu, 25 Apr 2024 14:18:08 -0300 Subject: [PATCH 1/8] feat(shared): Introduce client-side caching for telemetry events --- .changeset/rotten-days-whisper.md | 5 +++ packages/shared/src/telemetry/clientCache.ts | 41 +++++++++++++++++++ packages/shared/src/telemetry/collector.ts | 24 +++++++---- .../src/telemetry/events/component-mounted.ts | 4 ++ .../src/telemetry/events/method-called.ts | 6 +++ packages/shared/src/telemetry/types.ts | 14 +++++++ 6 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 .changeset/rotten-days-whisper.md create mode 100644 packages/shared/src/telemetry/clientCache.ts diff --git a/.changeset/rotten-days-whisper.md b/.changeset/rotten-days-whisper.md new file mode 100644 index 00000000000..774035e6164 --- /dev/null +++ b/.changeset/rotten-days-whisper.md @@ -0,0 +1,5 @@ +--- +'@clerk/shared': patch +--- + +Introduce client-side caching to TelemetryCollector diff --git a/packages/shared/src/telemetry/clientCache.ts b/packages/shared/src/telemetry/clientCache.ts new file mode 100644 index 00000000000..8be5a9f4852 --- /dev/null +++ b/packages/shared/src/telemetry/clientCache.ts @@ -0,0 +1,41 @@ +import type { TelemetryClientCacheOptions } from './types'; + +type TtlInMilliseconds = number; + +const DEFAULT_CACHE_TTL_MS = 86400000; // 24 hours + +export class TelemetryClientCache { + #key: TelemetryClientCacheOptions['key']; + #cacheTtl: NonNullable; + + constructor(options: TelemetryClientCacheOptions) { + this.#key = options.key; + this.#cacheTtl = options.cacheTtl ?? DEFAULT_CACHE_TTL_MS; + } + + cacheAndRetrieve() { + const now = Date.now(); + const item = this.#getItem(); + + if (!item) { + localStorage.setItem(this.#key, now.toString()); + } + + const isExpired = item && now - item > this.#cacheTtl; + if (isExpired) { + localStorage.removeItem(this.#key); + } + + return !!item; + } + + #getItem(): TtlInMilliseconds | undefined { + const cacheString = localStorage.getItem(this.#key); + + if (!cacheString) { + return; + } + + return JSON.parse(cacheString); + } +} diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index 2122c7e2da0..48df1d3b1c2 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -110,7 +110,7 @@ export class TelemetryCollector { this.#logEvent(preparedPayload.event, preparedPayload); - if (!this.#shouldRecord(event.eventSamplingRate)) { + if (!this.#shouldRecord(event)) { return; } @@ -119,13 +119,23 @@ export class TelemetryCollector { this.#scheduleFlush(); } - #shouldRecord(eventSamplingRate?: number): boolean { - const randomSeed = Math.random(); - const shouldBeSampled = - randomSeed <= this.#config.samplingRate && - (typeof eventSamplingRate === 'undefined' || randomSeed <= eventSamplingRate); + #shouldRecord(event: TelemetryEventRaw): boolean { + return this.isEnabled && !this.isDebug && this.#shouldBeSampled(event); + } + + #shouldBeSampled({ eventSamplingRate, eventSamplingClientCache }: TelemetryEventRaw): boolean { + if (typeof window === 'undefined' || !eventSamplingRate) { + const randomSeed = Math.random(); + + return ( + randomSeed <= this.#config.samplingRate && + (typeof eventSamplingRate === 'undefined' || randomSeed <= eventSamplingRate) + ); + } + + const isCached = eventSamplingClientCache?.cacheAndRetrieve(); - return this.isEnabled && !this.isDebug && shouldBeSampled; + return !isCached; } #scheduleFlush(): void { diff --git a/packages/shared/src/telemetry/events/component-mounted.ts b/packages/shared/src/telemetry/events/component-mounted.ts index 8649bf1a113..1f6f3556f1a 100644 --- a/packages/shared/src/telemetry/events/component-mounted.ts +++ b/packages/shared/src/telemetry/events/component-mounted.ts @@ -1,3 +1,4 @@ +import { TelemetryClientCache } from '../clientCache'; import type { TelemetryEventRaw } from '../types'; const EVENT_COMPONENT_MOUNTED = 'COMPONENT_MOUNTED' as const; @@ -20,6 +21,9 @@ export function eventComponentMounted( ): TelemetryEventRaw { return { event: EVENT_COMPONENT_MOUNTED, + eventSamplingClientCache: new TelemetryClientCache({ + key: `${EVENT_COMPONENT_MOUNTED}:${component}`, + }), eventSamplingRate: EVENT_SAMPLING_RATE, payload: { component, diff --git a/packages/shared/src/telemetry/events/method-called.ts b/packages/shared/src/telemetry/events/method-called.ts index 0c3b6a49338..76593dffa5d 100644 --- a/packages/shared/src/telemetry/events/method-called.ts +++ b/packages/shared/src/telemetry/events/method-called.ts @@ -1,6 +1,8 @@ +import { TelemetryClientCache } from '../clientCache'; import type { TelemetryEventRaw } from '../types'; const EVENT_METHOD_CALLED = 'METHOD_CALLED' as const; +const EVENT_SAMPLING_RATE = 0.1; type EventMethodCalled = { method: string; @@ -15,6 +17,10 @@ export function eventMethodCalled( ): TelemetryEventRaw { return { event: EVENT_METHOD_CALLED, + eventSamplingRate: EVENT_SAMPLING_RATE, + eventSamplingClientCache: new TelemetryClientCache({ + key: `${EVENT_METHOD_CALLED}:${method}`, + }), payload: { method, ...payload, diff --git a/packages/shared/src/telemetry/types.ts b/packages/shared/src/telemetry/types.ts index e594590db58..d6e0915526c 100644 --- a/packages/shared/src/telemetry/types.ts +++ b/packages/shared/src/telemetry/types.ts @@ -1,5 +1,7 @@ import type { InstanceType } from '@clerk/types'; +import type { TelemetryClientCache } from './clientCache'; + export type TelemetryCollectorOptions = { /** * If true, telemetry will not be collected. @@ -71,5 +73,17 @@ export type TelemetryEvent = { export type TelemetryEventRaw = { event: TelemetryEvent['event']; eventSamplingRate?: number; + eventSamplingClientCache?: TelemetryClientCache; payload: Payload; }; + +export type TelemetryClientCacheOptions = { + /** + * The unique identifier for the cache entry. + */ + key: string; + /** + * The time-to-live (TTL) for the cache entry, in milliseconds. If not specified, a default value will be used. + */ + cacheTtl?: number; +}; From 6ac00de519495447cfdd9adf2ebf4dac93f02070 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Thu, 25 Apr 2024 17:16:52 -0300 Subject: [PATCH 2/8] fix(shared): Fallback to sampling rate when storage is not supported --- packages/shared/src/telemetry/clientCache.ts | 35 +++++++++++++++++-- packages/shared/src/telemetry/collector.ts | 20 +++++------ .../src/telemetry/events/component-mounted.ts | 2 +- .../src/telemetry/events/method-called.ts | 2 +- packages/shared/src/telemetry/types.ts | 2 +- 5 files changed, 44 insertions(+), 17 deletions(-) diff --git a/packages/shared/src/telemetry/clientCache.ts b/packages/shared/src/telemetry/clientCache.ts index 8be5a9f4852..1156b265967 100644 --- a/packages/shared/src/telemetry/clientCache.ts +++ b/packages/shared/src/telemetry/clientCache.ts @@ -4,6 +4,10 @@ type TtlInMilliseconds = number; const DEFAULT_CACHE_TTL_MS = 86400000; // 24 hours +/** + * Manages caching for telemetry events using the browser's localStorage to + * mitigate event flooding in frequently executed code paths. + */ export class TelemetryClientCache { #key: TelemetryClientCacheOptions['key']; #cacheTtl: NonNullable; @@ -13,7 +17,7 @@ export class TelemetryClientCache { this.#cacheTtl = options.cacheTtl ?? DEFAULT_CACHE_TTL_MS; } - cacheAndRetrieve() { + cacheAndRetrieve(): boolean { const now = Date.now(); const item = this.#getItem(); @@ -21,8 +25,8 @@ export class TelemetryClientCache { localStorage.setItem(this.#key, now.toString()); } - const isExpired = item && now - item > this.#cacheTtl; - if (isExpired) { + const hasExpired = item && now - item > this.#cacheTtl; + if (hasExpired) { localStorage.removeItem(this.#key); } @@ -38,4 +42,29 @@ export class TelemetryClientCache { return JSON.parse(cacheString); } + + /** + * Checks if the browser's localStorage is supported and writable. + * + * If any of these operations fail, it indicates that localStorage is either + * not supported or not writable (e.g., in cases where the storage is full or + * the browser is in a privacy mode that restricts localStorage usage). + */ + get isStorageSupported(): boolean { + const storage = window['localStorage']; + + if (!storage) { + return false; + } + + try { + const testKey = `__storage_test__`; + storage.setItem(testKey, testKey); + storage.removeItem(testKey); + + return true; + } catch (err) { + return false; + } + } } diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index 48df1d3b1c2..395602fda67 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -123,19 +123,17 @@ export class TelemetryCollector { return this.isEnabled && !this.isDebug && this.#shouldBeSampled(event); } - #shouldBeSampled({ eventSamplingRate, eventSamplingClientCache }: TelemetryEventRaw): boolean { - if (typeof window === 'undefined' || !eventSamplingRate) { - const randomSeed = Math.random(); - - return ( - randomSeed <= this.#config.samplingRate && - (typeof eventSamplingRate === 'undefined' || randomSeed <= eventSamplingRate) - ); - } + #shouldBeSampled({ eventSamplingRate, clientCache }: TelemetryEventRaw) { + const randomSeed = Math.random(); - const isCached = eventSamplingClientCache?.cacheAndRetrieve(); + if (window && clientCache?.isStorageSupported) { + return clientCache?.cacheAndRetrieve(); + } - return !isCached; + return ( + randomSeed <= this.#config.samplingRate && + (typeof eventSamplingRate === 'undefined' || randomSeed <= eventSamplingRate) + ); } #scheduleFlush(): void { diff --git a/packages/shared/src/telemetry/events/component-mounted.ts b/packages/shared/src/telemetry/events/component-mounted.ts index 1f6f3556f1a..bdd47f4f2e4 100644 --- a/packages/shared/src/telemetry/events/component-mounted.ts +++ b/packages/shared/src/telemetry/events/component-mounted.ts @@ -21,7 +21,7 @@ export function eventComponentMounted( ): TelemetryEventRaw { return { event: EVENT_COMPONENT_MOUNTED, - eventSamplingClientCache: new TelemetryClientCache({ + clientCache: new TelemetryClientCache({ key: `${EVENT_COMPONENT_MOUNTED}:${component}`, }), eventSamplingRate: EVENT_SAMPLING_RATE, diff --git a/packages/shared/src/telemetry/events/method-called.ts b/packages/shared/src/telemetry/events/method-called.ts index 76593dffa5d..5b3f1933b22 100644 --- a/packages/shared/src/telemetry/events/method-called.ts +++ b/packages/shared/src/telemetry/events/method-called.ts @@ -18,7 +18,7 @@ export function eventMethodCalled( return { event: EVENT_METHOD_CALLED, eventSamplingRate: EVENT_SAMPLING_RATE, - eventSamplingClientCache: new TelemetryClientCache({ + clientCache: new TelemetryClientCache({ key: `${EVENT_METHOD_CALLED}:${method}`, }), payload: { diff --git a/packages/shared/src/telemetry/types.ts b/packages/shared/src/telemetry/types.ts index d6e0915526c..3f76d69229b 100644 --- a/packages/shared/src/telemetry/types.ts +++ b/packages/shared/src/telemetry/types.ts @@ -73,7 +73,7 @@ export type TelemetryEvent = { export type TelemetryEventRaw = { event: TelemetryEvent['event']; eventSamplingRate?: number; - eventSamplingClientCache?: TelemetryClientCache; + clientCache?: TelemetryClientCache; payload: Payload; }; From 0ee1108510afa6c5b3095d19cb523937ff1f1289 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Thu, 25 Apr 2024 18:48:20 -0300 Subject: [PATCH 3/8] perf(shared): Clear storage key on quota exceeded error --- packages/shared/src/telemetry/clientCache.ts | 40 ++++++++++++++----- .../src/telemetry/events/component-mounted.ts | 2 +- .../src/telemetry/events/method-called.ts | 2 +- packages/shared/src/telemetry/types.ts | 2 +- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/packages/shared/src/telemetry/clientCache.ts b/packages/shared/src/telemetry/clientCache.ts index 1156b265967..d8190bb9391 100644 --- a/packages/shared/src/telemetry/clientCache.ts +++ b/packages/shared/src/telemetry/clientCache.ts @@ -9,32 +9,38 @@ const DEFAULT_CACHE_TTL_MS = 86400000; // 24 hours * mitigate event flooding in frequently executed code paths. */ export class TelemetryClientCache { - #key: TelemetryClientCacheOptions['key']; + #storageKey = 'clerk_telemetry'; + #eventKey: TelemetryClientCacheOptions['eventKey']; #cacheTtl: NonNullable; constructor(options: TelemetryClientCacheOptions) { - this.#key = options.key; + this.#eventKey = options.eventKey; this.#cacheTtl = options.cacheTtl ?? DEFAULT_CACHE_TTL_MS; } cacheAndRetrieve(): boolean { const now = Date.now(); - const item = this.#getItem(); + const event = this.#cache?.[this.#eventKey]; - if (!item) { - localStorage.setItem(this.#key, now.toString()); + if (!event) { + localStorage.setItem( + this.#storageKey, + JSON.stringify({ + [this.#eventKey]: now, + }), + ); } - const hasExpired = item && now - item > this.#cacheTtl; + const hasExpired = event && now - event > this.#cacheTtl; if (hasExpired) { - localStorage.removeItem(this.#key); + localStorage.removeItem(this.#storageKey); } - return !!item; + return !!event; } - #getItem(): TtlInMilliseconds | undefined { - const cacheString = localStorage.getItem(this.#key); + get #cache(): Record | undefined { + const cacheString = localStorage.getItem(this.#storageKey); if (!cacheString) { return; @@ -58,13 +64,25 @@ export class TelemetryClientCache { } try { - const testKey = `__storage_test__`; + const testKey = 'test'; storage.setItem(testKey, testKey); storage.removeItem(testKey); return true; } catch (err) { + if (this.#isQuotaExceededError(err) && storage.length > 0) { + storage.removeItem(this.#storageKey); + } + return false; } } + + #isQuotaExceededError(err: unknown): boolean { + return ( + err instanceof DOMException && + // Check error names for different browsers + (err.name === 'QuotaExceededError' || err.name === 'NS_ERROR_DOM_QUOTA_REACHED') + ); + } } diff --git a/packages/shared/src/telemetry/events/component-mounted.ts b/packages/shared/src/telemetry/events/component-mounted.ts index bdd47f4f2e4..edd8a8afa6d 100644 --- a/packages/shared/src/telemetry/events/component-mounted.ts +++ b/packages/shared/src/telemetry/events/component-mounted.ts @@ -22,7 +22,7 @@ export function eventComponentMounted( return { event: EVENT_COMPONENT_MOUNTED, clientCache: new TelemetryClientCache({ - key: `${EVENT_COMPONENT_MOUNTED}:${component}`, + eventKey: `${EVENT_COMPONENT_MOUNTED}:${component}`, }), eventSamplingRate: EVENT_SAMPLING_RATE, payload: { diff --git a/packages/shared/src/telemetry/events/method-called.ts b/packages/shared/src/telemetry/events/method-called.ts index 5b3f1933b22..06147ca8edf 100644 --- a/packages/shared/src/telemetry/events/method-called.ts +++ b/packages/shared/src/telemetry/events/method-called.ts @@ -19,7 +19,7 @@ export function eventMethodCalled( event: EVENT_METHOD_CALLED, eventSamplingRate: EVENT_SAMPLING_RATE, clientCache: new TelemetryClientCache({ - key: `${EVENT_METHOD_CALLED}:${method}`, + eventKey: `${EVENT_METHOD_CALLED}:${method}`, }), payload: { method, diff --git a/packages/shared/src/telemetry/types.ts b/packages/shared/src/telemetry/types.ts index 3f76d69229b..8c4de7d5a65 100644 --- a/packages/shared/src/telemetry/types.ts +++ b/packages/shared/src/telemetry/types.ts @@ -81,7 +81,7 @@ export type TelemetryClientCacheOptions = { /** * The unique identifier for the cache entry. */ - key: string; + eventKey: string; /** * The time-to-live (TTL) for the cache entry, in milliseconds. If not specified, a default value will be used. */ From 9038c88ae71502ec2c181b22811c842c2d4e73fa Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Fri, 26 Apr 2024 09:34:03 -0300 Subject: [PATCH 4/8] test(shared): Add unit tests for client cache --- .../shared/src/__tests__/telemetry.test.ts | 93 +++++++++++++++++++ packages/shared/src/telemetry/clientCache.ts | 17 ++-- packages/shared/src/telemetry/collector.ts | 3 +- .../src/telemetry/events/method-called.ts | 2 +- 4 files changed, 103 insertions(+), 12 deletions(-) diff --git a/packages/shared/src/__tests__/telemetry.test.ts b/packages/shared/src/__tests__/telemetry.test.ts index 7f2c0a41e7f..6c2d6fa6c07 100644 --- a/packages/shared/src/__tests__/telemetry.test.ts +++ b/packages/shared/src/__tests__/telemetry.test.ts @@ -1,6 +1,7 @@ import 'cross-fetch/polyfill'; import { TelemetryCollector } from '../telemetry'; +import { TelemetryClientCache } from '../telemetry/clientCache'; jest.useFakeTimers(); @@ -170,4 +171,96 @@ describe('TelemetryCollector', () => { fetchSpy.mockRestore(); randomSpy.mockRestore; }); + + describe('with client-side caching', () => { + beforeEach(() => { + localStorage.clear(); + }); + + test('sends event when it is not in the cache', () => { + const fetchSpy = jest.spyOn(global, 'fetch'); + + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + const event = 'TEST_EVENT'; + + collector.record({ + event, + payload: {}, + clientCache: new TelemetryClientCache({ + eventKey: `${event}:foo`, + }), + }); + + jest.runAllTimers(); + + expect(fetchSpy).toHaveBeenCalled(); + + fetchSpy.mockRestore(); + }); + + test('does not send event when it is in the cache', () => { + const fetchSpy = jest.spyOn(global, 'fetch'); + + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + const event = 'TEST_EVENT'; + + collector.record({ + event, + eventSamplingRate: 0.01, + payload: {}, + clientCache: new TelemetryClientCache({ + eventKey: `${event}:foo`, + }), + }); + + collector.record({ + event, + payload: {}, + clientCache: new TelemetryClientCache({ + eventKey: `${event}:foo`, + }), + }); + + jest.runAllTimers(); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + + fetchSpy.mockRestore(); + }); + + test('fallbacks to event-specific sampling rate when storage is not supported', () => { + jest.spyOn(TelemetryClientCache.prototype, 'isStorageSupported', 'get').mockReturnValue(false); + + const fetchSpy = jest.spyOn(global, 'fetch'); + const randomSpy = jest.spyOn(Math, 'random').mockReturnValue(0.1); + + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + const event = 'TEST_EVENT'; + + collector.record({ + event: 'TEST_EVENT', + eventSamplingRate: 0.01, + payload: {}, + clientCache: new TelemetryClientCache({ + eventKey: `${event}:foo`, + }), + }); + + jest.runAllTimers(); + + expect(fetchSpy).not.toHaveBeenCalled(); + + fetchSpy.mockRestore(); + randomSpy.mockRestore; + }); + }); }); diff --git a/packages/shared/src/telemetry/clientCache.ts b/packages/shared/src/telemetry/clientCache.ts index d8190bb9391..608a4a914c5 100644 --- a/packages/shared/src/telemetry/clientCache.ts +++ b/packages/shared/src/telemetry/clientCache.ts @@ -69,20 +69,17 @@ export class TelemetryClientCache { storage.removeItem(testKey); return true; - } catch (err) { - if (this.#isQuotaExceededError(err) && storage.length > 0) { + } catch (err: unknown) { + const isQuotaExceededError = + err instanceof DOMException && + // Check error names for different browsers + (err.name === 'QuotaExceededError' || err.name === 'NS_ERROR_DOM_QUOTA_REACHED'); + + if (isQuotaExceededError && storage.length > 0) { storage.removeItem(this.#storageKey); } return false; } } - - #isQuotaExceededError(err: unknown): boolean { - return ( - err instanceof DOMException && - // Check error names for different browsers - (err.name === 'QuotaExceededError' || err.name === 'NS_ERROR_DOM_QUOTA_REACHED') - ); - } } diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index 395602fda67..cd0b8a6b2c5 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -127,7 +127,8 @@ export class TelemetryCollector { const randomSeed = Math.random(); if (window && clientCache?.isStorageSupported) { - return clientCache?.cacheAndRetrieve(); + const isCached = clientCache?.cacheAndRetrieve(); + return !isCached; } return ( diff --git a/packages/shared/src/telemetry/events/method-called.ts b/packages/shared/src/telemetry/events/method-called.ts index 06147ca8edf..1a007240bb0 100644 --- a/packages/shared/src/telemetry/events/method-called.ts +++ b/packages/shared/src/telemetry/events/method-called.ts @@ -17,10 +17,10 @@ export function eventMethodCalled( ): TelemetryEventRaw { return { event: EVENT_METHOD_CALLED, - eventSamplingRate: EVENT_SAMPLING_RATE, clientCache: new TelemetryClientCache({ eventKey: `${EVENT_METHOD_CALLED}:${method}`, }), + eventSamplingRate: EVENT_SAMPLING_RATE, payload: { method, ...payload, From 39e5aafa3be7b42efacf5e177d5eae2a9d2c7442 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Fri, 26 Apr 2024 11:30:27 -0300 Subject: [PATCH 5/8] perf(shared): Move TelemetryClientCache instance to TelemetryCollector --- .../shared/src/__tests__/telemetry.test.ts | 39 ++++++++----------- packages/shared/src/telemetry/clientCache.ts | 16 ++------ packages/shared/src/telemetry/collector.ts | 17 +++++--- .../src/telemetry/events/component-mounted.ts | 5 +-- .../src/telemetry/events/method-called.ts | 5 +-- packages/shared/src/telemetry/types.ts | 15 +------ 6 files changed, 35 insertions(+), 62 deletions(-) diff --git a/packages/shared/src/__tests__/telemetry.test.ts b/packages/shared/src/__tests__/telemetry.test.ts index 6c2d6fa6c07..14b711f7ca4 100644 --- a/packages/shared/src/__tests__/telemetry.test.ts +++ b/packages/shared/src/__tests__/telemetry.test.ts @@ -184,14 +184,12 @@ describe('TelemetryCollector', () => { publishableKey: TEST_PK, }); - const event = 'TEST_EVENT'; - collector.record({ - event, - payload: {}, - clientCache: new TelemetryClientCache({ - eventKey: `${event}:foo`, - }), + event: 'TEST_EVENT', + payload: { + foo: true, + }, + clientCacheKey: `TEST_EVENT:foo`, }); jest.runAllTimers(); @@ -212,19 +210,18 @@ describe('TelemetryCollector', () => { collector.record({ event, - eventSamplingRate: 0.01, - payload: {}, - clientCache: new TelemetryClientCache({ - eventKey: `${event}:foo`, - }), + payload: { + foo: true, + }, + clientCacheKey: `TEST_EVENT:foo`, }); collector.record({ event, - payload: {}, - clientCache: new TelemetryClientCache({ - eventKey: `${event}:foo`, - }), + payload: { + foo: true, + }, + clientCacheKey: `TEST_EVENT:foo`, }); jest.runAllTimers(); @@ -244,15 +241,13 @@ describe('TelemetryCollector', () => { publishableKey: TEST_PK, }); - const event = 'TEST_EVENT'; - collector.record({ event: 'TEST_EVENT', eventSamplingRate: 0.01, - payload: {}, - clientCache: new TelemetryClientCache({ - eventKey: `${event}:foo`, - }), + payload: { + foo: true, + }, + clientCacheKey: `TEST_EVENT:foo`, }); jest.runAllTimers(); diff --git a/packages/shared/src/telemetry/clientCache.ts b/packages/shared/src/telemetry/clientCache.ts index 608a4a914c5..72abb5d31bf 100644 --- a/packages/shared/src/telemetry/clientCache.ts +++ b/packages/shared/src/telemetry/clientCache.ts @@ -1,5 +1,3 @@ -import type { TelemetryClientCacheOptions } from './types'; - type TtlInMilliseconds = number; const DEFAULT_CACHE_TTL_MS = 86400000; // 24 hours @@ -10,23 +8,17 @@ const DEFAULT_CACHE_TTL_MS = 86400000; // 24 hours */ export class TelemetryClientCache { #storageKey = 'clerk_telemetry'; - #eventKey: TelemetryClientCacheOptions['eventKey']; - #cacheTtl: NonNullable; - - constructor(options: TelemetryClientCacheOptions) { - this.#eventKey = options.eventKey; - this.#cacheTtl = options.cacheTtl ?? DEFAULT_CACHE_TTL_MS; - } + #cacheTtl = DEFAULT_CACHE_TTL_MS; - cacheAndRetrieve(): boolean { + cacheAndRetrieve(key: string): boolean { const now = Date.now(); - const event = this.#cache?.[this.#eventKey]; + const event = this.#cache?.[key]; if (!event) { localStorage.setItem( this.#storageKey, JSON.stringify({ - [this.#eventKey]: now, + [key]: now, }), ); } diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index cd0b8a6b2c5..7cda9018d47 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -14,6 +14,7 @@ import type { InstanceType } from '@clerk/types'; import { parsePublishableKey } from '../keys'; import { isTruthy } from '../underscore'; +import { TelemetryClientCache } from './clientCache'; import type { TelemetryCollectorOptions, TelemetryEvent, TelemetryEventRaw } from './types'; type TelemetryCollectorConfig = Pick< @@ -43,6 +44,7 @@ const DEFAULT_CONFIG: Partial = { export class TelemetryCollector { #config: Required; + #clientCache: TelemetryClientCache; #metadata: TelemetryMetadata = {} as TelemetryMetadata; #buffer: TelemetryEvent[] = []; #pendingFlush: any; @@ -78,6 +80,8 @@ export class TelemetryCollector { // Only send the first 16 characters of the secret key to to avoid sending the full key. We can still query against the partial key. this.#metadata.secretKey = options.secretKey.substring(0, 16); } + + this.#clientCache = new TelemetryClientCache(); } get isEnabled(): boolean { @@ -110,7 +114,7 @@ export class TelemetryCollector { this.#logEvent(preparedPayload.event, preparedPayload); - if (!this.#shouldRecord(event)) { + if (!this.#shouldRecord(event.eventSamplingRate, event.clientCacheKey)) { return; } @@ -119,15 +123,16 @@ export class TelemetryCollector { this.#scheduleFlush(); } - #shouldRecord(event: TelemetryEventRaw): boolean { - return this.isEnabled && !this.isDebug && this.#shouldBeSampled(event); + #shouldRecord(eventSamplingRate?: number, clientCacheKey?: string): boolean { + return this.isEnabled && !this.isDebug && this.#shouldBeSampled(eventSamplingRate, clientCacheKey); } - #shouldBeSampled({ eventSamplingRate, clientCache }: TelemetryEventRaw) { + #shouldBeSampled(eventSamplingRate?: number, clientCacheKey?: string) { const randomSeed = Math.random(); + const clientCache = this.#clientCache; - if (window && clientCache?.isStorageSupported) { - const isCached = clientCache?.cacheAndRetrieve(); + if (window && clientCache.isStorageSupported && clientCacheKey) { + const isCached = clientCache.cacheAndRetrieve(clientCacheKey); return !isCached; } diff --git a/packages/shared/src/telemetry/events/component-mounted.ts b/packages/shared/src/telemetry/events/component-mounted.ts index edd8a8afa6d..1ef57eb5c9d 100644 --- a/packages/shared/src/telemetry/events/component-mounted.ts +++ b/packages/shared/src/telemetry/events/component-mounted.ts @@ -1,4 +1,3 @@ -import { TelemetryClientCache } from '../clientCache'; import type { TelemetryEventRaw } from '../types'; const EVENT_COMPONENT_MOUNTED = 'COMPONENT_MOUNTED' as const; @@ -21,9 +20,7 @@ export function eventComponentMounted( ): TelemetryEventRaw { return { event: EVENT_COMPONENT_MOUNTED, - clientCache: new TelemetryClientCache({ - eventKey: `${EVENT_COMPONENT_MOUNTED}:${component}`, - }), + clientCacheKey: `${EVENT_COMPONENT_MOUNTED}:${component}`, eventSamplingRate: EVENT_SAMPLING_RATE, payload: { component, diff --git a/packages/shared/src/telemetry/events/method-called.ts b/packages/shared/src/telemetry/events/method-called.ts index 1a007240bb0..536b1e491c7 100644 --- a/packages/shared/src/telemetry/events/method-called.ts +++ b/packages/shared/src/telemetry/events/method-called.ts @@ -1,4 +1,3 @@ -import { TelemetryClientCache } from '../clientCache'; import type { TelemetryEventRaw } from '../types'; const EVENT_METHOD_CALLED = 'METHOD_CALLED' as const; @@ -17,9 +16,7 @@ export function eventMethodCalled( ): TelemetryEventRaw { return { event: EVENT_METHOD_CALLED, - clientCache: new TelemetryClientCache({ - eventKey: `${EVENT_METHOD_CALLED}:${method}`, - }), + clientCacheKey: `${EVENT_METHOD_CALLED}:${method}`, eventSamplingRate: EVENT_SAMPLING_RATE, payload: { method, diff --git a/packages/shared/src/telemetry/types.ts b/packages/shared/src/telemetry/types.ts index 8c4de7d5a65..7dca7ccc27d 100644 --- a/packages/shared/src/telemetry/types.ts +++ b/packages/shared/src/telemetry/types.ts @@ -1,7 +1,5 @@ import type { InstanceType } from '@clerk/types'; -import type { TelemetryClientCache } from './clientCache'; - export type TelemetryCollectorOptions = { /** * If true, telemetry will not be collected. @@ -73,17 +71,6 @@ export type TelemetryEvent = { export type TelemetryEventRaw = { event: TelemetryEvent['event']; eventSamplingRate?: number; - clientCache?: TelemetryClientCache; + clientCacheKey?: `${TelemetryEvent['event']}:${string}`; payload: Payload; }; - -export type TelemetryClientCacheOptions = { - /** - * The unique identifier for the cache entry. - */ - eventKey: string; - /** - * The time-to-live (TTL) for the cache entry, in milliseconds. If not specified, a default value will be used. - */ - cacheTtl?: number; -}; From 625fd6acfceffc390380406c327e81c041ba5198 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Mon, 29 Apr 2024 11:37:36 -0300 Subject: [PATCH 6/8] fix(shared): Verify if window is defined --- packages/shared/src/telemetry/clientCache.ts | 4 ++++ packages/shared/src/telemetry/collector.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/telemetry/clientCache.ts b/packages/shared/src/telemetry/clientCache.ts index 72abb5d31bf..21bf16115b5 100644 --- a/packages/shared/src/telemetry/clientCache.ts +++ b/packages/shared/src/telemetry/clientCache.ts @@ -49,6 +49,10 @@ export class TelemetryClientCache { * the browser is in a privacy mode that restricts localStorage usage). */ get isStorageSupported(): boolean { + if (typeof window === 'undefined') { + return false; + } + const storage = window['localStorage']; if (!storage) { diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index 7cda9018d47..cd580ebd838 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -131,7 +131,7 @@ export class TelemetryCollector { const randomSeed = Math.random(); const clientCache = this.#clientCache; - if (window && clientCache.isStorageSupported && clientCacheKey) { + if (clientCache.isStorageSupported && clientCacheKey) { const isCached = clientCache.cacheAndRetrieve(clientCacheKey); return !isCached; } From 253e682a35fe886502df501edd0adc6876ac5123 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Mon, 29 Apr 2024 12:44:16 -0300 Subject: [PATCH 7/8] perf(shared): Generate cache key based on payload --- .../shared/src/__tests__/telemetry.test.ts | 50 +++++++++++-------- packages/shared/src/telemetry/clientCache.ts | 23 +++++++-- packages/shared/src/telemetry/collector.ts | 14 +++--- .../src/telemetry/events/component-mounted.ts | 1 - .../src/telemetry/events/method-called.ts | 1 - packages/shared/src/telemetry/types.ts | 1 - 6 files changed, 55 insertions(+), 35 deletions(-) diff --git a/packages/shared/src/__tests__/telemetry.test.ts b/packages/shared/src/__tests__/telemetry.test.ts index 14b711f7ca4..f5b9e0e4861 100644 --- a/packages/shared/src/__tests__/telemetry.test.ts +++ b/packages/shared/src/__tests__/telemetry.test.ts @@ -8,6 +8,10 @@ jest.useFakeTimers(); const TEST_PK = 'pk_test_Zm9vLWJhci0xMy5jbGVyay5hY2NvdW50cy5kZXYk'; describe('TelemetryCollector', () => { + beforeEach(() => { + localStorage.clear(); + }); + test('does nothing when disabled', async () => { const fetchSpy = jest.spyOn(global, 'fetch'); @@ -146,37 +150,47 @@ describe('TelemetryCollector', () => { publishableKey: TEST_PK, }); - collector.record({ event: 'TEST_EVENT', payload: {} }); - collector.record({ event: 'TEST_EVENT', payload: {} }); + collector.record({ event: 'TEST_EVENT', payload: { method: 'useFoo' } }); + collector.record({ event: 'TEST_EVENT', payload: { method: 'useBar' } }); expect(fetchSpy).toHaveBeenCalled(); fetchSpy.mockRestore(); }); - test('does not send events if the random seed does not exceed the event-specific sampling rate', async () => { - const fetchSpy = jest.spyOn(global, 'fetch'); - const randomSpy = jest.spyOn(Math, 'random').mockReturnValue(0.1); + describe('with server-side sampling', () => { + let windowSpy; - const collector = new TelemetryCollector({ - publishableKey: TEST_PK, + beforeEach(() => { + windowSpy = jest.spyOn(window, 'window', 'get'); }); - collector.record({ event: 'TEST_EVENT', eventSamplingRate: 0.01, payload: {} }); + afterEach(() => { + windowSpy.mockRestore(); + }); - jest.runAllTimers(); + test('does not send events if the random seed does not exceed the event-specific sampling rate', async () => { + windowSpy.mockImplementation(() => undefined); - expect(fetchSpy).not.toHaveBeenCalled(); + const fetchSpy = jest.spyOn(global, 'fetch'); + const randomSpy = jest.spyOn(Math, 'random').mockReturnValue(0.1); - fetchSpy.mockRestore(); - randomSpy.mockRestore; - }); + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); - describe('with client-side caching', () => { - beforeEach(() => { - localStorage.clear(); + collector.record({ event: 'TEST_EVENT', eventSamplingRate: 0.01, payload: {} }); + + jest.runAllTimers(); + + expect(fetchSpy).not.toHaveBeenCalled(); + + fetchSpy.mockRestore(); + randomSpy.mockRestore; }); + }); + describe('with client-side caching', () => { test('sends event when it is not in the cache', () => { const fetchSpy = jest.spyOn(global, 'fetch'); @@ -189,7 +203,6 @@ describe('TelemetryCollector', () => { payload: { foo: true, }, - clientCacheKey: `TEST_EVENT:foo`, }); jest.runAllTimers(); @@ -213,7 +226,6 @@ describe('TelemetryCollector', () => { payload: { foo: true, }, - clientCacheKey: `TEST_EVENT:foo`, }); collector.record({ @@ -221,7 +233,6 @@ describe('TelemetryCollector', () => { payload: { foo: true, }, - clientCacheKey: `TEST_EVENT:foo`, }); jest.runAllTimers(); @@ -247,7 +258,6 @@ describe('TelemetryCollector', () => { payload: { foo: true, }, - clientCacheKey: `TEST_EVENT:foo`, }); jest.runAllTimers(); diff --git a/packages/shared/src/telemetry/clientCache.ts b/packages/shared/src/telemetry/clientCache.ts index 21bf16115b5..b7443460e80 100644 --- a/packages/shared/src/telemetry/clientCache.ts +++ b/packages/shared/src/telemetry/clientCache.ts @@ -1,3 +1,5 @@ +import type { TelemetryEventRaw } from './types'; + type TtlInMilliseconds = number; const DEFAULT_CACHE_TTL_MS = 86400000; // 24 hours @@ -10,11 +12,12 @@ export class TelemetryClientCache { #storageKey = 'clerk_telemetry'; #cacheTtl = DEFAULT_CACHE_TTL_MS; - cacheAndRetrieve(key: string): boolean { + cacheAndRetrieve(event: TelemetryEventRaw): boolean { + const key = this.#getCacheKey(event); const now = Date.now(); - const event = this.#cache?.[key]; - if (!event) { + const cacheEntry = this.#cache?.[key]; + if (!cacheEntry) { localStorage.setItem( this.#storageKey, JSON.stringify({ @@ -23,12 +26,22 @@ export class TelemetryClientCache { ); } - const hasExpired = event && now - event > this.#cacheTtl; + const hasExpired = cacheEntry && now - cacheEntry > this.#cacheTtl; if (hasExpired) { localStorage.removeItem(this.#storageKey); } - return !!event; + return !!cacheEntry; + } + + #getCacheKey({ event, payload }: TelemetryEventRaw): string { + const payloadUniqueKey = JSON.stringify( + Object.keys(payload) + .sort() + .map(key => payload[key]), + ); + + return `${event}:${payloadUniqueKey}`; } get #cache(): Record | undefined { diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index cd580ebd838..0f1a84fb449 100644 --- a/packages/shared/src/telemetry/collector.ts +++ b/packages/shared/src/telemetry/collector.ts @@ -114,7 +114,7 @@ export class TelemetryCollector { this.#logEvent(preparedPayload.event, preparedPayload); - if (!this.#shouldRecord(event.eventSamplingRate, event.clientCacheKey)) { + if (!this.#shouldRecord(event)) { return; } @@ -123,22 +123,22 @@ export class TelemetryCollector { this.#scheduleFlush(); } - #shouldRecord(eventSamplingRate?: number, clientCacheKey?: string): boolean { - return this.isEnabled && !this.isDebug && this.#shouldBeSampled(eventSamplingRate, clientCacheKey); + #shouldRecord(event: TelemetryEventRaw) { + return this.isEnabled && !this.isDebug && this.#shouldBeSampled(event); } - #shouldBeSampled(eventSamplingRate?: number, clientCacheKey?: string) { + #shouldBeSampled(event: TelemetryEventRaw) { const randomSeed = Math.random(); const clientCache = this.#clientCache; - if (clientCache.isStorageSupported && clientCacheKey) { - const isCached = clientCache.cacheAndRetrieve(clientCacheKey); + if (clientCache.isStorageSupported) { + const isCached = clientCache.cacheAndRetrieve(event); return !isCached; } return ( randomSeed <= this.#config.samplingRate && - (typeof eventSamplingRate === 'undefined' || randomSeed <= eventSamplingRate) + (typeof event.eventSamplingRate === 'undefined' || randomSeed <= event.eventSamplingRate) ); } diff --git a/packages/shared/src/telemetry/events/component-mounted.ts b/packages/shared/src/telemetry/events/component-mounted.ts index 1ef57eb5c9d..8649bf1a113 100644 --- a/packages/shared/src/telemetry/events/component-mounted.ts +++ b/packages/shared/src/telemetry/events/component-mounted.ts @@ -20,7 +20,6 @@ export function eventComponentMounted( ): TelemetryEventRaw { return { event: EVENT_COMPONENT_MOUNTED, - clientCacheKey: `${EVENT_COMPONENT_MOUNTED}:${component}`, eventSamplingRate: EVENT_SAMPLING_RATE, payload: { component, diff --git a/packages/shared/src/telemetry/events/method-called.ts b/packages/shared/src/telemetry/events/method-called.ts index 536b1e491c7..bcf9e746269 100644 --- a/packages/shared/src/telemetry/events/method-called.ts +++ b/packages/shared/src/telemetry/events/method-called.ts @@ -16,7 +16,6 @@ export function eventMethodCalled( ): TelemetryEventRaw { return { event: EVENT_METHOD_CALLED, - clientCacheKey: `${EVENT_METHOD_CALLED}:${method}`, eventSamplingRate: EVENT_SAMPLING_RATE, payload: { method, diff --git a/packages/shared/src/telemetry/types.ts b/packages/shared/src/telemetry/types.ts index 7dca7ccc27d..e594590db58 100644 --- a/packages/shared/src/telemetry/types.ts +++ b/packages/shared/src/telemetry/types.ts @@ -71,6 +71,5 @@ export type TelemetryEvent = { export type TelemetryEventRaw = { event: TelemetryEvent['event']; eventSamplingRate?: number; - clientCacheKey?: `${TelemetryEvent['event']}:${string}`; payload: Payload; }; From 43d94bc373bb8fffe2ad91106eae6c724ad15c60 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:05:20 -0300 Subject: [PATCH 8/8] fix(shared): Do not override previous cache state --- packages/shared/src/telemetry/clientCache.ts | 28 ++++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/shared/src/telemetry/clientCache.ts b/packages/shared/src/telemetry/clientCache.ts index b7443460e80..cc0f279176b 100644 --- a/packages/shared/src/telemetry/clientCache.ts +++ b/packages/shared/src/telemetry/clientCache.ts @@ -13,28 +13,28 @@ export class TelemetryClientCache { #cacheTtl = DEFAULT_CACHE_TTL_MS; cacheAndRetrieve(event: TelemetryEventRaw): boolean { - const key = this.#getCacheKey(event); const now = Date.now(); + const key = this.#generateKey(event); + const entry = this.#cache?.[key]; - const cacheEntry = this.#cache?.[key]; - if (!cacheEntry) { - localStorage.setItem( - this.#storageKey, - JSON.stringify({ - [key]: now, - }), - ); + if (!entry) { + const updatedCache = { + ...this.#cache, + [key]: now, + }; + + localStorage.setItem(this.#storageKey, JSON.stringify(updatedCache)); } - const hasExpired = cacheEntry && now - cacheEntry > this.#cacheTtl; + const hasExpired = entry && now - entry > this.#cacheTtl; if (hasExpired) { - localStorage.removeItem(this.#storageKey); + localStorage.removeItem(key); } - return !!cacheEntry; + return !!entry; } - #getCacheKey({ event, payload }: TelemetryEventRaw): string { + #generateKey({ event, payload }: TelemetryEventRaw): string { const payloadUniqueKey = JSON.stringify( Object.keys(payload) .sort() @@ -48,7 +48,7 @@ export class TelemetryClientCache { const cacheString = localStorage.getItem(this.#storageKey); if (!cacheString) { - return; + return {}; } return JSON.parse(cacheString);