From a78bc447c1aabaa41bcbaa2a8fe3c48f31275574 Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Tue, 30 Apr 2024 09:46:49 -0700 Subject: [PATCH] feat(shared): Introduce client-side caching for telemetry events (#3287) * feat(shared): Introduce client-side caching for telemetry events * fix(shared): Remove entry from cache once expired * fix(shared): Reinvalidate cache for next retrieval * fix(shared): Fix TTL constant value * refactor(shared): Rename to Throttler * docs(*): Update changeset * fix(shared): Use prepared payload to generate key --- .changeset/metal-bulldogs-destroy.md | 5 + .../shared/src/__tests__/telemetry.test.ts | 208 +++++++++++++++--- packages/shared/src/telemetry/collector.ts | 24 +- packages/shared/src/telemetry/throttler.ts | 112 ++++++++++ 4 files changed, 312 insertions(+), 37 deletions(-) create mode 100644 .changeset/metal-bulldogs-destroy.md create mode 100644 packages/shared/src/telemetry/throttler.ts diff --git a/.changeset/metal-bulldogs-destroy.md b/.changeset/metal-bulldogs-destroy.md new file mode 100644 index 0000000000..632eac0ddd --- /dev/null +++ b/.changeset/metal-bulldogs-destroy.md @@ -0,0 +1,5 @@ +--- +'@clerk/shared': patch +--- + +Internal change to add client-side caching to Clerk's telemetry. This prevents event flooding in frequently executed code paths, such as for React hooks or components. Gracefully falls back to the old behavior if e.g. `localStorage` is not available. As such, no public API changes and user changes are required. diff --git a/packages/shared/src/__tests__/telemetry.test.ts b/packages/shared/src/__tests__/telemetry.test.ts index 7f2c0a41e7..a0bb77b0c6 100644 --- a/packages/shared/src/__tests__/telemetry.test.ts +++ b/packages/shared/src/__tests__/telemetry.test.ts @@ -1,15 +1,29 @@ import 'cross-fetch/polyfill'; +import assert from 'assert'; + import { TelemetryCollector } from '../telemetry'; +import type { TelemetryEvent } from '../telemetry/types'; jest.useFakeTimers(); const TEST_PK = 'pk_test_Zm9vLWJhci0xMy5jbGVyay5hY2NvdW50cy5kZXYk'; describe('TelemetryCollector', () => { - test('does nothing when disabled', async () => { - const fetchSpy = jest.spyOn(global, 'fetch'); + let windowSpy; + let fetchSpy; + beforeEach(() => { + fetchSpy = jest.spyOn(global, 'fetch'); + windowSpy = jest.spyOn(window, 'window', 'get'); + }); + + afterEach(() => { + windowSpy.mockRestore(); + fetchSpy.mockRestore(); + }); + + test('does nothing when disabled', async () => { const collector = new TelemetryCollector({ disabled: true, publishableKey: TEST_PK, @@ -20,15 +34,11 @@ describe('TelemetryCollector', () => { jest.runAllTimers(); expect(fetchSpy).not.toHaveBeenCalled(); - - fetchSpy.mockRestore(); }); test('does nothing when CLERK_TELEMETRY_DISABLED is set', async () => { process.env.CLERK_TELEMETRY_DISABLED = '1'; - const fetchSpy = jest.spyOn(global, 'fetch'); - const collector = new TelemetryCollector({ publishableKey: TEST_PK, }); @@ -39,13 +49,10 @@ describe('TelemetryCollector', () => { expect(fetchSpy).not.toHaveBeenCalled(); - fetchSpy.mockRestore(); - process.env.CLERK_TELEMETRY_DISABLED = undefined; }); test('does not send events when debug is enabled, logs them instead', async () => { - const fetchSpy = jest.spyOn(global, 'fetch'); const consoleGroupSpy = jest.spyOn(console, 'groupCollapsed').mockImplementation(() => {}); const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); @@ -78,13 +85,11 @@ describe('TelemetryCollector', () => { consoleGroupSpy.mockRestore(); consoleSpy.mockRestore(); - fetchSpy.mockRestore(); }); test('enables debug via environment variable', async () => { process.env.CLERK_TELEMETRY_DEBUG = '1'; - const fetchSpy = jest.spyOn(global, 'fetch'); const consoleGroupSpy = jest.spyOn(console, 'groupCollapsed').mockImplementation(() => {}); const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); @@ -116,14 +121,11 @@ describe('TelemetryCollector', () => { consoleGroupSpy.mockRestore(); consoleSpy.mockRestore(); - fetchSpy.mockRestore(); process.env.CLERK_TELEMETRY_DEBUG = undefined; }); test('sends events after a delay when buffer is not full', async () => { - const fetchSpy = jest.spyOn(global, 'fetch'); - const collector = new TelemetryCollector({ publishableKey: TEST_PK, }); @@ -133,41 +135,185 @@ describe('TelemetryCollector', () => { jest.runAllTimers(); expect(fetchSpy).toHaveBeenCalled(); - - fetchSpy.mockRestore(); }); test('sends events immediately when the buffer limit is reached', async () => { - const fetchSpy = jest.spyOn(global, 'fetch'); - const collector = new TelemetryCollector({ maxBufferSize: 2, 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(); + describe('with server-side sampling', () => { + test('does not send events if the random seed does not exceed the event-specific sampling rate', async () => { + windowSpy.mockImplementation(() => undefined); + + const randomSpy = jest.spyOn(Math, 'random').mockReturnValue(0.1); + + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + collector.record({ event: 'TEST_EVENT', eventSamplingRate: 0.01, payload: {} }); + + jest.runAllTimers(); + + expect(fetchSpy).not.toHaveBeenCalled(); + + randomSpy.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 client-side throttling', () => { + beforeEach(() => { + localStorage.clear(); + }); + + test('sends event when it is not in the cache', () => { + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); - const collector = new TelemetryCollector({ - publishableKey: TEST_PK, + collector.record({ + event: 'TEST_EVENT', + payload: { + foo: true, + }, + }); + + jest.runAllTimers(); + + expect(fetchSpy).toHaveBeenCalled(); + + fetchSpy.mockRestore(); }); - collector.record({ event: 'TEST_EVENT', eventSamplingRate: 0.01, payload: {} }); + test('sends event when it is in the cache but has expired', () => { + const originalDateNow = Date.now; + const cacheTtl = 86400000; - jest.runAllTimers(); + let now = originalDateNow(); + const dateNowSpy = jest.spyOn(Date, 'now').mockImplementation(() => now); - expect(fetchSpy).not.toHaveBeenCalled(); + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + maxBufferSize: 1, + }); - fetchSpy.mockRestore(); - randomSpy.mockRestore; + const event = 'TEST_EVENT'; + const payload = { + foo: true, + }; + + collector.record({ + event, + payload, + }); + + // Move time forward beyond the cache TTL + now += cacheTtl + 1; + + collector.record({ + event, + payload, + }); + + collector.record({ + event, + payload, + }); + + jest.runAllTimers(); + + expect(fetchSpy).toHaveBeenCalledTimes(2); + + dateNowSpy.mockRestore(); + }); + + test('does not send event when it is in the cache', () => { + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + const event = 'TEST_EVENT'; + + collector.record({ + event, + payload: { + foo: true, + }, + }); + + collector.record({ + event, + payload: { + foo: true, + }, + }); + + jest.runAllTimers(); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + test('fallbacks to event-specific sampling rate when storage is not supported', () => { + windowSpy.mockImplementation(() => ({ + localStorage: undefined, + })); + + const randomSpy = jest.spyOn(Math, 'random').mockReturnValue(0.1); + + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + collector.record({ + event: 'TEST_EVENT', + eventSamplingRate: 0.01, + payload: { + foo: true, + }, + }); + + expect(fetchSpy).not.toHaveBeenCalled(); + + randomSpy.mockRestore; + }); + + test('generates unique key without credentials based on event payload', () => { + const collector = new TelemetryCollector({ + publishableKey: TEST_PK, + }); + + const event: TelemetryEvent = { + sk: '123', + pk: TEST_PK, + it: 'development', + event: 'TEST_EVENT', + cv: '0.1', + sdkv: '0.1', + payload: { + foo: true, + }, + }; + + collector.record(event); + collector.record(event); + + jest.runAllTimers(); + + const item = localStorage.getItem('clerk_telemetry_throttler'); + assert(item); + const expectedKey = '["","TEST_EVENT",true,"development",null,null]'; + + expect(JSON.parse(item)[expectedKey]).toEqual(expect.any(Number)); + + fetchSpy.mockRestore(); + }); }); }); diff --git a/packages/shared/src/telemetry/collector.ts b/packages/shared/src/telemetry/collector.ts index 2122c7e2da..839b2cf979 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 { TelemetryEventThrottler } from './throttler'; import type { TelemetryCollectorOptions, TelemetryEvent, TelemetryEventRaw } from './types'; type TelemetryCollectorConfig = Pick< @@ -43,6 +44,7 @@ const DEFAULT_CONFIG: Partial = { export class TelemetryCollector { #config: Required; + #eventThrottler: TelemetryEventThrottler; #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.#eventThrottler = new TelemetryEventThrottler(); } get isEnabled(): boolean { @@ -110,7 +114,7 @@ export class TelemetryCollector { this.#logEvent(preparedPayload.event, preparedPayload); - if (!this.#shouldRecord(event.eventSamplingRate)) { + if (!this.#shouldRecord(preparedPayload, event.eventSamplingRate)) { return; } @@ -119,13 +123,21 @@ export class TelemetryCollector { this.#scheduleFlush(); } - #shouldRecord(eventSamplingRate?: number): boolean { + #shouldRecord(preparedPayload: TelemetryEvent, eventSamplingRate?: number) { + return this.isEnabled && !this.isDebug && this.#shouldBeSampled(preparedPayload, eventSamplingRate); + } + + #shouldBeSampled(preparedPayload: TelemetryEvent, eventSamplingRate?: number) { const randomSeed = Math.random(); - const shouldBeSampled = - randomSeed <= this.#config.samplingRate && - (typeof eventSamplingRate === 'undefined' || randomSeed <= eventSamplingRate); - return this.isEnabled && !this.isDebug && shouldBeSampled; + if (this.#eventThrottler.isEventThrottled(preparedPayload)) { + return false; + } + + return ( + randomSeed <= this.#config.samplingRate && + (typeof eventSamplingRate === 'undefined' || randomSeed <= eventSamplingRate) + ); } #scheduleFlush(): void { diff --git a/packages/shared/src/telemetry/throttler.ts b/packages/shared/src/telemetry/throttler.ts new file mode 100644 index 0000000000..53dc3d72f1 --- /dev/null +++ b/packages/shared/src/telemetry/throttler.ts @@ -0,0 +1,112 @@ +import type { TelemetryEvent } from './types'; + +type TtlInMilliseconds = number; + +const DEFAULT_CACHE_TTL_MS = 86400000; // 24 hours + +/** + * Manages throttling for telemetry events using the browser's localStorage to + * mitigate event flooding in frequently executed code paths. + */ +export class TelemetryEventThrottler { + #storageKey = 'clerk_telemetry_throttler'; + #cacheTtl = DEFAULT_CACHE_TTL_MS; + + isEventThrottled(payload: TelemetryEvent): boolean { + if (!this.#isValidBrowser) { + return false; + } + + const now = Date.now(); + const key = this.#generateKey(payload); + const entry = this.#cache?.[key]; + + if (!entry) { + const updatedCache = { + ...this.#cache, + [key]: now, + }; + + localStorage.setItem(this.#storageKey, JSON.stringify(updatedCache)); + } + + const shouldInvalidate = entry && now - entry > this.#cacheTtl; + if (shouldInvalidate) { + const updatedCache = this.#cache; + delete updatedCache[key]; + + localStorage.setItem(this.#storageKey, JSON.stringify(updatedCache)); + } + + return !!entry; + } + + /** + * Generates a consistent unique key for telemetry events by sorting payload properties. + * This ensures that payloads with identical content in different orders produce the same key. + */ + #generateKey(event: TelemetryEvent): string { + const { sk: _sk, pk: _pk, payload, ...rest } = event; + + const sanitizedEvent = { + ...payload, + ...rest, + }; + + return JSON.stringify( + Object.keys({ + ...payload, + ...rest, + }) + .sort() + .map(key => sanitizedEvent[key]), + ); + } + + get #cache(): Record | undefined { + const cacheString = localStorage.getItem(this.#storageKey); + + if (!cacheString) { + return {}; + } + + 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 #isValidBrowser(): boolean { + if (typeof window === 'undefined') { + return false; + } + + const storage = window.localStorage; + if (!storage) { + return false; + } + + try { + const testKey = 'test'; + storage.setItem(testKey, testKey); + storage.removeItem(testKey); + + return true; + } 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; + } + } +}