Skip to content

Commit

Permalink
Merge branch 'impressions_toggle_track_property' into impressions_tog…
Browse files Browse the repository at this point in the history
…gle_storage_refactors
  • Loading branch information
EmilianoSanchez committed Dec 11, 2024
2 parents ac5659a + 522debd commit a771764
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 28 deletions.
36 changes: 20 additions & 16 deletions src/sdkClient/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand All @@ -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;
};

Expand All @@ -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<string, IEvaluationResult>) => {
const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = [];
const queue: ImpressionDecorated[] = [];
const treatments: Record<string, SplitIO.Treatment | SplitIO.TreatmentWithConfig> = {};
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;
};

Expand All @@ -87,15 +88,15 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
const stopTelemetryTracker = telemetryTracker.trackEval(method);

const wrapUp = (evaluationResults: Record<string, IEvaluationResult>) => {
const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = [];
const queue: ImpressionDecorated[] = [];
const treatments: Record<string, SplitIO.Treatment | SplitIO.TreatmentWithConfig> = {};
const evaluations = evaluationResults;
Object.keys(evaluations).forEach(featureFlagName => {
treatments[featureFlagName] = processEvaluation(evaluations[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;
};

Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
16 changes: 8 additions & 8 deletions src/trackers/__tests__/impressionsTracker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
});
Expand All @@ -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.
Expand Down Expand Up @@ -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];

Expand All @@ -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];

Expand All @@ -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
});

Expand Down
6 changes: 3 additions & 3 deletions src/trackers/impressionsTracker.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);

Expand Down
13 changes: 12 additions & 1 deletion src/trackers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit a771764

Please sign in to comment.