From b9a64f02b8b61f9b27684af62e50a205eaabbbd2 Mon Sep 17 00:00:00 2001 From: Ryan Golden Date: Wed, 26 Aug 2026 13:51:55 -0500 Subject: [PATCH 1/2] feat(reporter): add per-source identify caching with segment source-specific files --- src/common/impl/reporter.ts | 20 ++-- src/node/redHatServiceNodeProvider.ts | 7 +- src/tests/reporter.test.ts | 106 ++++++++++++++++++ .../redHatServiceWebWorkerProvider.ts | 7 +- 4 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 src/tests/reporter.test.ts diff --git a/src/common/impl/reporter.ts b/src/common/impl/reporter.ts index 9853e8d..c73dce5 100644 --- a/src/common/impl/reporter.ts +++ b/src/common/impl/reporter.ts @@ -1,5 +1,4 @@ import type { CoreAnalytics } from '@segment/analytics-core'; -import { sha1 } from 'object-hash'; import type { AnalyticsEvent } from '../api/analyticsEvent'; import type { CacheService } from '../api/cacheService'; import type { IReporter } from '../api/reporter'; @@ -12,6 +11,7 @@ export class Reporter implements IReporter { constructor( private analytics?: CoreAnalytics, private cacheService?: CacheService, + private writeKey?: string, ) {} public async report(event: AnalyticsEvent): Promise { @@ -22,16 +22,17 @@ export class Reporter implements IReporter { try { switch (event.type) { case 'identify': { - //Avoid identifying the user several times, until some data has changed. - const hash = sha1(payloadString); - const cached = await this.cacheService?.get('identify'); - if (hash === cached) { - Logger.log(`Skipping 'identify' event! Already sent:\n${payloadString}`); + // Skip if we already sent an identify event today. + const identifyCacheName = this.getIdentifyCacheName(); + const cachedDate = await this.cacheService?.get(identifyCacheName); + const today = new Date().toDateString(); + if (cachedDate === today) { + Logger.log(`Skipping 'identify' event! Already sent today:\n${payloadString}`); return; } Logger.log(`Sending 'identify' event with\n${payloadString}`); await this.analytics?.identify(event); - this.cacheService?.put('identify', hash); + await this.cacheService?.put(identifyCacheName, today); break; } case 'track': @@ -62,6 +63,11 @@ export class Reporter implements IReporter { return this.analytics.closeAndFlush(); } } + + private getIdentifyCacheName(): string { + // Fall back to "identify" when no writeKey is set (backward compat). + return this.writeKey ? `${this.writeKey}-identify` : 'identify'; + } } interface Flusheable { diff --git a/src/node/redHatServiceNodeProvider.ts b/src/node/redHatServiceNodeProvider.ts index 15a0bae..9083f46 100644 --- a/src/node/redHatServiceNodeProvider.ts +++ b/src/node/redHatServiceNodeProvider.ts @@ -4,6 +4,7 @@ import { EventCacheService } from '../common/impl/eventCacheService'; import { Reporter } from '../common/impl/reporter'; import { TelemetryServiceBuilder } from '../common/telemetryServiceBuilder'; import { getExtension, getPackageJson } from '../common/utils/extensions'; +import { getSegmentKey } from '../common/utils/keyLocator'; import { FileSystemStorageService } from '../common/vscode/fileSystemStorageService'; import { AbstractRedHatServiceProvider } from '../common/vscode/redhatServiceInitializer'; import { IdManagerFactory } from './idManagerFactory'; @@ -15,7 +16,11 @@ export class RedHatServiceNodeProvider extends AbstractRedHatServiceProvider { const extensionId = extensionInfo.id; const packageJson = getPackageJson(extensionInfo); const storageService = new FileSystemStorageService(this.getCachePath()); - const reporter = new Reporter(this.getSegmentApi(packageJson), new EventCacheService(storageService)); + const reporter = new Reporter( + this.getSegmentApi(packageJson), + new EventCacheService(storageService), + getSegmentKey(packageJson), + ); const idManager = IdManagerFactory.getIdManager(); const builder = new TelemetryServiceBuilder(packageJson) .setContext(this.context) diff --git a/src/tests/reporter.test.ts b/src/tests/reporter.test.ts new file mode 100644 index 0000000..151d0bf --- /dev/null +++ b/src/tests/reporter.test.ts @@ -0,0 +1,106 @@ +import * as assert from 'node:assert'; +import type { AnalyticsEvent } from '../common/api/analyticsEvent'; +import type { CacheService } from '../common/api/cacheService'; +import { Reporter } from '../common/impl/reporter'; + +class MockCacheService implements CacheService { + private store: Map = new Map(); + + async get(key: string): Promise { + return this.store.get(key); + } + + async put(key: string, value: string): Promise { + this.store.set(key, value); + return true; + } + + keys(): string[] { + return Array.from(this.store.keys()); + } +} + +class MockAnalytics { + identifyCalls: AnalyticsEvent[] = []; + async identify(event: AnalyticsEvent): Promise { + this.identifyCalls.push(event); + } + async track(): Promise {} + async page(): Promise {} +} + +const identifyEvent: AnalyticsEvent = { type: 'identify', userId: 'user1', traits: { name: 'Test' } } as any; + +suite('Reporter identify caching', () => { + test('sends identify on first call', async () => { + const analytics = new MockAnalytics(); + const cache = new MockCacheService(); + const reporter = new Reporter(analytics as any, cache, 'key-abc'); + + await reporter.report(identifyEvent); + + assert.strictEqual(analytics.identifyCalls.length, 1); + }); + + test('skips duplicate identify sent on the same day', async () => { + const analytics = new MockAnalytics(); + const cache = new MockCacheService(); + const reporter = new Reporter(analytics as any, cache, 'key-abc'); + + await reporter.report(identifyEvent); + await reporter.report(identifyEvent); + + assert.strictEqual(analytics.identifyCalls.length, 1); + }); + + test('sends identify again after cache is cleared (new day simulation)', async () => { + const analytics = new MockAnalytics(); + const cache = new MockCacheService(); + const reporter = new Reporter(analytics as any, cache, 'key-abc'); + + await reporter.report(identifyEvent); + // Simulate a new day by overwriting the cached date with a stale value. + await cache.put('key-abc-identify', 'Mon Jan 01 2000'); + await reporter.report(identifyEvent); + + assert.strictEqual(analytics.identifyCalls.length, 2); + }); + + test('uses per-source cache keys — different writeKeys do not share cache', async () => { + const analyticsA = new MockAnalytics(); + const analyticsB = new MockAnalytics(); + const cache = new MockCacheService(); + + const reporterA = new Reporter(analyticsA as any, cache, 'key-ext-a'); + const reporterB = new Reporter(analyticsB as any, cache, 'key-ext-b'); + + await reporterA.report(identifyEvent); + // reporterB has a different writeKey so its cache entry is independent. + await reporterB.report(identifyEvent); + + assert.strictEqual(analyticsA.identifyCalls.length, 1); + assert.strictEqual(analyticsB.identifyCalls.length, 1); + assert.ok(cache.keys().includes('key-ext-a-identify')); + assert.ok(cache.keys().includes('key-ext-b-identify')); + }); + + test('falls back to "identify" cache key when writeKey is undefined', async () => { + const analytics = new MockAnalytics(); + const cache = new MockCacheService(); + const reporter = new Reporter(analytics as any, cache, undefined); + + await reporter.report(identifyEvent); + + assert.ok(cache.keys().includes('identify')); + assert.ok(!cache.keys().some((k) => k.startsWith('undefined'))); + }); + + test('skips send when no analytics instance provided', async () => { + const cache = new MockCacheService(); + const reporter = new Reporter(undefined, cache, 'key-abc'); + + // Should not throw and cache should remain empty. + await reporter.report(identifyEvent); + assert.strictEqual(cache.keys().length, 0); + }); +}); diff --git a/src/webworker/redHatServiceWebWorkerProvider.ts b/src/webworker/redHatServiceWebWorkerProvider.ts index 1a11010..ac7dce1 100644 --- a/src/webworker/redHatServiceWebWorkerProvider.ts +++ b/src/webworker/redHatServiceWebWorkerProvider.ts @@ -4,6 +4,7 @@ import { EventCacheService } from '../common/impl/eventCacheService'; import { Reporter } from '../common/impl/reporter'; import { TelemetryServiceBuilder } from '../common/telemetryServiceBuilder'; import { getExtension, getPackageJson } from '../common/utils/extensions'; +import { getSegmentKey } from '../common/utils/keyLocator'; import { FileSystemStorageService } from '../common/vscode/fileSystemStorageService'; import { AbstractRedHatServiceProvider } from '../common/vscode/redhatServiceInitializer'; import { getEnvironment } from './platform'; @@ -15,7 +16,11 @@ export class RedHatServiceWebWorkerProvider extends AbstractRedHatServiceProvide const extensionId = extensionInfo.id; const packageJson = getPackageJson(extensionInfo); const storageService = new FileSystemStorageService(this.getCachePath()); - const reporter = new Reporter(this.getSegmentApi(packageJson), new EventCacheService(storageService)); + const reporter = new Reporter( + this.getSegmentApi(packageJson), + new EventCacheService(storageService), + getSegmentKey(packageJson), + ); const idManager = new VFSSystemIdProvider(storageService); const builder = new TelemetryServiceBuilder(packageJson) .setContext(this.context) From 57e001b4d4989ebb93869da4db948524c077085f Mon Sep 17 00:00:00 2001 From: Ryan Golden Date: Mon, 31 Aug 2026 13:33:27 -0500 Subject: [PATCH 2/2] fix(reporter): serialize identify cache reads/writes and restore sha1 dedup - Add identifyInFlight promise chain to prevent concurrent report() calls from both passing the cache check before either completes the write - Restore sha1(payload) deduplication so trait changes mid-session trigger a resend immediately, rather than being suppressed until the next day - Preserve per-source cache key (writeKey + '-identify') from current branch - Add .catch() on the chain so a failed identify does not poison subsequent calls - Fix missing vitest suite/test imports in reporter.test.ts - Update tests to reflect sha1-based dedup contract Signed-off-by: Ryan Golden --- src/common/impl/reporter.ts | 26 +++++++++++++++++--------- src/tests/reporter.test.ts | 10 +++++----- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/common/impl/reporter.ts b/src/common/impl/reporter.ts index c73dce5..85c3c0f 100644 --- a/src/common/impl/reporter.ts +++ b/src/common/impl/reporter.ts @@ -1,4 +1,5 @@ import type { CoreAnalytics } from '@segment/analytics-core'; +import { sha1 } from 'object-hash'; import type { AnalyticsEvent } from '../api/analyticsEvent'; import type { CacheService } from '../api/cacheService'; import type { IReporter } from '../api/reporter'; @@ -8,6 +9,8 @@ import { Logger } from '../utils/logger'; * Sends Telemetry events to a segment.io backend */ export class Reporter implements IReporter { + private identifyInFlight: Promise | undefined; + constructor( private analytics?: CoreAnalytics, private cacheService?: CacheService, @@ -24,15 +27,20 @@ export class Reporter implements IReporter { case 'identify': { // Skip if we already sent an identify event today. const identifyCacheName = this.getIdentifyCacheName(); - const cachedDate = await this.cacheService?.get(identifyCacheName); - const today = new Date().toDateString(); - if (cachedDate === today) { - Logger.log(`Skipping 'identify' event! Already sent today:\n${payloadString}`); - return; - } - Logger.log(`Sending 'identify' event with\n${payloadString}`); - await this.analytics?.identify(event); - await this.cacheService?.put(identifyCacheName, today); + this.identifyInFlight = (this.identifyInFlight ?? Promise.resolve()) + .then(async () => { + const hash = sha1(payloadString); + const cached = await this.cacheService?.get(identifyCacheName); + if (hash === cached) { + Logger.log(`Skipping 'identify' event! Already sent:\n${payloadString}`); + return; + } + Logger.log(`Sending 'identify' event with\n${payloadString}`); + await this.analytics?.identify(event); + await this.cacheService?.put(identifyCacheName, hash); + }) + .catch((e) => Logger.log(`Failed to send 'identify' event ${toErrorMessage(e)}`)); + await this.identifyInFlight; break; } case 'track': diff --git a/src/tests/reporter.test.ts b/src/tests/reporter.test.ts index 151d0bf..518a115 100644 --- a/src/tests/reporter.test.ts +++ b/src/tests/reporter.test.ts @@ -1,4 +1,5 @@ import * as assert from 'node:assert'; +import { suite, test } from 'vitest'; import type { AnalyticsEvent } from '../common/api/analyticsEvent'; import type { CacheService } from '../common/api/cacheService'; import { Reporter } from '../common/impl/reporter'; @@ -42,7 +43,7 @@ suite('Reporter identify caching', () => { assert.strictEqual(analytics.identifyCalls.length, 1); }); - test('skips duplicate identify sent on the same day', async () => { + test('skips duplicate identify with identical payload', async () => { const analytics = new MockAnalytics(); const cache = new MockCacheService(); const reporter = new Reporter(analytics as any, cache, 'key-abc'); @@ -53,15 +54,14 @@ suite('Reporter identify caching', () => { assert.strictEqual(analytics.identifyCalls.length, 1); }); - test('sends identify again after cache is cleared (new day simulation)', async () => { + test('resends identify when traits change', async () => { const analytics = new MockAnalytics(); const cache = new MockCacheService(); const reporter = new Reporter(analytics as any, cache, 'key-abc'); + const updatedEvent: AnalyticsEvent = { type: 'identify', userId: 'user1', traits: { name: 'Updated' } } as any; await reporter.report(identifyEvent); - // Simulate a new day by overwriting the cached date with a stale value. - await cache.put('key-abc-identify', 'Mon Jan 01 2000'); - await reporter.report(identifyEvent); + await reporter.report(updatedEvent); assert.strictEqual(analytics.identifyCalls.length, 2); });