From e6d46fd599d069d076ec25aca03bdee0462c8f5c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 6 Dec 2024 17:19:08 -0300 Subject: [PATCH 1/3] 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 f9dc9474a7b571137c5d9aa6216b8aa82ad5b02f Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 Dec 2024 14:02:25 -0300 Subject: [PATCH 2/3] 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 522debdf3f97d2e669631eb0c7d128f32a8ad930 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 Dec 2024 15:26:31 -0300 Subject: [PATCH 3/3] 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 */