From e6d46fd599d069d076ec25aca03bdee0462c8f5c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 6 Dec 2024 17:19:08 -0300 Subject: [PATCH 01/10] Add 'track' property in ImpressionDTO and 'trackImpressions' property in ISplit --- src/dtos/types.ts | 3 ++- src/evaluator/index.ts | 2 ++ src/evaluator/types.ts | 2 +- src/sdkClient/client.ts | 5 +++-- types/splitio.d.ts | 1 + 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/dtos/types.ts b/src/dtos/types.ts index dce1d12d..41b0edab 100644 --- a/src/dtos/types.ts +++ b/src/dtos/types.ts @@ -208,7 +208,8 @@ export interface ISplit { configurations?: { [treatmentName: string]: string }, - sets?: string[] + sets?: string[], + trackImpressions?: boolean } // Split definition used in offline mode diff --git a/src/evaluator/index.ts b/src/evaluator/index.ts index c0576019..d76a9d93 100644 --- a/src/evaluator/index.ts +++ b/src/evaluator/index.ts @@ -156,12 +156,14 @@ function getEvaluation( return evaluation.then(result => { result.changeNumber = split.getChangeNumber(); result.config = splitJSON.configurations && splitJSON.configurations[result.treatment] || null; + result.track = splitJSON.trackImpressions; return result; }); } else { evaluation.changeNumber = split.getChangeNumber(); // Always sync and optional evaluation.config = splitJSON.configurations && splitJSON.configurations[evaluation.treatment] || null; + evaluation.track = splitJSON.trackImpressions; } } diff --git a/src/evaluator/types.ts b/src/evaluator/types.ts index 92e3446d..4c968fbd 100644 --- a/src/evaluator/types.ts +++ b/src/evaluator/types.ts @@ -25,7 +25,7 @@ export interface IEvaluation { config?: string | null } -export type IEvaluationResult = IEvaluation & { treatment: string } +export type IEvaluationResult = IEvaluation & { treatment: string; track?: boolean } export type ISplitEvaluator = (log: ILogger, key: SplitIO.SplitKey, splitName: string, attributes: SplitIO.Attributes | undefined, storage: IStorageSync | IStorageAsync) => MaybeThenable diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 01c5053d..0bd00f97 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -133,7 +133,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const matchingKey = getMatching(key); const bucketingKey = getBucketing(key); - const { treatment, label, changeNumber, config = null } = evaluation; + const { treatment, label, changeNumber, config = null, track } = evaluation; log.info(IMPRESSION, [featureFlagName, matchingKey, treatment, label]); if (validateSplitExistence(log, readinessManager, featureFlagName, label, invokingMethodName)) { @@ -145,7 +145,8 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl time: Date.now(), bucketingKey, label, - changeNumber: changeNumber as number + changeNumber: changeNumber as number, + track }); } diff --git a/types/splitio.d.ts b/types/splitio.d.ts index f2faa2df..d520e64c 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -811,6 +811,7 @@ declare namespace SplitIO { label: string; changeNumber: number; pt?: number; + track?: boolean; } /** * Object with information about an impression. It contains the generated impression DTO as well as From b4c0350a64e79a07e32fd2911b3ec2a7c57b9abd Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 9 Dec 2024 16:12:29 -0300 Subject: [PATCH 02/10] Refactor storages and submitter, now that 'impressionCounts' and 'uniqueKeys' caches will be required --- src/listeners/browser.ts | 4 +-- src/storages/inLocalStorage/index.ts | 9 ++--- src/storages/inMemory/InMemoryStorage.ts | 8 ++--- src/storages/inMemory/InMemoryStorageCS.ts | 9 ++--- src/storages/inRedis/index.ts | 20 +++++------ src/storages/pluggable/index.ts | 36 +++++++++---------- src/storages/types.ts | 4 +-- .../submitters/impressionCountsSubmitter.ts | 6 ++-- src/sync/submitters/submitterManager.ts | 7 ++-- src/sync/submitters/uniqueKeysSubmitter.ts | 5 ++- 10 files changed, 50 insertions(+), 58 deletions(-) diff --git a/src/listeners/browser.ts b/src/listeners/browser.ts index 12f13b74..ffd86004 100644 --- a/src/listeners/browser.ts +++ b/src/listeners/browser.ts @@ -8,7 +8,6 @@ import { IResponse, ISplitApi } from '../services/types'; import { ISettings } from '../types'; import SplitIO from '../../types/splitio'; import { ImpressionsPayload } from '../sync/submitters/types'; -import { OPTIMIZED, DEBUG, NONE } from '../utils/constants'; import { objectAssign } from '../utils/lang/objectAssign'; import { CLEANUP_REGISTERING, CLEANUP_DEREGISTERING } from '../logger/constants'; import { ISyncManager } from '../sync/types'; @@ -78,10 +77,9 @@ export class BrowserSignalListener implements ISignalListener { // Flush impressions & events data if there is user consent if (isConsentGranted(this.settings)) { - const sim = this.settings.sync.impressionsMode; const extraMetadata = { // sim stands for Sync/Split Impressions Mode - sim: sim === OPTIMIZED ? OPTIMIZED : sim === DEBUG ? DEBUG : NONE + sim: this.settings.sync.impressionsMode }; this._flushData(events + '/testImpressions/beacon', this.storage.impressions, this.serviceApi.postTestImpressionsBulk, this.fromImpressionsCollector, extraMetadata); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 871be592..c621141d 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -10,7 +10,7 @@ import { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser'; import { InMemoryStorageCSFactory } from '../inMemory/InMemoryStorageCS'; import { LOG_PREFIX } from './constants'; -import { DEBUG, NONE, STORAGE_LOCALSTORAGE } from '../../utils/constants'; +import { STORAGE_LOCALSTORAGE } from '../../utils/constants'; import { shouldRecordTelemetry, TelemetryCacheInMemory } from '../inMemory/TelemetryCacheInMemory'; import { UniqueKeysCacheInMemoryCS } from '../inMemory/UniqueKeysCacheInMemoryCS'; import { getMatching } from '../../utils/key'; @@ -34,7 +34,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn return InMemoryStorageCSFactory(params); } - const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize, }, sync: { impressionsMode } } } = params; + const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize } } } = params; const matchingKey = getMatching(settings.core.key); const keys = new KeyBuilderCS(prefix, matchingKey); const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; @@ -48,10 +48,10 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn segments, largeSegments, impressions: new ImpressionsCacheInMemory(impressionsQueueSize), - impressionCounts: impressionsMode !== DEBUG ? new ImpressionCountsCacheInMemory() : undefined, + impressionCounts: new ImpressionCountsCacheInMemory(), events: new EventsCacheInMemory(eventsQueueSize), telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined, - uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined, + uniqueKeys: new UniqueKeysCacheInMemoryCS(), destroy() { }, @@ -66,6 +66,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn impressionCounts: this.impressionCounts, events: this.events, telemetry: this.telemetry, + uniqueKeys: this.uniqueKeys, destroy() { } }; diff --git a/src/storages/inMemory/InMemoryStorage.ts b/src/storages/inMemory/InMemoryStorage.ts index 565d37ba..b8833370 100644 --- a/src/storages/inMemory/InMemoryStorage.ts +++ b/src/storages/inMemory/InMemoryStorage.ts @@ -4,7 +4,7 @@ import { ImpressionsCacheInMemory } from './ImpressionsCacheInMemory'; import { EventsCacheInMemory } from './EventsCacheInMemory'; import { IStorageFactoryParams, IStorageSync } from '../types'; import { ImpressionCountsCacheInMemory } from './ImpressionCountsCacheInMemory'; -import { DEBUG, LOCALHOST_MODE, NONE, STORAGE_MEMORY } from '../../utils/constants'; +import { LOCALHOST_MODE, STORAGE_MEMORY } from '../../utils/constants'; import { shouldRecordTelemetry, TelemetryCacheInMemory } from './TelemetryCacheInMemory'; import { UniqueKeysCacheInMemory } from './UniqueKeysCacheInMemory'; @@ -14,7 +14,7 @@ import { UniqueKeysCacheInMemory } from './UniqueKeysCacheInMemory'; * @param params - parameters required by EventsCacheSync */ export function InMemoryStorageFactory(params: IStorageFactoryParams): IStorageSync { - const { settings: { scheduler: { impressionsQueueSize, eventsQueueSize, }, sync: { impressionsMode, __splitFiltersValidation } } } = params; + const { settings: { scheduler: { impressionsQueueSize, eventsQueueSize, }, sync: { __splitFiltersValidation } } } = params; const splits = new SplitsCacheInMemory(__splitFiltersValidation); const segments = new SegmentsCacheInMemory(); @@ -23,10 +23,10 @@ export function InMemoryStorageFactory(params: IStorageFactoryParams): IStorageS splits, segments, impressions: new ImpressionsCacheInMemory(impressionsQueueSize), - impressionCounts: impressionsMode !== DEBUG ? new ImpressionCountsCacheInMemory() : undefined, + impressionCounts: new ImpressionCountsCacheInMemory(), events: new EventsCacheInMemory(eventsQueueSize), telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined, - uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemory() : undefined, + uniqueKeys: new UniqueKeysCacheInMemory(), destroy() { } }; diff --git a/src/storages/inMemory/InMemoryStorageCS.ts b/src/storages/inMemory/InMemoryStorageCS.ts index 051bd9d6..09d62ac8 100644 --- a/src/storages/inMemory/InMemoryStorageCS.ts +++ b/src/storages/inMemory/InMemoryStorageCS.ts @@ -4,7 +4,7 @@ import { ImpressionsCacheInMemory } from './ImpressionsCacheInMemory'; import { EventsCacheInMemory } from './EventsCacheInMemory'; import { IStorageSync, IStorageFactoryParams } from '../types'; import { ImpressionCountsCacheInMemory } from './ImpressionCountsCacheInMemory'; -import { DEBUG, LOCALHOST_MODE, NONE, STORAGE_MEMORY } from '../../utils/constants'; +import { LOCALHOST_MODE, STORAGE_MEMORY } from '../../utils/constants'; import { shouldRecordTelemetry, TelemetryCacheInMemory } from './TelemetryCacheInMemory'; import { UniqueKeysCacheInMemoryCS } from './UniqueKeysCacheInMemoryCS'; @@ -14,7 +14,7 @@ import { UniqueKeysCacheInMemoryCS } from './UniqueKeysCacheInMemoryCS'; * @param params - parameters required by EventsCacheSync */ export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorageSync { - const { settings: { scheduler: { impressionsQueueSize, eventsQueueSize, }, sync: { impressionsMode, __splitFiltersValidation } } } = params; + const { settings: { scheduler: { impressionsQueueSize, eventsQueueSize }, sync: { __splitFiltersValidation } } } = params; const splits = new SplitsCacheInMemory(__splitFiltersValidation); const segments = new MySegmentsCacheInMemory(); @@ -25,10 +25,10 @@ export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorag segments, largeSegments, impressions: new ImpressionsCacheInMemory(impressionsQueueSize), - impressionCounts: impressionsMode !== DEBUG ? new ImpressionCountsCacheInMemory() : undefined, + impressionCounts: new ImpressionCountsCacheInMemory(), events: new EventsCacheInMemory(eventsQueueSize), telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined, - uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined, + uniqueKeys: new UniqueKeysCacheInMemoryCS(), destroy() { }, @@ -42,6 +42,7 @@ export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorag impressionCounts: this.impressionCounts, events: this.events, telemetry: this.telemetry, + uniqueKeys: this.uniqueKeys, destroy() { } }; diff --git a/src/storages/inRedis/index.ts b/src/storages/inRedis/index.ts index 80474209..e548142d 100644 --- a/src/storages/inRedis/index.ts +++ b/src/storages/inRedis/index.ts @@ -6,7 +6,7 @@ import { SplitsCacheInRedis } from './SplitsCacheInRedis'; import { SegmentsCacheInRedis } from './SegmentsCacheInRedis'; import { ImpressionsCacheInRedis } from './ImpressionsCacheInRedis'; import { EventsCacheInRedis } from './EventsCacheInRedis'; -import { DEBUG, NONE, STORAGE_REDIS } from '../../utils/constants'; +import { STORAGE_REDIS } from '../../utils/constants'; import { TelemetryCacheInRedis } from './TelemetryCacheInRedis'; import { UniqueKeysCacheInRedis } from './UniqueKeysCacheInRedis'; import { ImpressionCountsCacheInRedis } from './ImpressionCountsCacheInRedis'; @@ -30,19 +30,19 @@ export function InRedisStorage(options: InRedisStorageOptions = {}): IStorageAsy const prefix = validatePrefix(options.prefix); function InRedisStorageFactory(params: IStorageFactoryParams): IStorageAsync { - const { onReadyCb, settings, settings: { log, sync: { impressionsMode } } } = params; + const { onReadyCb, settings, settings: { log } } = params; const metadata = metadataBuilder(settings); const keys = new KeyBuilderSS(prefix, metadata); const redisClient: RedisAdapter = new RD(log, options.options || {}); const telemetry = new TelemetryCacheInRedis(log, keys, redisClient); - const impressionCountsCache = impressionsMode !== DEBUG ? new ImpressionCountsCacheInRedis(log, keys.buildImpressionsCountKey(), redisClient) : undefined; - const uniqueKeysCache = impressionsMode === NONE ? new UniqueKeysCacheInRedis(log, keys.buildUniqueKeysKey(), redisClient) : undefined; + const impressionCountsCache = new ImpressionCountsCacheInRedis(log, keys.buildImpressionsCountKey(), redisClient); + const uniqueKeysCache = new UniqueKeysCacheInRedis(log, keys.buildUniqueKeysKey(), redisClient); // subscription to Redis connect event in order to emit SDK_READY event on consumer mode redisClient.on('connect', () => { onReadyCb(); - if (impressionCountsCache) impressionCountsCache.start(); - if (uniqueKeysCache) uniqueKeysCache.start(); + impressionCountsCache.start(); + uniqueKeysCache.start(); // Synchronize config telemetry.recordConfig(); @@ -60,10 +60,10 @@ export function InRedisStorage(options: InRedisStorageOptions = {}): IStorageAsy // When using REDIS we should: // 1- Disconnect from the storage destroy(): Promise { - let promises = []; - if (impressionCountsCache) promises.push(impressionCountsCache.stop()); - if (uniqueKeysCache) promises.push(uniqueKeysCache.stop()); - return Promise.all(promises).then(() => { redisClient.disconnect(); }); + return Promise.all([ + impressionCountsCache.stop(), + uniqueKeysCache.stop() + ]).then(() => { redisClient.disconnect(); }); // @TODO check that caches works as expected when redisClient is disconnected } }; diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index 372eeeb4..ee8b1872 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -8,7 +8,7 @@ import { EventsCachePluggable } from './EventsCachePluggable'; import { wrapperAdapter, METHODS_TO_PROMISE_WRAP } from './wrapperAdapter'; import { isObject } from '../../utils/lang'; import { getStorageHash, validatePrefix } from '../KeyBuilder'; -import { CONSUMER_PARTIAL_MODE, DEBUG, NONE, STORAGE_PLUGGABLE } from '../../utils/constants'; +import { CONSUMER_PARTIAL_MODE, STORAGE_PLUGGABLE } from '../../utils/constants'; import { ImpressionsCacheInMemory } from '../inMemory/ImpressionsCacheInMemory'; import { EventsCacheInMemory } from '../inMemory/EventsCacheInMemory'; import { ImpressionCountsCacheInMemory } from '../inMemory/ImpressionCountsCacheInMemory'; @@ -63,35 +63,31 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn const prefix = validatePrefix(options.prefix); function PluggableStorageFactory(params: IStorageFactoryParams): IStorageAsync { - const { onReadyCb, settings, settings: { log, mode, sync: { impressionsMode }, scheduler: { impressionsQueueSize, eventsQueueSize } } } = params; + const { onReadyCb, settings, settings: { log, mode, scheduler: { impressionsQueueSize, eventsQueueSize } } } = params; const metadata = metadataBuilder(settings); const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); - const isSyncronizer = mode === undefined; // If mode is not defined, the synchronizer is running + const isSynchronizer = mode === undefined; // If mode is not defined, the synchronizer is running const isPartialConsumer = mode === CONSUMER_PARTIAL_MODE; - const telemetry = shouldRecordTelemetry(params) || isSyncronizer ? + const telemetry = shouldRecordTelemetry(params) || isSynchronizer ? isPartialConsumer ? new TelemetryCacheInMemory() : new TelemetryCachePluggable(log, keys, wrapper) : undefined; - const impressionCountsCache = impressionsMode !== DEBUG || isSyncronizer ? - isPartialConsumer ? - new ImpressionCountsCacheInMemory() : - new ImpressionCountsCachePluggable(log, keys.buildImpressionsCountKey(), wrapper) : - undefined; + const impressionCountsCache = isPartialConsumer ? + new ImpressionCountsCacheInMemory() : + new ImpressionCountsCachePluggable(log, keys.buildImpressionsCountKey(), wrapper); - const uniqueKeysCache = impressionsMode === NONE || isSyncronizer ? - isPartialConsumer ? - settings.core.key === undefined ? new UniqueKeysCacheInMemory() : new UniqueKeysCacheInMemoryCS() : - new UniqueKeysCachePluggable(log, keys.buildUniqueKeysKey(), wrapper) : - undefined; + const uniqueKeysCache = isPartialConsumer ? + settings.core.key === undefined ? new UniqueKeysCacheInMemory() : new UniqueKeysCacheInMemoryCS() : + new UniqueKeysCachePluggable(log, keys.buildUniqueKeysKey(), wrapper); // Connects to wrapper and emits SDK_READY event on main client const connectPromise = wrapper.connect().then(() => { - if (isSyncronizer) { + if (isSynchronizer) { // In standalone or producer mode, clear storage if SDK key or feature flag filter has changed return wrapper.get(keys.buildHashKey()).then((hash) => { const currentHash = getStorageHash(settings); @@ -106,8 +102,8 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn }); } else { // Start periodic flush of async storages if not running synchronizer (producer mode) - if (impressionCountsCache && (impressionCountsCache as ImpressionCountsCachePluggable).start) (impressionCountsCache as ImpressionCountsCachePluggable).start(); - if (uniqueKeysCache && (uniqueKeysCache as UniqueKeysCachePluggable).start) (uniqueKeysCache as UniqueKeysCachePluggable).start(); + if ((impressionCountsCache as ImpressionCountsCachePluggable).start) (impressionCountsCache as ImpressionCountsCachePluggable).start(); + if ((uniqueKeysCache as UniqueKeysCachePluggable).start) (uniqueKeysCache as UniqueKeysCachePluggable).start(); if (telemetry && (telemetry as ITelemetryCacheAsync).recordConfig) (telemetry as ITelemetryCacheAsync).recordConfig(); onReadyCb(); @@ -129,9 +125,9 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn // Stop periodic flush and disconnect the underlying storage destroy() { - return Promise.all(isSyncronizer ? [] : [ - impressionCountsCache && (impressionCountsCache as ImpressionCountsCachePluggable).stop && (impressionCountsCache as ImpressionCountsCachePluggable).stop(), - uniqueKeysCache && (uniqueKeysCache as UniqueKeysCachePluggable).stop && (uniqueKeysCache as UniqueKeysCachePluggable).stop(), + return Promise.all(isSynchronizer ? [] : [ + (impressionCountsCache as ImpressionCountsCachePluggable).stop && (impressionCountsCache as ImpressionCountsCachePluggable).stop(), + (uniqueKeysCache as UniqueKeysCachePluggable).stop && (uniqueKeysCache as UniqueKeysCachePluggable).stop(), ]).then(() => wrapper.disconnect()); }, diff --git a/src/storages/types.ts b/src/storages/types.ts index 5d7b40b6..638c4606 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -439,10 +439,10 @@ export interface IStorageBase< splits: TSplitsCache, segments: TSegmentsCache, impressions: TImpressionsCache, - impressionCounts?: TImpressionsCountCache, + impressionCounts: TImpressionsCountCache, events: TEventsCache, telemetry?: TTelemetryCache, - uniqueKeys?: TUniqueKeysCache, + uniqueKeys: TUniqueKeysCache, destroy(): void | Promise, shared?: (matchingKey: string, onReadyCb: (error?: any) => void) => this } diff --git a/src/sync/submitters/impressionCountsSubmitter.ts b/src/sync/submitters/impressionCountsSubmitter.ts index 48131021..fb02045c 100644 --- a/src/sync/submitters/impressionCountsSubmitter.ts +++ b/src/sync/submitters/impressionCountsSubmitter.ts @@ -39,8 +39,6 @@ export function impressionCountsSubmitterFactory(params: ISdkFactoryContextSync) storage: { impressionCounts } } = params; - if (impressionCounts) { - // retry impressions counts only once. - return submitterFactory(log, postTestImpressionsCount, impressionCounts, IMPRESSIONS_COUNT_RATE, 'impression counts', fromImpressionCountsCollector, 1); - } + // retry impressions counts only once. + return submitterFactory(log, postTestImpressionsCount, impressionCounts, IMPRESSIONS_COUNT_RATE, 'impression counts', fromImpressionCountsCollector, 1); } diff --git a/src/sync/submitters/submitterManager.ts b/src/sync/submitters/submitterManager.ts index 9313048c..bcff5f1b 100644 --- a/src/sync/submitters/submitterManager.ts +++ b/src/sync/submitters/submitterManager.ts @@ -10,13 +10,12 @@ export function submitterManagerFactory(params: ISdkFactoryContextSync): ISubmit const submitters = [ impressionsSubmitterFactory(params), - eventsSubmitterFactory(params) + eventsSubmitterFactory(params), + impressionCountsSubmitterFactory(params), + uniqueKeysSubmitterFactory(params) ]; - const impressionCountsSubmitter = impressionCountsSubmitterFactory(params); - if (impressionCountsSubmitter) submitters.push(impressionCountsSubmitter); const telemetrySubmitter = telemetrySubmitterFactory(params); - if (params.storage.uniqueKeys) submitters.push(uniqueKeysSubmitterFactory(params)); return { // `onlyTelemetry` true if SDK is created with userConsent not GRANTED diff --git a/src/sync/submitters/uniqueKeysSubmitter.ts b/src/sync/submitters/uniqueKeysSubmitter.ts index f51c90b8..f10a1aca 100644 --- a/src/sync/submitters/uniqueKeysSubmitter.ts +++ b/src/sync/submitters/uniqueKeysSubmitter.ts @@ -19,10 +19,10 @@ export function uniqueKeysSubmitterFactory(params: ISdkFactoryContextSync) { const isClientSide = key !== undefined; const postUniqueKeysBulk = isClientSide ? postUniqueKeysBulkCs : postUniqueKeysBulkSs; - const syncTask = submitterFactory(log, postUniqueKeysBulk, uniqueKeys!, UNIQUE_KEYS_RATE, DATA_NAME); + const syncTask = submitterFactory(log, postUniqueKeysBulk, uniqueKeys, UNIQUE_KEYS_RATE, DATA_NAME); // register unique keys submitter to be executed when uniqueKeys cache is full - uniqueKeys!.setOnFullQueueCb(() => { + uniqueKeys.setOnFullQueueCb(() => { if (syncTask.isRunning()) { log.info(SUBMITTERS_PUSH_FULL_QUEUE, [DATA_NAME]); syncTask.execute(); @@ -33,4 +33,3 @@ export function uniqueKeysSubmitterFactory(params: ISdkFactoryContextSync) { return syncTask; } - From c2f1e624aa4c06beb132208ca94b19b5dd80710a Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 9 Dec 2024 16:35:28 -0300 Subject: [PATCH 03/10] uniqueKeysTracker required --- src/sdkClient/__tests__/sdkClientMethod.spec.ts | 7 +++++-- src/sdkClient/__tests__/sdkClientMethodCS.spec.ts | 2 ++ src/sdkClient/sdkClient.ts | 2 +- src/sdkFactory/index.ts | 8 ++++---- src/sdkFactory/types.ts | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/sdkClient/__tests__/sdkClientMethod.spec.ts b/src/sdkClient/__tests__/sdkClientMethod.spec.ts index 068d0278..e2f53f83 100644 --- a/src/sdkClient/__tests__/sdkClientMethod.spec.ts +++ b/src/sdkClient/__tests__/sdkClientMethod.spec.ts @@ -16,7 +16,8 @@ const paramMocks = [ signalListener: undefined, settings: { mode: CONSUMER_MODE, log: loggerMock, core: { authorizationKey: 'sdk key '} }, telemetryTracker: telemetryTrackerFactory(), - clients: {} + clients: {}, + uniqueKeysTracker: { start: jest.fn(), stop: jest.fn() } }, // SyncManager (i.e., Sync SDK) and Signal listener { @@ -26,7 +27,8 @@ const paramMocks = [ signalListener: { stop: jest.fn() }, settings: { mode: STANDALONE_MODE, log: loggerMock, core: { authorizationKey: 'sdk key '} }, telemetryTracker: telemetryTrackerFactory(), - clients: {} + clients: {}, + uniqueKeysTracker: { start: jest.fn(), stop: jest.fn() } } ]; @@ -70,6 +72,7 @@ test.each(paramMocks)('sdkClientMethodFactory', (params, done: any) => { client.destroy().then(() => { expect(params.sdkReadinessManager.readinessManager.destroy).toBeCalledTimes(1); expect(params.storage.destroy).toBeCalledTimes(1); + expect(params.uniqueKeysTracker.stop).toBeCalledTimes(1); if (params.syncManager) { expect(params.syncManager.stop).toBeCalledTimes(1); diff --git a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts index 8822a21f..5924358f 100644 --- a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts +++ b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts @@ -46,6 +46,7 @@ const params = { settings: settingsWithKey, telemetryTracker: telemetryTrackerFactory(), clients: {}, + uniqueKeysTracker: { start: jest.fn(), stop: jest.fn() } }; const invalidAttributes = [ @@ -95,6 +96,7 @@ describe('sdkClientMethodCSFactory', () => { expect(params.syncManager.stop).toBeCalledTimes(1); expect(params.syncManager.flush).toBeCalledTimes(1); expect(params.signalListener.stop).toBeCalledTimes(1); + expect(params.uniqueKeysTracker.stop).toBeCalledTimes(1); }); }); diff --git a/src/sdkClient/sdkClient.ts b/src/sdkClient/sdkClient.ts index cda5ba2e..cc44c9f7 100644 --- a/src/sdkClient/sdkClient.ts +++ b/src/sdkClient/sdkClient.ts @@ -61,7 +61,7 @@ export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: bo releaseApiKey(settings.core.authorizationKey); telemetryTracker.sessionLength(); signalListener && signalListener.stop(); - uniqueKeysTracker && uniqueKeysTracker.stop(); + uniqueKeysTracker.stop(); } // Stop background jobs diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 680b8f7f..3a43c955 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -59,15 +59,15 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA const integrationsManager = integrationsManagerFactory && integrationsManagerFactory({ settings, storage, telemetryTracker }); const observer = impressionsObserverFactory(); - const uniqueKeysTracker = impressionsMode === NONE ? uniqueKeysTrackerFactory(log, storage.uniqueKeys!, filterAdapterFactory && filterAdapterFactory()) : undefined; + const uniqueKeysTracker = uniqueKeysTrackerFactory(log, storage.uniqueKeys, filterAdapterFactory && filterAdapterFactory()); let strategy; switch (impressionsMode) { case OPTIMIZED: - strategy = strategyOptimizedFactory(observer, storage.impressionCounts!); + strategy = strategyOptimizedFactory(observer, storage.impressionCounts); break; case NONE: - strategy = strategyNoneFactory(storage.impressionCounts!, uniqueKeysTracker!); + strategy = strategyNoneFactory(storage.impressionCounts, uniqueKeysTracker); break; default: strategy = strategyDebugFactory(observer); @@ -99,7 +99,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA // We will just log and allow for the SDK to end up throwing an SDK_TIMEOUT event for devs to handle. validateAndTrackApiKey(log, settings.core.authorizationKey); readiness.init(); - uniqueKeysTracker && uniqueKeysTracker.start(); + uniqueKeysTracker.start(); syncManager && syncManager.start(); signalListener && signalListener.start(); diff --git a/src/sdkFactory/types.ts b/src/sdkFactory/types.ts index 19fa5f10..27bdc49a 100644 --- a/src/sdkFactory/types.ts +++ b/src/sdkFactory/types.ts @@ -46,7 +46,7 @@ export interface ISdkFactoryContext { eventTracker: IEventTracker, telemetryTracker: ITelemetryTracker, storage: IStorageSync | IStorageAsync, - uniqueKeysTracker?: IUniqueKeysTracker, + uniqueKeysTracker: IUniqueKeysTracker, signalListener?: ISignalListener splitApi?: ISplitApi syncManager?: ISyncManager, From 77f8dee44e21f7f68c1c08741b3a06fc5e26ebc2 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 9 Dec 2024 16:37:23 -0300 Subject: [PATCH 04/10] Refactor strategies and impressionsTracker --- src/sdkFactory/index.ts | 21 +++++------- .../__tests__/impressionsTracker.spec.ts | 19 +++++++---- src/trackers/impressionsTracker.ts | 31 +++++++++--------- .../strategy/__tests__/strategyDebug.spec.ts | 32 +++++++------------ .../strategy/__tests__/strategyNone.spec.ts | 26 ++++----------- .../__tests__/strategyOptimized.spec.ts | 32 ++++++++----------- src/trackers/strategy/strategyDebug.ts | 15 +++------ src/trackers/strategy/strategyNone.ts | 28 +++++++--------- src/trackers/strategy/strategyOptimized.ts | 30 ++++++----------- src/trackers/types.ts | 8 +---- 10 files changed, 92 insertions(+), 150 deletions(-) diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 3a43c955..b342a6e0 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -13,7 +13,7 @@ import { strategyDebugFactory } from '../trackers/strategy/strategyDebug'; import { strategyOptimizedFactory } from '../trackers/strategy/strategyOptimized'; import { strategyNoneFactory } from '../trackers/strategy/strategyNone'; import { uniqueKeysTrackerFactory } from '../trackers/uniqueKeysTracker'; -import { NONE, OPTIMIZED } from '../utils/constants'; +import { DEBUG, OPTIMIZED } from '../utils/constants'; /** * Modular SDK factory @@ -61,19 +61,14 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA const observer = impressionsObserverFactory(); const uniqueKeysTracker = uniqueKeysTrackerFactory(log, storage.uniqueKeys, filterAdapterFactory && filterAdapterFactory()); - let strategy; - switch (impressionsMode) { - case OPTIMIZED: - strategy = strategyOptimizedFactory(observer, storage.impressionCounts); - break; - case NONE: - strategy = strategyNoneFactory(storage.impressionCounts, uniqueKeysTracker); - break; - default: - strategy = strategyDebugFactory(observer); - } + const noneStrategy = strategyNoneFactory(storage.impressionCounts, uniqueKeysTracker); + const strategy = impressionsMode === OPTIMIZED ? + strategyOptimizedFactory(observer, storage.impressionCounts) : + impressionsMode === DEBUG ? + strategyDebugFactory(observer) : + noneStrategy; - const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, strategy, whenInit, integrationsManager, storage.telemetry); + const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, noneStrategy, strategy, whenInit, integrationsManager, storage.telemetry); const eventTracker = eventTrackerFactory(settings, storage.events, whenInit, integrationsManager, storage.telemetry); // splitApi is used by SyncManager and Browser signal listener diff --git a/src/trackers/__tests__/impressionsTracker.spec.ts b/src/trackers/__tests__/impressionsTracker.spec.ts index 08ec9f71..7b5bc600 100644 --- a/src/trackers/__tests__/impressionsTracker.spec.ts +++ b/src/trackers/__tests__/impressionsTracker.spec.ts @@ -36,6 +36,11 @@ const fakeSettingsWithListener = { }; const fakeWhenInit = (cb: () => void) => cb(); +// @TODO validate logic with `impression.track` false +const fakeNoneStrategy = { + process: jest.fn(() => false) +}; + /* Tests */ describe('Impressions Tracker', () => { @@ -51,12 +56,12 @@ describe('Impressions Tracker', () => { test('Tracker API', () => { expect(typeof impressionsTrackerFactory).toBe('function'); // The module should return a function which acts as a factory. - const instance = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, strategy, fakeWhenInit); + const instance = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy,strategy, fakeWhenInit); expect(typeof instance.track).toBe('function'); // The instance should implement the track method which will actually track queued impressions. }); test('Should be able to track impressions (in DEBUG mode without Previous Time).', () => { - const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, strategy, fakeWhenInit); + const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit); const imp1 = { feature: '10', @@ -76,7 +81,7 @@ describe('Impressions Tracker', () => { }); test('Tracked impressions should be sent to impression listener and integration manager when we invoke .track()', (done) => { - const tracker = impressionsTrackerFactory(fakeSettingsWithListener, fakeImpressionsCache, strategy, fakeWhenInit, fakeIntegrationsManager); + const tracker = impressionsTrackerFactory(fakeSettingsWithListener, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit, fakeIntegrationsManager); const fakeImpression = { feature: 'impression' @@ -150,8 +155,8 @@ describe('Impressions Tracker', () => { impression3.time = 1234567891; const trackers = [ - impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, strategyDebugFactory(impressionObserverSSFactory()), fakeWhenInit, undefined), - impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, strategyDebugFactory(impressionObserverCSFactory()), fakeWhenInit, undefined) + impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverSSFactory()), fakeWhenInit, undefined), + impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverCSFactory()), fakeWhenInit, undefined) ]; expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked. @@ -178,7 +183,7 @@ describe('Impressions Tracker', () => { impression3.time = Date.now(); const impressionCountsCache = new ImpressionCountsCacheInMemory(); - const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, strategyOptimizedFactory(impressionObserverCSFactory(), impressionCountsCache), fakeWhenInit, undefined, fakeTelemetryCache as any); + const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyOptimizedFactory(impressionObserverCSFactory(), impressionCountsCache), fakeWhenInit, undefined, fakeTelemetryCache as any); expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker @@ -201,7 +206,7 @@ describe('Impressions Tracker', () => { test('Should track or not impressions depending on user consent status', () => { const settings = { ...fullSettings }; - const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, strategy, fakeWhenInit); + const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit); tracker.track([impression]); expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined diff --git a/src/trackers/impressionsTracker.ts b/src/trackers/impressionsTracker.ts index 485d0694..b1607eb5 100644 --- a/src/trackers/impressionsTracker.ts +++ b/src/trackers/impressionsTracker.ts @@ -9,16 +9,11 @@ import SplitIO from '../../types/splitio'; /** * Impressions tracker stores impressions in cache and pass them to the listener and integrations manager if provided. - * - * @param impressionsCache - cache to save impressions - * @param metadata - runtime metadata (ip, hostname and version) - * @param impressionListener - optional impression listener - * @param integrationsManager - optional integrations manager - * @param strategy - strategy for impressions tracking. */ export function impressionsTrackerFactory( settings: ISettings, impressionsCache: IImpressionsCacheBase, + noneStrategy: IStrategy, strategy: IStrategy, whenInit: (cb: () => void) => void, integrationsManager?: IImpressionsHandler, @@ -31,37 +26,41 @@ export function impressionsTrackerFactory( track(impressions: SplitIO.ImpressionDTO[], attributes?: SplitIO.Attributes) { if (settings.userConsent === CONSENT_DECLINED) return; - const impressionsCount = impressions.length; - const { impressionsToStore, impressionsToListener, deduped } = strategy.process(impressions); + const impressionsToStore = impressions.filter((impression) => { + return impression.track === false ? + noneStrategy.process(impression) : + strategy.process(impression); + }); - const impressionsToListenerCount = impressionsToListener.length; + const impressionsLength = impressions.length; + const impressionsToStoreLength = impressionsToStore.length; - if (impressionsToStore.length > 0) { + if (impressionsToStoreLength) { const res = impressionsCache.track(impressionsToStore); // If we're on an async storage, handle error and log it. if (thenable(res)) { res.then(() => { - log.info(IMPRESSIONS_TRACKER_SUCCESS, [impressionsCount]); + log.info(IMPRESSIONS_TRACKER_SUCCESS, [impressionsLength]); }).catch(err => { - log.error(ERROR_IMPRESSIONS_TRACKER, [impressionsCount, err]); + log.error(ERROR_IMPRESSIONS_TRACKER, [impressionsLength, err]); }); } else { // Record when impressionsCache is sync only (standalone mode) // @TODO we are not dropping impressions on full queue yet, so DROPPED stats are not recorded if (telemetryCache) { - (telemetryCache as ITelemetryCacheSync).recordImpressionStats(QUEUED, impressionsToStore.length); - (telemetryCache as ITelemetryCacheSync).recordImpressionStats(DEDUPED, deduped); + (telemetryCache as ITelemetryCacheSync).recordImpressionStats(QUEUED, impressionsToStoreLength); + (telemetryCache as ITelemetryCacheSync).recordImpressionStats(DEDUPED, impressionsLength - impressionsToStoreLength); } } } // @TODO next block might be handled by the integration manager. In that case, the metadata object doesn't need to be passed in the constructor if (impressionListener || integrationsManager) { - for (let i = 0; i < impressionsToListenerCount; i++) { + for (let i = 0; i < impressionsLength; i++) { const impressionData: SplitIO.ImpressionData = { // copy of impression, to avoid unexpected behaviour if modified by integrations or impressionListener - impression: objectAssign({}, impressionsToListener[i]), + impression: objectAssign({}, impressions[i]), attributes, ip, hostname, diff --git a/src/trackers/strategy/__tests__/strategyDebug.spec.ts b/src/trackers/strategy/__tests__/strategyDebug.spec.ts index 8ef5e71c..d2fd7585 100644 --- a/src/trackers/strategy/__tests__/strategyDebug.spec.ts +++ b/src/trackers/strategy/__tests__/strategyDebug.spec.ts @@ -3,29 +3,21 @@ import { impressionObserverCSFactory } from '../../impressionObserver/impression import { strategyDebugFactory } from '../strategyDebug'; import { impression1, impression2 } from './testUtils'; -test('strategyDebug', () => { +test.each([ + impressionObserverSSFactory(), + impressionObserverCSFactory()] +)('strategyDebug', (impressionObserver) => { - let augmentedImp1 = { ...impression1, pt: undefined }; - let augmentedImp12 = { ...impression1, pt: impression1.time }; - let augmentedImp2 = { ...impression2, pt: undefined }; + const strategyDebug = strategyDebugFactory(impressionObserver); - let impressions = [impression1, impression2, {...impression1}]; - let augmentedImpressions = [augmentedImp1, augmentedImp2, augmentedImp12]; + const impressions = [{ ...impression1 }, { ...impression2 }, { ...impression1 }]; - const strategyDebugSS = strategyDebugFactory(impressionObserverSSFactory()); + expect(strategyDebug.process(impressions[0])).toBe(true); + expect(impressions[0]).toEqual({ ...impression1, pt: undefined }); - let { impressionsToStore, impressionsToListener, deduped } = strategyDebugSS.process(impressions); - - expect(impressionsToStore).toStrictEqual(augmentedImpressions); - expect(impressionsToListener).toStrictEqual(augmentedImpressions); - expect(deduped).toStrictEqual(0); - - const strategyDebugCS = strategyDebugFactory(impressionObserverCSFactory()); - - ({ impressionsToStore, impressionsToListener, deduped } = strategyDebugCS.process(impressions)); - - expect(impressionsToStore).toStrictEqual(augmentedImpressions); - expect(impressionsToListener).toStrictEqual(augmentedImpressions); - expect(deduped).toStrictEqual(0); + expect(strategyDebug.process(impressions[1])).toBe(true); + expect(impressions[1]).toEqual({ ...impression2, pt: undefined }); + expect(strategyDebug.process(impressions[2])).toBe(true); + expect(impressions[2]).toEqual({ ...impression1, pt: impression1.time }); }); diff --git a/src/trackers/strategy/__tests__/strategyNone.spec.ts b/src/trackers/strategy/__tests__/strategyNone.spec.ts index 60e871c2..96ca7edf 100644 --- a/src/trackers/strategy/__tests__/strategyNone.spec.ts +++ b/src/trackers/strategy/__tests__/strategyNone.spec.ts @@ -28,11 +28,9 @@ test('strategyNone - Client side', () => { const strategyNone = strategyNoneFactory(impressionCountsCache, uniqueKeysTracker); - const { - impressionsToStore: impressionsToStoreCs, - impressionsToListener: impressionsToListenerCs, - deduped: dedupedCs - } = strategyNone.process(impressions); + impressions.forEach(impression => { + expect(strategyNone.process(impression)).toBe(false); + }); expect(uniqueKeysCacheCS.pop()).toStrictEqual({ keys: [ @@ -52,11 +50,6 @@ test('strategyNone - Client side', () => { }); expect(uniqueKeysCacheCS.pop()).toStrictEqual({ keys: [] }); - - expect(impressionsToStoreCs).toStrictEqual([]); - expect(impressionsToListenerCs).toStrictEqual(impressions); - expect(dedupedCs).toStrictEqual(0); - }); test('strategyNone - Server side', () => { @@ -67,11 +60,9 @@ test('strategyNone - Server side', () => { const strategyNone = strategyNoneFactory(impressionCountsCache, uniqueKeysTracker); - const { - impressionsToStore: impressionsToStoreSs, - impressionsToListener: impressionsToListenerSs, - deduped: dedupedSs - } = strategyNone.process(impressions); + impressions.forEach(impression => { + expect(strategyNone.process(impression)).toBe(false); + }); expect(uniqueKeysCache.pop()).toStrictEqual({ keys: [ @@ -90,9 +81,4 @@ test('strategyNone - Server side', () => { ] }); expect(uniqueKeysCache.pop()).toStrictEqual({ keys: [] }); - - expect(impressionsToStoreSs).toStrictEqual([]); - expect(impressionsToListenerSs).toStrictEqual(impressions); - expect(dedupedSs).toStrictEqual(0); - }); diff --git a/src/trackers/strategy/__tests__/strategyOptimized.spec.ts b/src/trackers/strategy/__tests__/strategyOptimized.spec.ts index f13219b1..fc65b3d0 100644 --- a/src/trackers/strategy/__tests__/strategyOptimized.spec.ts +++ b/src/trackers/strategy/__tests__/strategyOptimized.spec.ts @@ -4,31 +4,25 @@ import { strategyOptimizedFactory } from '../strategyOptimized'; import { ImpressionCountsCacheInMemory } from '../../../storages/inMemory/ImpressionCountsCacheInMemory'; import { impression1, impression2 } from './testUtils'; -test('strategyOptimized', () => { - - let augmentedImp1 = { ...impression1, pt: undefined }; - let augmentedImp12 = { ...impression1, pt: impression1.time }; - let augmentedImp13 = { ...impression1, pt: impression1.time }; - let augmentedImp2 = { ...impression2, pt: undefined }; +test.each([ + impressionObserverSSFactory(), + impressionObserverCSFactory() +])('strategyOptimized', () => { const impressionCountsCache = new ImpressionCountsCacheInMemory(); - const impressions = [impression1, impression2, {...impression1}, {...impression1}]; - const augmentedImpressions = [augmentedImp1, augmentedImp2, augmentedImp12, augmentedImp13]; - const strategyOptimizedSS = strategyOptimizedFactory(impressionObserverSSFactory(), impressionCountsCache); - let { impressionsToStore, impressionsToListener, deduped } = strategyOptimizedSS.process(impressions); - - expect(impressionsToStore).toStrictEqual([augmentedImp1, augmentedImp2]); - expect(impressionsToListener).toStrictEqual(augmentedImpressions); - expect(deduped).toStrictEqual(2); + const impressions = [{...impression1}, {...impression2}, {...impression1}, {...impression1}]; - const strategyOptimizedCS = strategyOptimizedFactory(impressionObserverCSFactory(), impressionCountsCache); + expect(strategyOptimizedSS.process(impressions[0])).toBe(true); + expect(impressions[0]).toEqual({ ...impression1, pt: undefined }); - ({ impressionsToStore, impressionsToListener, deduped } = strategyOptimizedCS.process(impressions)); + expect(strategyOptimizedSS.process(impressions[1])).toBe(true); + expect(impressions[1]).toEqual({ ...impression2, pt: undefined }); - expect(impressionsToStore).toStrictEqual([augmentedImp1, augmentedImp2]); - expect(impressionsToListener).toStrictEqual(augmentedImpressions); - expect(deduped).toStrictEqual(2); + expect(strategyOptimizedSS.process(impressions[2])).toBe(false); + expect(impressions[2]).toEqual({ ...impression1, pt: impression1.time }); + expect(strategyOptimizedSS.process(impressions[3])).toBe(false); + expect(impressions[3]).toEqual({ ...impression1, pt: impression1.time }); }); diff --git a/src/trackers/strategy/strategyDebug.ts b/src/trackers/strategy/strategyDebug.ts index 21ca3a5d..65bc06b3 100644 --- a/src/trackers/strategy/strategyDebug.ts +++ b/src/trackers/strategy/strategyDebug.ts @@ -6,23 +6,16 @@ import { IStrategy } from '../types'; * Debug strategy for impressions tracker. Wraps impressions to store and adds previousTime if it corresponds * * @param impressionsObserver - impression observer. Previous time (pt property) is included in impression instances - * @returns IStrategyResult + * @returns Debug strategy */ export function strategyDebugFactory( impressionsObserver: IImpressionObserver ): IStrategy { return { - process(impressions: SplitIO.ImpressionDTO[]) { - impressions.forEach((impression) => { - // Adds previous time if it is enabled - impression.pt = impressionsObserver.testAndSet(impression); - }); - return { - impressionsToStore: impressions, - impressionsToListener: impressions, - deduped: 0 - }; + process(impression: SplitIO.ImpressionDTO) { + impression.pt = impressionsObserver.testAndSet(impression); + return true; } }; } diff --git a/src/trackers/strategy/strategyNone.ts b/src/trackers/strategy/strategyNone.ts index 452ae594..5319a64d 100644 --- a/src/trackers/strategy/strategyNone.ts +++ b/src/trackers/strategy/strategyNone.ts @@ -5,30 +5,24 @@ import { IStrategy, IUniqueKeysTracker } from '../types'; /** * None strategy for impressions tracker. * - * @param impressionsCounter - cache to save impressions count. impressions will be deduped (OPTIMIZED mode) + * @param impressionCounts - cache to save impressions count. impressions will be deduped (OPTIMIZED mode) * @param uniqueKeysTracker - unique keys tracker in charge of tracking the unique keys per split. - * @returns IStrategyResult + * @returns None strategy */ export function strategyNoneFactory( - impressionsCounter: IImpressionCountsCacheBase, + impressionCounts: IImpressionCountsCacheBase, uniqueKeysTracker: IUniqueKeysTracker ): IStrategy { return { - process(impressions: SplitIO.ImpressionDTO[]) { - impressions.forEach((impression) => { - const now = Date.now(); - // Increments impression counter per featureName - impressionsCounter.track(impression.feature, now, 1); - // Keep track by unique key - uniqueKeysTracker.track(impression.keyName, impression.feature); - }); - - return { - impressionsToStore: [], - impressionsToListener: impressions, - deduped: 0 - }; + process(impression: SplitIO.ImpressionDTO) { + const now = Date.now(); + // Increments impression counter per featureName + impressionCounts.track(impression.feature, now, 1); + // Keep track by unique key + uniqueKeysTracker.track(impression.keyName, impression.feature); + // Do not store impressions + return false; } }; } diff --git a/src/trackers/strategy/strategyOptimized.ts b/src/trackers/strategy/strategyOptimized.ts index 9fe61af1..9a9cf883 100644 --- a/src/trackers/strategy/strategyOptimized.ts +++ b/src/trackers/strategy/strategyOptimized.ts @@ -8,35 +8,25 @@ import { IStrategy } from '../types'; * Optimized strategy for impressions tracker. Wraps impressions to store and adds previousTime if it corresponds * * @param impressionsObserver - impression observer. previous time (pt property) is included in impression instances - * @param impressionsCounter - cache to save impressions count. impressions will be deduped (OPTIMIZED mode) - * @returns IStrategyResult + * @param impressionCounts - cache to save impressions count. impressions will be deduped (OPTIMIZED mode) + * @returns Optimized strategy */ export function strategyOptimizedFactory( impressionsObserver: IImpressionObserver, - impressionsCounter: IImpressionCountsCacheBase, + impressionCounts: IImpressionCountsCacheBase, ): IStrategy { return { - process(impressions: SplitIO.ImpressionDTO[]) { - const impressionsToStore: SplitIO.ImpressionDTO[] = []; - impressions.forEach((impression) => { - impression.pt = impressionsObserver.testAndSet(impression); + process(impression: SplitIO.ImpressionDTO) { + impression.pt = impressionsObserver.testAndSet(impression); - const now = Date.now(); + const now = Date.now(); - // Increments impression counter per featureName - if (impression.pt) impressionsCounter.track(impression.feature, now, 1); + // Increments impression counter per featureName + if (impression.pt) impressionCounts.track(impression.feature, now, 1); - // Checks if the impression should be added in queue to be sent - if (!impression.pt || impression.pt < truncateTimeFrame(now)) { - impressionsToStore.push(impression); - } - }); - return { - impressionsToStore: impressionsToStore, - impressionsToListener: impressions, - deduped: impressions.length - impressionsToStore.length - }; + // Checks if the impression should be added in queue to be sent + return (!impression.pt || impression.pt < truncateTimeFrame(now)) ? true : false; } }; } diff --git a/src/trackers/types.ts b/src/trackers/types.ts index db6d5bcb..9f426123 100644 --- a/src/trackers/types.ts +++ b/src/trackers/types.ts @@ -70,12 +70,6 @@ export interface IUniqueKeysTracker { track(key: string, featureName: string): void; } -export interface IStrategyResult { - impressionsToStore: SplitIO.ImpressionDTO[], - impressionsToListener: SplitIO.ImpressionDTO[], - deduped: number -} - export interface IStrategy { - process(impressions: SplitIO.ImpressionDTO[]): IStrategyResult + process(impression: SplitIO.ImpressionDTO): boolean } From 5eb34bfed30c9b107c997beb5f17062dc30be963 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 10 Dec 2024 16:08:49 -0300 Subject: [PATCH 05/10] Add trackImpressions property to SplitView --- src/sdkManager/__tests__/mocks/output.json | 3 ++- src/sdkManager/index.ts | 3 ++- types/splitio.d.ts | 4 ++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/sdkManager/__tests__/mocks/output.json b/src/sdkManager/__tests__/mocks/output.json index a2af9e81..ef956a69 100644 --- a/src/sdkManager/__tests__/mocks/output.json +++ b/src/sdkManager/__tests__/mocks/output.json @@ -8,5 +8,6 @@ "on": "\"color\": \"green\"" }, "sets": ["set_a"], - "defaultTreatment": "off" + "defaultTreatment": "off", + "trackImpressions": true } diff --git a/src/sdkManager/index.ts b/src/sdkManager/index.ts index 1246f16f..ef43335e 100644 --- a/src/sdkManager/index.ts +++ b/src/sdkManager/index.ts @@ -31,7 +31,8 @@ function objectToView(splitObject: ISplit | null): SplitIO.SplitView | null { treatments: collectTreatments(splitObject), configs: splitObject.configurations || {}, sets: splitObject.sets || [], - defaultTreatment: splitObject.defaultTreatment + defaultTreatment: splitObject.defaultTreatment, + trackImpressions: splitObject.trackImpressions !== false }; } diff --git a/types/splitio.d.ts b/types/splitio.d.ts index d520e64c..977d318f 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -863,6 +863,10 @@ declare namespace SplitIO { * The default treatment of the feature flag. */ defaultTreatment: string; + /** + * Whether the feature flag has impressions tracking enabled or not. + */ + trackImpressions: boolean; }; /** * A promise that resolves to a feature flag view or null if the feature flag is not found. From f9dc9474a7b571137c5d9aa6216b8aa82ad5b02f Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 Dec 2024 14:02:25 -0300 Subject: [PATCH 06/10] Remove 'track' property from ImpressionDTO, to not modify the ImpressionListener types --- src/sdkClient/client.ts | 19 +++++++++---------- .../__tests__/impressionsTracker.spec.ts | 16 ++++++++-------- src/trackers/impressionsTracker.ts | 3 ++- src/trackers/types.ts | 2 +- types/splitio.d.ts | 1 - 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 0bd00f97..5216d2d2 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -34,11 +34,11 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENT_WITH_CONFIG : TREATMENT); const wrapUp = (evaluationResult: IEvaluationResult) => { - const queue: SplitIO.ImpressionDTO[] = []; + const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; const treatment = processEvaluation(evaluationResult, featureFlagName, key, attributes, withConfig, methodName, queue); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0].label); + stopTelemetryTracker(queue[0] && queue[0][0].label); return treatment; }; @@ -59,14 +59,14 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS); const wrapUp = (evaluationResults: Record) => { - const queue: SplitIO.ImpressionDTO[] = []; + const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; const treatments: Record = {}; Object.keys(evaluationResults).forEach(featureFlagName => { treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, methodName, queue); }); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0].label); + stopTelemetryTracker(queue[0] && queue[0][0].label); return treatments; }; @@ -87,7 +87,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(method); const wrapUp = (evaluationResults: Record) => { - const queue: SplitIO.ImpressionDTO[] = []; + const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; const treatments: Record = {}; const evaluations = evaluationResults; Object.keys(evaluations).forEach(featureFlagName => { @@ -95,7 +95,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl }); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0].label); + stopTelemetryTracker(queue[0] && queue[0][0].label); return treatments; }; @@ -128,7 +128,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl attributes: SplitIO.Attributes | undefined, withConfig: boolean, invokingMethodName: string, - queue: SplitIO.ImpressionDTO[] + queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] ): SplitIO.Treatment | SplitIO.TreatmentWithConfig { const matchingKey = getMatching(key); const bucketingKey = getBucketing(key); @@ -138,7 +138,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl if (validateSplitExistence(log, readinessManager, featureFlagName, label, invokingMethodName)) { log.info(IMPRESSION_QUEUEING); - queue.push({ + queue.push([{ feature: featureFlagName, keyName: matchingKey, treatment, @@ -146,8 +146,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl bucketingKey, label, changeNumber: changeNumber as number, - track - }); + }, track]); } if (withConfig) { diff --git a/src/trackers/__tests__/impressionsTracker.spec.ts b/src/trackers/__tests__/impressionsTracker.spec.ts index 08ec9f71..8a0cfd51 100644 --- a/src/trackers/__tests__/impressionsTracker.spec.ts +++ b/src/trackers/__tests__/impressionsTracker.spec.ts @@ -70,7 +70,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker - tracker.track([imp1, imp2, imp3]); + tracker.track([[imp1], [imp2], [imp3]]); expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([imp1, imp2, imp3]); // Should call the storage track method once we invoke .track() method, passing queued params in a sequence. }); @@ -93,7 +93,7 @@ describe('Impressions Tracker', () => { expect(fakeIntegrationsManager.handleImpression).not.toBeCalled(); // The integrations manager handleImpression method should not be invoked if we haven't tracked impressions. // We signal that we actually want to track the queued impressions. - tracker.track([fakeImpression, fakeImpression2], fakeAttributes); + tracker.track([[fakeImpression], [fakeImpression2]], fakeAttributes); expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([fakeImpression, fakeImpression2]); // Even with a listener, impression should be sent to the cache expect(fakeListener.logImpression).not.toBeCalled(); // The listener should not be executed synchronously. @@ -157,7 +157,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked. trackers.forEach(tracker => { - tracker.track([impression, impression2, impression3]); + tracker.track([[impression], [impression2], [impression3]]); const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1]; @@ -182,7 +182,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker - tracker.track([impression, impression2, impression3]); + tracker.track([[impression], [impression2], [impression3]]); const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1]; @@ -203,19 +203,19 @@ describe('Impressions Tracker', () => { const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, strategy, fakeWhenInit); - tracker.track([impression]); + tracker.track([[impression]]); expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined settings.userConsent = 'UNKNOWN'; - tracker.track([impression]); + tracker.track([[impression]]); expect(fakeImpressionsCache.track).toBeCalledTimes(2); // impression should be tracked if userConsent is unknown settings.userConsent = 'GRANTED'; - tracker.track([impression]); + tracker.track([[impression]]); expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should be tracked if userConsent is granted settings.userConsent = 'DECLINED'; - tracker.track([impression]); + tracker.track([[impression]]); expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should not be tracked if userConsent is declined }); diff --git a/src/trackers/impressionsTracker.ts b/src/trackers/impressionsTracker.ts index 485d0694..54a5ed0b 100644 --- a/src/trackers/impressionsTracker.ts +++ b/src/trackers/impressionsTracker.ts @@ -28,9 +28,10 @@ export function impressionsTrackerFactory( const { log, impressionListener, runtime: { ip, hostname }, version } = settings; return { - track(impressions: SplitIO.ImpressionDTO[], attributes?: SplitIO.Attributes) { + track(imps: [impression: SplitIO.ImpressionDTO, track?: boolean][], attributes?: SplitIO.Attributes) { if (settings.userConsent === CONSENT_DECLINED) return; + const impressions = imps.map((item) => item[0]); const impressionsCount = impressions.length; const { impressionsToStore, impressionsToListener, deduped } = strategy.process(impressions); diff --git a/src/trackers/types.ts b/src/trackers/types.ts index db6d5bcb..71c5534a 100644 --- a/src/trackers/types.ts +++ b/src/trackers/types.ts @@ -18,7 +18,7 @@ export interface IImpressionsHandler { } export interface IImpressionsTracker { - track(impressions: SplitIO.ImpressionDTO[], attributes?: SplitIO.Attributes): void + track(impressions: [impression: SplitIO.ImpressionDTO, track?: boolean][], attributes?: SplitIO.Attributes): void } /** Telemetry tracker */ diff --git a/types/splitio.d.ts b/types/splitio.d.ts index d520e64c..f2faa2df 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -811,7 +811,6 @@ declare namespace SplitIO { label: string; changeNumber: number; pt?: number; - track?: boolean; } /** * Object with information about an impression. It contains the generated impression DTO as well as From c16be4af8cf7660ef937dd85866dddebf96d700a Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 Dec 2024 14:33:33 -0300 Subject: [PATCH 07/10] rc --- CHANGES.txt | 4 ++++ package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 02a7bd61..e92cb38d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,7 @@ +2.1.0 (January XX, 2025) + - Added `trackImpressions` property to SDK Manager's `SplitView` type. + - Updated implementation of the impressions tracker and strategies to support feature flags with impressions tracking disabled. + 2.0.2 (December 3, 2024) - Updated the factory `init` and `destroy` methods to support re-initialization after destruction. This update ensures compatibility of the React SDK with React Strict Mode, where the factory's `init` and `destroy` effects are executed an extra time to validate proper resource cleanup. - Bugfixing - Sanitize the `SplitSDKMachineName` header value to avoid exceptions on HTTP/S requests when it contains non ISO-8859-1 characters (Related to issue https://github.com/splitio/javascript-client/issues/847). diff --git a/package-lock.json b/package-lock.json index 971d1486..616a6bd8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.0.2", + "version": "2.0.3-rc.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "2.0.2", + "version": "2.0.3-rc.0", "license": "Apache-2.0", "dependencies": { "@types/ioredis": "^4.28.0", diff --git a/package.json b/package.json index 936c23bd..518beb20 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.0.2", + "version": "2.0.3-rc.0", "description": "Split JavaScript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", From ac5659a8647c4c26610a9dbcfe4d3916d6e0fccb Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 Dec 2024 15:16:06 -0300 Subject: [PATCH 08/10] Polishing --- src/storages/inMemory/InMemoryStorage.ts | 4 ++-- src/storages/inMemory/InMemoryStorageCS.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/storages/inMemory/InMemoryStorage.ts b/src/storages/inMemory/InMemoryStorage.ts index b8833370..7ec099d1 100644 --- a/src/storages/inMemory/InMemoryStorage.ts +++ b/src/storages/inMemory/InMemoryStorage.ts @@ -37,8 +37,8 @@ export function InMemoryStorageFactory(params: IStorageFactoryParams): IStorageS const noopTrack = () => true; storage.impressions.track = noopTrack; storage.events.track = noopTrack; - if (storage.impressionCounts) storage.impressionCounts.track = noopTrack; - if (storage.uniqueKeys) storage.uniqueKeys.track = noopTrack; + storage.impressionCounts.track = noopTrack; + storage.uniqueKeys.track = noopTrack; } return storage; diff --git a/src/storages/inMemory/InMemoryStorageCS.ts b/src/storages/inMemory/InMemoryStorageCS.ts index 09d62ac8..bfaec159 100644 --- a/src/storages/inMemory/InMemoryStorageCS.ts +++ b/src/storages/inMemory/InMemoryStorageCS.ts @@ -55,8 +55,8 @@ export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorag const noopTrack = () => true; storage.impressions.track = noopTrack; storage.events.track = noopTrack; - if (storage.impressionCounts) storage.impressionCounts.track = noopTrack; - if (storage.uniqueKeys) storage.uniqueKeys.track = noopTrack; + storage.impressionCounts.track = noopTrack; + storage.uniqueKeys.track = noopTrack; } return storage; From 522debdf3f97d2e669631eb0c7d128f32a8ad930 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 Dec 2024 15:26:31 -0300 Subject: [PATCH 09/10] New type ImpressionDecorated --- src/sdkClient/client.ts | 36 ++++++++++--------- .../__tests__/impressionsTracker.spec.ts | 16 ++++----- src/trackers/impressionsTracker.ts | 6 ++-- src/trackers/types.ts | 13 ++++++- 4 files changed, 43 insertions(+), 28 deletions(-) diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 5216d2d2..7a280122 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -11,6 +11,7 @@ import { IMPRESSION, IMPRESSION_QUEUEING } from '../logger/constants'; import { ISdkFactoryContext } from '../sdkFactory/types'; import { isConsumerMode } from '../utils/settingsValidation/mode'; import { Method } from '../sync/submitters/types'; +import { ImpressionDecorated } from '../trackers/types'; const treatmentNotReady = { treatment: CONTROL, label: SDK_NOT_READY }; @@ -34,11 +35,11 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENT_WITH_CONFIG : TREATMENT); const wrapUp = (evaluationResult: IEvaluationResult) => { - const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; + const queue: ImpressionDecorated[] = []; const treatment = processEvaluation(evaluationResult, featureFlagName, key, attributes, withConfig, methodName, queue); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0][0].label); + stopTelemetryTracker(queue[0] && queue[0].imp.label); return treatment; }; @@ -59,14 +60,14 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS); const wrapUp = (evaluationResults: Record) => { - const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; + const queue: ImpressionDecorated[] = []; const treatments: Record = {}; Object.keys(evaluationResults).forEach(featureFlagName => { treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, methodName, queue); }); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0][0].label); + stopTelemetryTracker(queue[0] && queue[0].imp.label); return treatments; }; @@ -87,7 +88,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(method); const wrapUp = (evaluationResults: Record) => { - const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; + const queue: ImpressionDecorated[] = []; const treatments: Record = {}; const evaluations = evaluationResults; Object.keys(evaluations).forEach(featureFlagName => { @@ -95,7 +96,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl }); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0][0].label); + stopTelemetryTracker(queue[0] && queue[0].imp.label); return treatments; }; @@ -128,7 +129,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl attributes: SplitIO.Attributes | undefined, withConfig: boolean, invokingMethodName: string, - queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] + queue: ImpressionDecorated[] ): SplitIO.Treatment | SplitIO.TreatmentWithConfig { const matchingKey = getMatching(key); const bucketingKey = getBucketing(key); @@ -138,15 +139,18 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl if (validateSplitExistence(log, readinessManager, featureFlagName, label, invokingMethodName)) { log.info(IMPRESSION_QUEUEING); - queue.push([{ - feature: featureFlagName, - keyName: matchingKey, - treatment, - time: Date.now(), - bucketingKey, - label, - changeNumber: changeNumber as number, - }, track]); + queue.push({ + imp: { + feature: featureFlagName, + keyName: matchingKey, + treatment, + time: Date.now(), + bucketingKey, + label, + changeNumber: changeNumber as number, + }, + track + }); } if (withConfig) { diff --git a/src/trackers/__tests__/impressionsTracker.spec.ts b/src/trackers/__tests__/impressionsTracker.spec.ts index 8a0cfd51..4d54bf68 100644 --- a/src/trackers/__tests__/impressionsTracker.spec.ts +++ b/src/trackers/__tests__/impressionsTracker.spec.ts @@ -70,7 +70,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker - tracker.track([[imp1], [imp2], [imp3]]); + tracker.track([{ imp: imp1 }, { imp: imp2 }, { imp: imp3 }]); expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([imp1, imp2, imp3]); // Should call the storage track method once we invoke .track() method, passing queued params in a sequence. }); @@ -93,7 +93,7 @@ describe('Impressions Tracker', () => { expect(fakeIntegrationsManager.handleImpression).not.toBeCalled(); // The integrations manager handleImpression method should not be invoked if we haven't tracked impressions. // We signal that we actually want to track the queued impressions. - tracker.track([[fakeImpression], [fakeImpression2]], fakeAttributes); + tracker.track([{ imp: fakeImpression }, { imp: fakeImpression2 }], fakeAttributes); expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([fakeImpression, fakeImpression2]); // Even with a listener, impression should be sent to the cache expect(fakeListener.logImpression).not.toBeCalled(); // The listener should not be executed synchronously. @@ -157,7 +157,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked. trackers.forEach(tracker => { - tracker.track([[impression], [impression2], [impression3]]); + tracker.track([{ imp: impression }, { imp: impression2 }, { imp: impression3 }]); const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1]; @@ -182,7 +182,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker - tracker.track([[impression], [impression2], [impression3]]); + tracker.track([{ imp: impression }, { imp: impression2 }, { imp: impression3 }]); const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1]; @@ -203,19 +203,19 @@ describe('Impressions Tracker', () => { const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, strategy, fakeWhenInit); - tracker.track([[impression]]); + tracker.track([{ imp: impression }]); expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined settings.userConsent = 'UNKNOWN'; - tracker.track([[impression]]); + tracker.track([{ imp: impression }]); expect(fakeImpressionsCache.track).toBeCalledTimes(2); // impression should be tracked if userConsent is unknown settings.userConsent = 'GRANTED'; - tracker.track([[impression]]); + tracker.track([{ imp: impression }]); expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should be tracked if userConsent is granted settings.userConsent = 'DECLINED'; - tracker.track([[impression]]); + tracker.track([{ imp: impression }]); expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should not be tracked if userConsent is declined }); diff --git a/src/trackers/impressionsTracker.ts b/src/trackers/impressionsTracker.ts index 54a5ed0b..1d3980cf 100644 --- a/src/trackers/impressionsTracker.ts +++ b/src/trackers/impressionsTracker.ts @@ -1,7 +1,7 @@ import { objectAssign } from '../utils/lang/objectAssign'; import { thenable } from '../utils/promise/thenable'; import { IImpressionsCacheBase, ITelemetryCacheSync, ITelemetryCacheAsync } from '../storages/types'; -import { IImpressionsHandler, IImpressionsTracker, IStrategy } from './types'; +import { IImpressionsHandler, IImpressionsTracker, ImpressionDecorated, IStrategy } from './types'; import { ISettings } from '../types'; import { IMPRESSIONS_TRACKER_SUCCESS, ERROR_IMPRESSIONS_TRACKER, ERROR_IMPRESSIONS_LISTENER } from '../logger/constants'; import { CONSENT_DECLINED, DEDUPED, QUEUED } from '../utils/constants'; @@ -28,10 +28,10 @@ export function impressionsTrackerFactory( const { log, impressionListener, runtime: { ip, hostname }, version } = settings; return { - track(imps: [impression: SplitIO.ImpressionDTO, track?: boolean][], attributes?: SplitIO.Attributes) { + track(imps: ImpressionDecorated[], attributes?: SplitIO.Attributes) { if (settings.userConsent === CONSENT_DECLINED) return; - const impressions = imps.map((item) => item[0]); + const impressions = imps.map((item) => item.imp); const impressionsCount = impressions.length; const { impressionsToStore, impressionsToListener, deduped } = strategy.process(impressions); diff --git a/src/trackers/types.ts b/src/trackers/types.ts index 71c5534a..a53c1486 100644 --- a/src/trackers/types.ts +++ b/src/trackers/types.ts @@ -17,8 +17,19 @@ export interface IImpressionsHandler { handleImpression(impressionData: SplitIO.ImpressionData): any } +export type ImpressionDecorated = { + /** + * Impression DTO + */ + imp: SplitIO.ImpressionDTO, + /** + * Whether the impression should be tracked or not + */ + track?: boolean +}; + export interface IImpressionsTracker { - track(impressions: [impression: SplitIO.ImpressionDTO, track?: boolean][], attributes?: SplitIO.Attributes): void + track(impressions: ImpressionDecorated[], attributes?: SplitIO.Attributes): void } /** Telemetry tracker */ From 7e1330663ff4d3eeebd7d6367f06d61eb051be73 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 12 Dec 2024 17:27:11 -0300 Subject: [PATCH 10/10] Update test --- .../__tests__/impressionsTracker.spec.ts | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/trackers/__tests__/impressionsTracker.spec.ts b/src/trackers/__tests__/impressionsTracker.spec.ts index c10bf309..60c7e5e9 100644 --- a/src/trackers/__tests__/impressionsTracker.spec.ts +++ b/src/trackers/__tests__/impressionsTracker.spec.ts @@ -36,7 +36,6 @@ const fakeSettingsWithListener = { }; const fakeWhenInit = (cb: () => void) => cb(); -// @TODO validate logic with `impression.track` false const fakeNoneStrategy = { process: jest.fn(() => false) }; @@ -53,13 +52,6 @@ describe('Impressions Tracker', () => { const strategy = strategyDebugFactory(impressionObserverCSFactory()); - test('Tracker API', () => { - expect(typeof impressionsTrackerFactory).toBe('function'); // The module should return a function which acts as a factory. - - const instance = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy,strategy, fakeWhenInit); - expect(typeof instance.track).toBe('function'); // The instance should implement the track method which will actually track queued impressions. - }); - test('Should be able to track impressions (in DEBUG mode without Previous Time).', () => { const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit); @@ -75,9 +67,9 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker - tracker.track([{ imp: imp1 }, { imp: imp2 }, { imp: imp3 }]); + tracker.track([{ imp: imp1 }, { imp: imp2, track: true }, { imp: imp3, track: false }]); - expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([imp1, imp2, imp3]); // Should call the storage track method once we invoke .track() method, passing queued params in a sequence. + expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([imp1, imp2]); // Should call the storage track method once we invoke .track() method, passing impressions with `track` enabled }); test('Tracked impressions should be sent to impression listener and integration manager when we invoke .track()', (done) => { @@ -98,9 +90,9 @@ describe('Impressions Tracker', () => { expect(fakeIntegrationsManager.handleImpression).not.toBeCalled(); // The integrations manager handleImpression method should not be invoked if we haven't tracked impressions. // We signal that we actually want to track the queued impressions. - tracker.track([{ imp: fakeImpression }, { imp: fakeImpression2 }], fakeAttributes); + tracker.track([{ imp: fakeImpression }, { imp: fakeImpression2, track: false }], fakeAttributes); - expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([fakeImpression, fakeImpression2]); // Even with a listener, impression should be sent to the cache + expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([fakeImpression]); // Even with a listener, impressions (with `track` enabled) should be sent to the cache expect(fakeListener.logImpression).not.toBeCalled(); // The listener should not be executed synchronously. expect(fakeIntegrationsManager.handleImpression).not.toBeCalled(); // The integrations manager handleImpression method should not be executed synchronously. @@ -162,7 +154,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked. trackers.forEach(tracker => { - tracker.track([{ imp: impression }, { imp: impression2 }, { imp: impression3 }]); + tracker.track([{ imp: impression, track: true }, { imp: impression2 }, { imp: impression3 }]); const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1];