Skip to content

Commit

Permalink
feat(shared): Introduce client-side caching for telemetry events (#3287)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
LauraBeatris authored Apr 30, 2024
1 parent 19cd424 commit a78bc44
Show file tree
Hide file tree
Showing 4 changed files with 312 additions and 37 deletions.
5 changes: 5 additions & 0 deletions .changeset/metal-bulldogs-destroy.md
Original file line number Diff line number Diff line change
@@ -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.
208 changes: 177 additions & 31 deletions packages/shared/src/__tests__/telemetry.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
});
Expand All @@ -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(() => {});

Expand Down Expand Up @@ -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(() => {});

Expand Down Expand Up @@ -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,
});
Expand All @@ -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();
});
});
});
24 changes: 18 additions & 6 deletions packages/shared/src/telemetry/collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down Expand Up @@ -43,6 +44,7 @@ const DEFAULT_CONFIG: Partial<TelemetryCollectorConfig> = {

export class TelemetryCollector {
#config: Required<TelemetryCollectorConfig>;
#eventThrottler: TelemetryEventThrottler;
#metadata: TelemetryMetadata = {} as TelemetryMetadata;
#buffer: TelemetryEvent[] = [];
#pendingFlush: any;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -110,7 +114,7 @@ export class TelemetryCollector {

this.#logEvent(preparedPayload.event, preparedPayload);

if (!this.#shouldRecord(event.eventSamplingRate)) {
if (!this.#shouldRecord(preparedPayload, event.eventSamplingRate)) {
return;
}

Expand All @@ -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 {
Expand Down
Loading

0 comments on commit a78bc44

Please sign in to comment.