From 0030f222cc0235f4b27c02af9894af926a1a34b7 Mon Sep 17 00:00:00 2001 From: Nishant Arora <1895906+whizzzkid@users.noreply.github.com> Date: Fri, 8 Dec 2023 05:30:25 -0700 Subject: [PATCH] fix(telemetry): local event count store Signed-off-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com> --- add-on/src/lib/telemetry.ts | 1 - add-on/src/lib/trackers/requestTracker.ts | 83 ++++++++++++++++--- .../lib/trackers/requestTrackers.test.ts | 31 ++++--- 3 files changed, 90 insertions(+), 25 deletions(-) diff --git a/add-on/src/lib/telemetry.ts b/add-on/src/lib/telemetry.ts index 6b3eca5f0..6987608c6 100644 --- a/add-on/src/lib/telemetry.ts +++ b/add-on/src/lib/telemetry.ts @@ -57,7 +57,6 @@ export function trackView (view: string, segments: Record): void * TrackView is a wrapper around ignite-metrics trackView * * @param event - * @param segments */ export function trackEvent (event: CountlyEvent): void { log('trackEvent called for event: ', event) diff --git a/add-on/src/lib/trackers/requestTracker.ts b/add-on/src/lib/trackers/requestTracker.ts index bd4ba9e8b..baa3523da 100644 --- a/add-on/src/lib/trackers/requestTracker.ts +++ b/add-on/src/lib/trackers/requestTracker.ts @@ -1,15 +1,21 @@ import debug from 'debug' -import type browser from 'webextension-polyfill' +import browser from 'webextension-polyfill' import { trackEvent } from '../telemetry.js' export const DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL = 1000 * 60 * 60 +export const REQUEST_TRACKER_SYNC_INTERVAL = 1000 * 60 * 60 * 24 +export const REQUEST_TRACKER_LOCAL_STORAGE_KEY = 'request-tracker' + +interface RequestTrackerPersistedState { + lastSync: number + requestTypeStore: { [key in browser.WebRequest.ResourceType]?: number } +} export class RequestTracker { private readonly eventKey: 'url-observed' | 'url-resolved' private readonly flushInterval: number private readonly log: debug.Debugger & { error?: debug.Debugger } - private lastSync: number = Date.now() - private requestTypeStore: { [key in browser.WebRequest.ResourceType]?: number } = {} + private requestTypeStore: RequestTrackerPersistedState['requestTypeStore'] = {} constructor (eventKey: 'url-observed' | 'url-resolved', flushInterval = DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL) { this.eventKey = eventKey @@ -24,27 +30,82 @@ export class RequestTracker { this.requestTypeStore[type] = (this.requestTypeStore[type] ?? 0) + 1 } - private flushStore (): void { - this.log('flushing') - const count = Object.values(this.requestTypeStore).reduce((a, b): number => a + b, 0) + private async flushStoreToMemory (): Promise { + this.log('flushing to memory') + + const persistedState = await browser.storage.local.get(REQUEST_TRACKER_LOCAL_STORAGE_KEY) as RequestTrackerPersistedState ?? { + lastSync: Date.now(), + requestTypeStore: {} + } + + // merge + const { lastSync, requestTypeStore } = persistedState + + const mergedRequestTypeKeys: Set = new Set([ + ...Object.keys(requestTypeStore) as browser.WebRequest.ResourceType[], + ...Object.keys(this.requestTypeStore) as browser.WebRequest.ResourceType[] + ]) + + const mergedRequestTypeStore = Object.fromEntries([...mergedRequestTypeKeys].map((key): [ + browser.WebRequest.ResourceType, + number + ] => ([ + key, + (requestTypeStore?.[key] ?? 0) + (this.requestTypeStore?.[key] ?? 0) + ]))) + + await browser.storage.local.set({ + [REQUEST_TRACKER_LOCAL_STORAGE_KEY]: { + lastSync, + requestTypeStore: mergedRequestTypeStore + } + }) + + // reset + this.requestTypeStore = {} + await this.syncEventsToTelemetry() + } + + private async syncEventsToTelemetry (): Promise { + this.log('syncing') + const currentTimestamp = Date.now() + const persistedState = await browser.storage.local.get(REQUEST_TRACKER_LOCAL_STORAGE_KEY) as RequestTrackerPersistedState + const { lastSync, requestTypeStore } = persistedState + + // skip if we already synced recently + if (lastSync + REQUEST_TRACKER_SYNC_INTERVAL > currentTimestamp) { + this.log('sync skipped') + return + } + + // skip if there is nothing to sync + const count = Object.values(requestTypeStore).reduce((a, b): number => a + b, 0) + if (count === 0) { this.log('nothing to flush') return } + + // sync trackEvent({ key: this.eventKey, count, - dur: Date.now() - this.lastSync, + dur: currentTimestamp - lastSync, segmentation: Object.assign({}, this.requestTypeStore) as unknown as Record }) + // reset - this.lastSync = Date.now() - this.requestTypeStore = {} + await browser.storage.local.set({ + [REQUEST_TRACKER_LOCAL_STORAGE_KEY]: { + lastSync: currentTimestamp, + requestTypeStore: {} + } + }) } private setupFlushScheduler (): void { - setTimeout(() => { - this.flushStore() + setTimeout(async () => { + await this.flushStoreToMemory() this.setupFlushScheduler() }, this.flushInterval) } diff --git a/test/functional/lib/trackers/requestTrackers.test.ts b/test/functional/lib/trackers/requestTrackers.test.ts index 8f3d52cff..2245143e0 100644 --- a/test/functional/lib/trackers/requestTrackers.test.ts +++ b/test/functional/lib/trackers/requestTrackers.test.ts @@ -2,14 +2,14 @@ import { expect } from 'chai'; import sinon from 'sinon'; import browser from 'sinon-chrome'; import PatchedCountly from 'countly-sdk-web' -import { DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL, RequestTracker } from './../../../../add-on/src/lib/trackers/requestTracker.js' +import { DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL, REQUEST_TRACKER_SYNC_INTERVAL, RequestTracker } from './../../../../add-on/src/lib/trackers/requestTracker.js' const sinonSandBox = sinon.createSandbox() describe('lib/trackers/requestTracker', () => { - let requestTracker: RequestTracker let countlySDKStub: sinon.SinonStub let clock: sinon.SinonFakeTimers + let requestTracker before(() => { clock = sinonSandBox.useFakeTimers() @@ -18,6 +18,8 @@ describe('lib/trackers/requestTracker', () => { afterEach(() => { sinonSandBox.resetHistory() + browser.storage.local.get.reset() + browser.storage.local.set.reset() }) describe('url-observed', () => { @@ -26,13 +28,15 @@ describe('lib/trackers/requestTracker', () => { }) it('should init a Tracker', () => { - expect(requestTracker).to.be.instanceOf(RequestTracker) expect(requestTracker).to.have.property('track') }) - it('should track a request', async () => { - await requestTracker.track({ type: 'main_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) + it.only('should track a request', async () => { + requestTracker.track({ type: 'main_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) + browser.storage.local.get.returns(null) clock.tick(DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL) + browser.storage.local.get.returns({ lastSync: Date.now(), requestTypeStore: { main_frame: 1 } }) + clock.tick(REQUEST_TRACKER_SYNC_INTERVAL) sinon.assert.calledWith(countlySDKStub.add_event, { key: 'url-observed', count: 1, @@ -44,9 +48,9 @@ describe('lib/trackers/requestTracker', () => { }) it('should track multiple requests', async () => { - await requestTracker.track({ type: 'main_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) - await requestTracker.track({ type: 'sub_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) - await requestTracker.track({ type: 'xmlHTTPRequest' } as browser.WebRequest.OnBeforeRequestDetailsType) + requestTracker.track({ type: 'main_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) + requestTracker.track({ type: 'sub_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) + requestTracker.track({ type: 'xmlHTTPRequest' } as browser.WebRequest.OnBeforeRequestDetailsType) clock.tick(DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL) sinon.assert.calledWith(countlySDKStub.add_event, { key: 'url-observed', @@ -78,8 +82,9 @@ describe('lib/trackers/requestTracker', () => { }) it('should track a request', async () => { - await requestTracker.track({ type: 'main_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) - clock.tick(DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL) + requestTracker.track({ type: 'main_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) + clock.tick(DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL + 1000) + expect(browser.storage.local.set.lastCall.args).to.deep.equal([]) sinon.assert.calledWith(countlySDKStub.add_event, { key: 'url-resolved', count: 1, @@ -91,9 +96,9 @@ describe('lib/trackers/requestTracker', () => { }) it('should track multiple requests', async () => { - await requestTracker.track({ type: 'main_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) - await requestTracker.track({ type: 'sub_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) - await requestTracker.track({ type: 'xmlHTTPRequest' } as browser.WebRequest.OnBeforeRequestDetailsType) + requestTracker.track({ type: 'main_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) + requestTracker.track({ type: 'sub_frame' } as browser.WebRequest.OnBeforeRequestDetailsType) + requestTracker.track({ type: 'xmlHTTPRequest' } as browser.WebRequest.OnBeforeRequestDetailsType) clock.tick(DEFAULT_REQUEST_TRACKER_FLUSH_INTERVAL) sinon.assert.calledWith(countlySDKStub.add_event, { key: 'url-resolved',