From 3bbf16b1a94fc6c18f58453034623585d5b34171 Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Thu, 21 Sep 2023 14:42:55 -0300 Subject: [PATCH 1/6] [SDKS-7557/7554] getTreatment/s byFlagset withConfig/s --- src/dtos/types.ts | 2 +- .../__tests__/evaluate-features.spec.ts | 57 ++++++++- src/evaluator/index.ts | 25 ++++ .../clientAttributesDecoration.spec.ts | 104 ++++++++++++++++ src/sdkClient/__tests__/testUtils.ts | 2 +- src/sdkClient/client.ts | 42 ++++++- src/sdkClient/clientAttributesDecoration.ts | 24 ++++ src/sdkClient/clientCS.ts | 4 + src/sdkClient/clientInputValidation.ts | 54 ++++++++- src/types.ts | 114 +++++++++++++++++- 10 files changed, 421 insertions(+), 7 deletions(-) diff --git a/src/dtos/types.ts b/src/dtos/types.ts index 4fdf562c..03363928 100644 --- a/src/dtos/types.ts +++ b/src/dtos/types.ts @@ -164,7 +164,7 @@ export interface ISplit { configurations?: { [treatmentName: string]: string }, - sets: string[] + sets?: string[] } // Split definition used in offline mode diff --git a/src/evaluator/__tests__/evaluate-features.spec.ts b/src/evaluator/__tests__/evaluate-features.spec.ts index c400c983..648c09cb 100644 --- a/src/evaluator/__tests__/evaluate-features.spec.ts +++ b/src/evaluator/__tests__/evaluate-features.spec.ts @@ -1,7 +1,9 @@ // @ts-nocheck -import { evaluateFeatures } from '../index'; +import { evaluateFeatures, evaluateFeaturesByFlagSets } from '../index'; import * as LabelsConstants from '../../utils/labels'; import { loggerMock } from '../../logger/__tests__/sdkLogger.mock'; +import { _Set } from '../../utils/lang/sets'; +import { returnSetsUnion } from '../../utils/lang/sets'; const splitsMock = { regular: { 'changeNumber': 1487277320548, 'trafficAllocationSeed': 1667452163, 'trafficAllocation': 100, 'trafficTypeName': 'user', 'name': 'always-on', 'seed': 1684183541, 'configurations': {}, 'status': 'ACTIVE', 'killed': false, 'defaultTreatment': 'off', 'conditions': [{ 'conditionType': 'ROLLOUT', 'matcherGroup': { 'combiner': 'AND', 'matchers': [{ 'keySelector': { 'trafficType': 'user', 'attribute': '' }, 'matcherType': 'ALL_KEYS', 'negate': false, 'userDefinedSegmentMatcherData': { 'segmentName': '' }, 'unaryNumericMatcherData': { 'dataType': '', 'value': 0 }, 'whitelistMatcherData': { 'whitelist': null }, 'betweenMatcherData': { 'dataType': '', 'start': 0, 'end': 0 } }] }, 'partitions': [{ 'treatment': 'on', 'size': 100 }, { 'treatment': 'off', 'size': 0 }], 'label': 'in segment all' }] }, @@ -14,6 +16,11 @@ const splitsMock = { trafficAlocation1WithConfig: { 'changeNumber': 1487277320548, 'trafficAllocationSeed': -1667452163, 'trafficAllocation': 1, 'trafficTypeName': 'user', 'name': 'always-on6', 'seed': 1684183541, 'configurations': { 'off': "{color:'black'}" }, 'status': 'ACTIVE', 'killed': false, 'defaultTreatment': 'off', 'conditions': [{ 'conditionType': 'ROLLOUT', 'matcherGroup': { 'combiner': 'AND', 'matchers': [{ 'keySelector': { 'trafficType': 'user', 'attribute': '' }, 'matcherType': 'ALL_KEYS', 'negate': false, 'userDefinedSegmentMatcherData': { 'segmentName': '' }, 'unaryNumericMatcherData': { 'dataType': '', 'value': 0 }, 'whitelistMatcherData': { 'whitelist': null }, 'betweenMatcherData': { 'dataType': '', 'start': 0, 'end': 0 } }] }, 'partitions': [{ 'treatment': 'on', 'size': 100 }, { 'treatment': 'off', 'size': 0 }], 'label': 'in segment all' }] } }; +const flagSetsMock = { + reg_and_config: new _Set(['regular', 'config']), + arch_and_killed: new _Set(['killed', 'archived']), +}; + const mockStorage = { splits: { getSplit(name) { @@ -29,6 +36,16 @@ const mockStorage = { }); return splits; + }, + getNamesByFlagSets(flagSets) { + let toReturn = new _Set([]); + flagSets.forEach(flagset => { + const featureFlagNames = flagSetsMock[flagset]; + if (featureFlagNames) { + toReturn = returnSetsUnion(toReturn, featureFlagNames); + } + }); + return toReturn; } } }; @@ -105,3 +122,41 @@ test('EVALUATOR - Multiple evaluations at once / should return right labels, tre // If the split is retrieved but is not in split (out of Traffic Allocation), we should get the right evaluation result, label and config. }); + +test('EVALUATOR - Multiple evaluations at once by flagsets / should return right labels, treatments and configs if storage returns without errors.', async function () { + const expectedOutput = { + config: { + treatment: 'on', label: 'in segment all', + config: '{color:\'black\'}', changeNumber: 1487277320548 + }, + not_existent_split: { + treatment: 'control', label: LabelsConstants.SPLIT_NOT_FOUND, config: null + }, + }; + + const multipleEvaluationAtOnce = await evaluateFeaturesByFlagSets( + loggerMock, + 'fake-key', + ['reg_and_config', 'arch_and_killed'], + null, + mockStorage, + ); + + // assert evaluationWithConfig + expect(multipleEvaluationAtOnce['config']).toEqual(expectedOutput['config']); // If the split is retrieved successfully we should get the right evaluation result, label and config. + // @todo assert flagset not found - for input validations + + // assert regular + expect(multipleEvaluationAtOnce['regular']).toEqual({ ...expectedOutput['config'], config: null }); // If the split is retrieved successfully we should get the right evaluation result, label and config. If Split has no config it should have config equal null. + // assert killed + expect(multipleEvaluationAtOnce['killed']).toEqual({ ...expectedOutput['config'], treatment: 'off', config: null, label: LabelsConstants.SPLIT_KILLED }); + // 'If the split is retrieved but is killed, we should get the right evaluation result, label and config. + + // assert archived + expect(multipleEvaluationAtOnce['archived']).toEqual({ ...expectedOutput['config'], treatment: 'control', label: LabelsConstants.SPLIT_ARCHIVED, config: null }); + // If the split is retrieved but is archived, we should get the right evaluation result, label and config. + + // assert not_existent_split not in evaluation if it is not related to defined flagsets + expect(multipleEvaluationAtOnce['not_existent_split']).toEqual(undefined); + +}); diff --git a/src/evaluator/index.ts b/src/evaluator/index.ts index dad4c9e4..700785fe 100644 --- a/src/evaluator/index.ts +++ b/src/evaluator/index.ts @@ -7,6 +7,7 @@ import { IStorageAsync, IStorageSync } from '../storages/types'; import { IEvaluationResult } from './types'; import { SplitIO } from '../types'; import { ILogger } from '../logger/types'; +import { setToArray } from '../utils/lang/sets'; const treatmentException = { treatment: CONTROL, @@ -87,6 +88,30 @@ export function evaluateFeatures( getEvaluations(log, splitNames, parsedSplits, key, attributes, storage); } +export function evaluateFeaturesByFlagSets( + log: ILogger, + key: SplitIO.SplitKey, + flagsets: string[], + attributes: SplitIO.Attributes | undefined, + storage: IStorageSync | IStorageAsync, +): MaybeThenable> { + let storedSplitNames; + + // get ff by flagsets + try { + storedSplitNames = storage.splits.getNamesByFlagSets(flagsets); + } catch (e) { + // Exception on sync `getSplits` storage. Not possible ATM with InMemory and InLocal storages. + // @todo - review exception + return treatmentsException(flagsets); + } + + return thenable(storedSplitNames) ? + storedSplitNames.then((splitNames) => evaluateFeatures(log, key, setToArray(splitNames), attributes, storage)) : + evaluateFeatures(log, key, setToArray(storedSplitNames), attributes, storage); + +} + function getEvaluation( log: ILogger, splitJSON: ISplit | null, diff --git a/src/sdkClient/__tests__/clientAttributesDecoration.spec.ts b/src/sdkClient/__tests__/clientAttributesDecoration.spec.ts index 45fe70ff..e007dc54 100644 --- a/src/sdkClient/__tests__/clientAttributesDecoration.spec.ts +++ b/src/sdkClient/__tests__/clientAttributesDecoration.spec.ts @@ -14,6 +14,22 @@ const clientMock = { }, getTreatmentsWithConfig(maybeKey: any, maybeFeatureFlagNames: string[], maybeAttributes?: any) { return maybeAttributes; + }, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + getTreatmentsByFlagSets(maybeKey: any, maybeFeatureFlagNames: string[], maybeAttributes?: any, maybeFlagSetNames?: string[] | undefined) { + return maybeAttributes; + }, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + getTreatmentsWithConfigByFlagSets(maybeKey: any, maybeFeatureFlagNames: string[], maybeAttributes?: any, maybeFlagSetNames?: string[] | undefined) { + return maybeAttributes; + }, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + getTreatmentsByFlagSet(maybeKey: any, maybeFeatureFlagNames: string[], maybeAttributes?: any, maybeFlagSetName?: string | undefined) { + return maybeAttributes; + }, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + getTreatmentsWithConfigByFlagSet(maybeKey: any, maybeFeatureFlagNames: string[], maybeAttributes?: any, maybeFlagSetName?: string | undefined) { + return maybeAttributes; } }; // @ts-expect-error @@ -225,4 +241,92 @@ describe('ATTRIBUTES DECORATION / evaluation', () => { }); + test('Evaluation attributes logic and precedence / getTreatmentsByFlagSets', () => { + + // If the same attribute is “cached” and provided on the function, the value received on the function call takes precedence. + expect(client.getTreatmentsByFlagSets('key', ['set'])).toEqual(undefined); // Nothing changes if no attributes were provided using the new api + expect(client.getTreatmentsByFlagSets('key', ['set'], { func_attr_bool: true, func_attr_str: 'true' })).toEqual({ func_attr_bool: true, func_attr_str: 'true' }); // Nothing changes if no attributes were provided using the new api + expect(client.getAttributes()).toEqual({}); // Attributes in memory storage must be empty + client.setAttribute('func_attr_bool', false); + expect(client.getAttributes()).toEqual({ 'func_attr_bool': false }); // In memory attribute storage must have the unique stored attribute + expect(client.getTreatmentsByFlagSets('key', ['set'], { func_attr_bool: true, func_attr_str: 'true' })).toEqual({ func_attr_bool: true, func_attr_str: 'true' }); // Function attributes has precedence against api ones + // @ts-ignore + expect(client.getTreatmentsByFlagSets('key', ['set'], null)).toEqual({ func_attr_bool: false }); // API attributes should be kept in memory and use for evaluations + expect(client.getTreatmentsByFlagSets('key', ['set'], { func_attr_str: 'true' })).toEqual({ func_attr_bool: false, func_attr_str: 'true' }); // API attributes should be kept in memory and use for evaluations + client.setAttributes({ func_attr_str: 'false' }); + expect(client.getAttributes()).toEqual({ 'func_attr_bool': false, 'func_attr_str': 'false' }); // In memory attribute storage must have two stored attributes + expect(client.getTreatmentsByFlagSets('key', ['set'], { func_attr_bool: true, func_attr_str: 'true', func_attr_number: 1 })).toEqual({ func_attr_bool: true, func_attr_str: 'true', func_attr_number: 1 }); // Function attributes has precedence against api ones + // @ts-ignore + expect(client.getTreatmentsByFlagSets('key', ['set'], null)).toEqual({ func_attr_bool: false, func_attr_str: 'false' }); // If the getTreatment function is called without attributes, stored attributes will be used to evaluate. + expect(client.getTreatmentsByFlagSets('key', ['set'])).toEqual({ func_attr_bool: false, func_attr_str: 'false' }); // If the getTreatment function is called without attributes, stored attributes will be used to evaluate. + client.clearAttributes(); + + }); + + test('Evaluation attributes logic and precedence / getTreatmentsWithConfigByFlagSets', () => { + + // If the same attribute is “cached” and provided on the function, the value received on the function call takes precedence. + expect(client.getTreatmentsWithConfigByFlagSets('key', ['set'])).toEqual(undefined); // Nothing changes if no attributes were provided using the new api + expect(client.getTreatmentsWithConfigByFlagSets('key', ['set'], { func_attr_bool: true, func_attr_str: 'true' })).toEqual({ func_attr_bool: true, func_attr_str: 'true' }); // Nothing changes if no attributes were provided using the new api + expect(client.getAttributes()).toEqual({}); // Attributes in memory storage must be empty + client.setAttribute('func_attr_bool', false); + expect(client.getAttributes()).toEqual({ 'func_attr_bool': false }); // In memory attribute storage must have the unique stored attribute + expect(client.getTreatmentsWithConfigByFlagSets('key', ['set'], { func_attr_bool: true, func_attr_str: 'true' })).toEqual({ func_attr_bool: true, func_attr_str: 'true' }); // Function attributes has precedence against api ones + // @ts-ignore + expect(client.getTreatmentsWithConfigByFlagSets('key', ['set'], null)).toEqual({ func_attr_bool: false }); // API attributes should be kept in memory and use for evaluations + expect(client.getTreatmentsWithConfigByFlagSets('key', ['set'], { func_attr_str: 'true' })).toEqual({ func_attr_bool: false, func_attr_str: 'true' }); // API attributes should be kept in memory and use for evaluations + client.setAttributes({ func_attr_str: 'false' }); + expect(client.getAttributes()).toEqual({ 'func_attr_bool': false, 'func_attr_str': 'false' }); // In memory attribute storage must have two stored attributes + expect(client.getTreatmentsWithConfigByFlagSets('key', ['set'], { func_attr_bool: true, func_attr_str: 'true', func_attr_number: 1 })).toEqual({ func_attr_bool: true, func_attr_str: 'true', func_attr_number: 1 }); // Function attributes has precedence against api ones + // @ts-ignore + expect(client.getTreatmentsWithConfigByFlagSets('key', ['set'], null)).toEqual({ func_attr_bool: false, func_attr_str: 'false' }); // If the getTreatment function is called without attributes, stored attributes will be used to evaluate. + expect(client.getTreatmentsWithConfigByFlagSets('key', ['set'])).toEqual({ func_attr_bool: false, func_attr_str: 'false' }); // If the getTreatment function is called without attributes, stored attributes will be used to evaluate. + client.clearAttributes(); + + }); + + test('Evaluation attributes logic and precedence / getTreatmentsByFlagSet', () => { + + // If the same attribute is “cached” and provided on the function, the value received on the function call takes precedence. + expect(client.getTreatmentsByFlagSet('key', 'set')).toEqual(undefined); // Nothing changes if no attributes were provided using the new api + expect(client.getTreatmentsByFlagSet('key', 'set', { func_attr_bool: true, func_attr_str: 'true' })).toEqual({ func_attr_bool: true, func_attr_str: 'true' }); // Nothing changes if no attributes were provided using the new api + expect(client.getAttributes()).toEqual({}); // Attributes in memory storage must be empty + client.setAttribute('func_attr_bool', false); + expect(client.getAttributes()).toEqual({ 'func_attr_bool': false }); // In memory attribute storage must have the unique stored attribute + expect(client.getTreatmentsByFlagSet('key', 'set', { func_attr_bool: true, func_attr_str: 'true' })).toEqual({ func_attr_bool: true, func_attr_str: 'true' }); // Function attributes has precedence against api ones + // @ts-ignore + expect(client.getTreatmentsByFlagSet('key', 'set', null)).toEqual({ func_attr_bool: false }); // API attributes should be kept in memory and use for evaluations + expect(client.getTreatmentsByFlagSet('key', 'set', { func_attr_str: 'true' })).toEqual({ func_attr_bool: false, func_attr_str: 'true' }); // API attributes should be kept in memory and use for evaluations + client.setAttributes({ func_attr_str: 'false' }); + expect(client.getAttributes()).toEqual({ 'func_attr_bool': false, 'func_attr_str': 'false' }); // In memory attribute storage must have two stored attributes + expect(client.getTreatmentsByFlagSet('key', 'set', { func_attr_bool: true, func_attr_str: 'true', func_attr_number: 1 })).toEqual({ func_attr_bool: true, func_attr_str: 'true', func_attr_number: 1 }); // Function attributes has precedence against api ones + // @ts-ignore + expect(client.getTreatmentsByFlagSet('key', 'set', null)).toEqual({ func_attr_bool: false, func_attr_str: 'false' }); // If the getTreatment function is called without attributes, stored attributes will be used to evaluate. + expect(client.getTreatmentsByFlagSet('key', 'set')).toEqual({ func_attr_bool: false, func_attr_str: 'false' }); // If the getTreatment function is called without attributes, stored attributes will be used to evaluate. + client.clearAttributes(); + + }); + + test('Evaluation attributes logic and precedence / getTreatmentsWithConfigByFlagSet', () => { + + // If the same attribute is “cached” and provided on the function, the value received on the function call takes precedence. + expect(client.getTreatmentsWithConfigByFlagSet('key', 'set')).toEqual(undefined); // Nothing changes if no attributes were provided using the new api + expect(client.getTreatmentsWithConfigByFlagSet('key', 'set', { func_attr_bool: true, func_attr_str: 'true' })).toEqual({ func_attr_bool: true, func_attr_str: 'true' }); // Nothing changes if no attributes were provided using the new api + expect(client.getAttributes()).toEqual({}); // Attributes in memory storage must be empty + client.setAttribute('func_attr_bool', false); + expect(client.getAttributes()).toEqual({ 'func_attr_bool': false }); // In memory attribute storage must have the unique stored attribute + expect(client.getTreatmentsWithConfigByFlagSet('key', 'set', { func_attr_bool: true, func_attr_str: 'true' })).toEqual({ func_attr_bool: true, func_attr_str: 'true' }); // Function attributes has precedence against api ones + // @ts-ignore + expect(client.getTreatmentsWithConfigByFlagSet('key', 'set', null)).toEqual({ func_attr_bool: false }); // API attributes should be kept in memory and use for evaluations + expect(client.getTreatmentsWithConfigByFlagSet('key', 'set', { func_attr_str: 'true' })).toEqual({ func_attr_bool: false, func_attr_str: 'true' }); // API attributes should be kept in memory and use for evaluations + client.setAttributes({ func_attr_str: 'false' }); + expect(client.getAttributes()).toEqual({ 'func_attr_bool': false, 'func_attr_str': 'false' }); // In memory attribute storage must have two stored attributes + expect(client.getTreatmentsWithConfigByFlagSet('key', 'set', { func_attr_bool: true, func_attr_str: 'true', func_attr_number: 1 })).toEqual({ func_attr_bool: true, func_attr_str: 'true', func_attr_number: 1 }); // Function attributes has precedence against api ones + // @ts-ignore + expect(client.getTreatmentsWithConfigByFlagSet('key', 'set', null)).toEqual({ func_attr_bool: false, func_attr_str: 'false' }); // If the getTreatment function is called without attributes, stored attributes will be used to evaluate. + expect(client.getTreatmentsWithConfigByFlagSet('key', 'set')).toEqual({ func_attr_bool: false, func_attr_str: 'false' }); // If the getTreatment function is called without attributes, stored attributes will be used to evaluate. + client.clearAttributes(); + + }); + }); diff --git a/src/sdkClient/__tests__/testUtils.ts b/src/sdkClient/__tests__/testUtils.ts index 30ee5966..901897e3 100644 --- a/src/sdkClient/__tests__/testUtils.ts +++ b/src/sdkClient/__tests__/testUtils.ts @@ -1,4 +1,4 @@ -const clientApiMethods = ['getTreatment', 'getTreatments', 'getTreatmentWithConfig', 'getTreatmentsWithConfig', 'track', 'destroy']; +const clientApiMethods = ['getTreatment', 'getTreatments', 'getTreatmentWithConfig', 'getTreatmentsWithConfig', 'getTreatmentsByFlagSets', 'getTreatmentsWithConfigByFlagSets', 'getTreatmentsByFlagSet', 'getTreatmentsWithConfigByFlagSet', 'track', 'destroy']; export function assertClientApi(client: any, sdkStatus?: object) { diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 11036519..7fed2261 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -1,4 +1,4 @@ -import { evaluateFeature, evaluateFeatures } from '../evaluator'; +import { evaluateFeature, evaluateFeatures, evaluateFeaturesByFlagSets } from '../evaluator'; import { thenable } from '../utils/promise/thenable'; import { getMatching, getBucketing } from '../utils/key'; import { validateSplitExistance } from '../utils/inputValidation/splitExistance'; @@ -81,6 +81,42 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl return getTreatments(key, featureFlagNames, attributes, true); } + function getTreatmentsByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false) { + const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS); + + const wrapUp = (evaluationResults: Record) => { + const queue: ImpressionDTO[] = []; + const treatments: Record = {}; + Object.keys(evaluationResults).forEach(featureFlagName => { + treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, `getTreatmentsByFlagSets${withConfig ? 'WithConfig' : ''}`, queue); + }); + impressionsTracker.track(queue, attributes); + + stopTelemetryTracker(queue[0] && queue[0].label); + return treatments; + }; + + const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ? + evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage) : + isStorageSync(settings) ? // If the SDK is not ready, treatment may be incorrect due to having splits but not segments data, or storage is not connected + treatmentsNotReady([]) : + Promise.resolve(treatmentsNotReady([])); // Promisify if async + + return thenable(evaluations) ? evaluations.then((res) => wrapUp(res)) : wrapUp(evaluations); + } + + function getTreatmentsWithConfigByFlagSets(key: SplitIO.SplitKey, featureFlagNames: string[], attributes: SplitIO.Attributes | undefined) { + return getTreatmentsByFlagSets(key, featureFlagNames, attributes, true); + } + + function getTreatmentsByFlagSet(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined) { + return getTreatmentsByFlagSets(key, [featureFlagName], attributes); + } + + function getTreatmentsWithConfigByFlagSet(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined) { + return getTreatmentsByFlagSets(key, [featureFlagName], attributes, true); + } + // Internal function function processEvaluation( evaluation: IEvaluationResult, @@ -155,6 +191,10 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl getTreatmentWithConfig, getTreatments, getTreatmentsWithConfig, + getTreatmentsByFlagSets, + getTreatmentsWithConfigByFlagSets, + getTreatmentsByFlagSet, + getTreatmentsWithConfigByFlagSet, track, isClientSide: false } as SplitIO.IClient | SplitIO.IAsyncClient; diff --git a/src/sdkClient/clientAttributesDecoration.ts b/src/sdkClient/clientAttributesDecoration.ts index 6db04e3e..57413ad9 100644 --- a/src/sdkClient/clientAttributesDecoration.ts +++ b/src/sdkClient/clientAttributesDecoration.ts @@ -16,6 +16,10 @@ export function clientAttributesDecoration} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsByFlagSets(key: SplitKey, flagSets: string[], attributes?: Attributes): Treatments, + /** + * Returns a TreatmentsWithConfig value, which is an object map with the TreatmentWithConfig (an object with both treatment and config string) for the feature flags related to the given flagSets. + * @function getTreatmentsWithConfigByFlagSets + * @param {string} key - The string key representing the consumer. + * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsWithConfigByFlagSets(key: SplitKey, flagSets: string[], attributes?: Attributes): TreatmentsWithConfig, /** * Tracks an event to be fed to the results product on Split user interface. * @function track @@ -1124,6 +1160,46 @@ export namespace SplitIO { * @returns {AsyncTreatmentsWithConfig} TreatmentsWithConfig promise that resolves to the map of TreatmentsWithConfig objects. */ getTreatmentsWithConfig(key: SplitKey, featureFlagNames: string[], attributes?: Attributes): AsyncTreatmentsWithConfig, + /** + * Returns a Treatments value, which will be (or eventually be) an object map with the treatments for the features related to the given flagset. + * For usage on NodeJS as we don't have only one key. + * @function getTreatmentsByFlagSet + * @param {string} key - The string key representing the consumer. + * @param {string} flagSet - The flagSet name we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsByFlagSet(key: SplitKey, flagSet: string, attributes?: Attributes): AsyncTreatments, + /** + * Returns a TreatmentWithConfig value, which will be (or eventually be) an object with both treatment and config string for features related to the given flagset. + * For usage on NodeJS as we don't have only one key. + * @function getTreatmentsWithConfigByFlagSet + * @param {string} key - The string key representing the consumer. + * @param {string} flagSet - The flagSet name we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsWithConfigByFlagSet(key: SplitKey, flagSet: string, attributes?: Attributes): AsyncTreatmentsWithConfig, + /** + * Returns a Treatments value, which will be (or eventually be) an object map with the treatments for the feature flags related to the given flagSets. + * For usage on NodeJS as we don't have only one key. + * @function getTreatmentsByFlagSets + * @param {string} key - The string key representing the consumer. + * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsByFlagSets(key: SplitKey, flagSets: string[], attributes?: Attributes): AsyncTreatments, + /** + * Returns a TreatmentWithConfig value, which will be (or eventually be) an object with both treatment and config string for the feature flags related to the given flagSets. + * For usage on NodeJS as we don't have only one key. + * @function getTreatmentsWithConfigByFlagSets + * @param {string} key - The string key representing the consumer. + * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsWithConfigByFlagSets(key: SplitKey, flagSets: string[], attributes?: Attributes): AsyncTreatmentsWithConfig, /** * Tracks an event to be fed to the results product on Split user interface, and returns a promise to signal when the event was successfully queued (or not). * @function track @@ -1174,6 +1250,42 @@ export namespace SplitIO { * @returns {TreatmentsWithConfig} The map with all the TreatmentWithConfig objects */ getTreatmentsWithConfig(featureFlagNames: string[], attributes?: Attributes): TreatmentsWithConfig, + /** + * Returns a Treatments value, which is an object map with the treatments for the feature flags related to the given flagSet. + * @function getTreatmentsByFlagSet + * @param {string} key - The string key representing the consumer. + * @param {string} flagSet - The flagSet name we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsByFlagSet(key: SplitKey, flagSet: string, attributes?: Attributes): Treatments, + /** + * Returns a TreatmentsWithConfig value, which is an object map with the TreatmentWithConfig (an object with both treatment and config string) for the feature flags related to the given flagSet. + * @function getTreatmentsWithConfigByFlagSet + * @param {string} key - The string key representing the consumer. + * @param {string} flagSet - The flagSet name we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsWithConfigByFlagSet(key: SplitKey, flagSet: string, attributes?: Attributes): TreatmentsWithConfig, + /** + * Returns a Returns a Treatments value, which is an object with both treatment and config string for to the feature flags related to the given flagSets. + * @function getTreatmentsByFlagSets + * @param {string} key - The string key representing the consumer. + * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsByFlagSets(key: SplitKey, flagSets: string[], attributes?: Attributes): Treatments, + /** + * Returns a TreatmentsWithConfig value, which is an object map with the TreatmentWithConfig (an object with both treatment and config string) for the feature flags related to the given flagSets. + * @function getTreatmentsWithConfigByFlagSets + * @param {string} key - The string key representing the consumer. + * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. + * @returns {Treatments} The map with all the TreatmentWithConfig objects + */ + getTreatmentsWithConfigByFlagSets(key: SplitKey, flagSets: string[], attributes?: Attributes): TreatmentsWithConfig, /** * Tracks an event to be fed to the results product on Split user interface. * @function track From ea83dbfbe15bae267ebd4bf6aaed732a308b2246 Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Fri, 22 Sep 2023 16:45:39 -0300 Subject: [PATCH 2/6] Fix response when flagset invalid --- src/evaluator/index.ts | 10 +++++----- src/sdkClient/client.ts | 4 +--- src/sdkClient/clientInputValidation.ts | 10 +++++----- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/evaluator/index.ts b/src/evaluator/index.ts index 700785fe..8c9c1810 100644 --- a/src/evaluator/index.ts +++ b/src/evaluator/index.ts @@ -95,20 +95,20 @@ export function evaluateFeaturesByFlagSets( attributes: SplitIO.Attributes | undefined, storage: IStorageSync | IStorageAsync, ): MaybeThenable> { - let storedSplitNames; + let storedFlagNames; // get ff by flagsets try { - storedSplitNames = storage.splits.getNamesByFlagSets(flagsets); + storedFlagNames = storage.splits.getNamesByFlagSets(flagsets); } catch (e) { // Exception on sync `getSplits` storage. Not possible ATM with InMemory and InLocal storages. // @todo - review exception return treatmentsException(flagsets); } - return thenable(storedSplitNames) ? - storedSplitNames.then((splitNames) => evaluateFeatures(log, key, setToArray(splitNames), attributes, storage)) : - evaluateFeatures(log, key, setToArray(storedSplitNames), attributes, storage); + return thenable(storedFlagNames) ? + storedFlagNames.then((splitNames) => evaluateFeatures(log, key, setToArray(splitNames), attributes, storage)) : + evaluateFeatures(log, key, setToArray(storedFlagNames), attributes, storage); } diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 7fed2261..c6afc365 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -98,9 +98,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ? evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage) : - isStorageSync(settings) ? // If the SDK is not ready, treatment may be incorrect due to having splits but not segments data, or storage is not connected - treatmentsNotReady([]) : - Promise.resolve(treatmentsNotReady([])); // Promisify if async + isStorageSync(settings) ? {} : Promise.resolve({}); // Promisify if async return thenable(evaluations) ? evaluations.then((res) => wrapUp(res)) : wrapUp(evaluations); } diff --git a/src/sdkClient/clientInputValidation.ts b/src/sdkClient/clientInputValidation.ts index 87f5cada..0161a43d 100644 --- a/src/sdkClient/clientInputValidation.ts +++ b/src/sdkClient/clientInputValidation.ts @@ -44,7 +44,7 @@ export function clientInputValidationDecorator Date: Fri, 22 Sep 2023 23:58:31 -0300 Subject: [PATCH 3/6] Return when calling --- .../__tests__/evaluate-features.spec.ts | 58 +++++++++++++++---- src/evaluator/index.ts | 34 +++++++++-- src/evaluator/types.ts | 5 ++ src/sdkClient/client.ts | 13 +++-- 4 files changed, 88 insertions(+), 22 deletions(-) diff --git a/src/evaluator/__tests__/evaluate-features.spec.ts b/src/evaluator/__tests__/evaluate-features.spec.ts index 648c09cb..0d6116bc 100644 --- a/src/evaluator/__tests__/evaluate-features.spec.ts +++ b/src/evaluator/__tests__/evaluate-features.spec.ts @@ -39,6 +39,14 @@ const mockStorage = { }, getNamesByFlagSets(flagSets) { let toReturn = new _Set([]); + // Forced thenable delayed response for testing purposes + if (flagSets[0] === 'delay') { + return new Promise((resolve) => { + setTimeout(() => { + resolve(toReturn); + }, 86); + }); + } flagSets.forEach(flagset => { const featureFlagNames = flagSetsMock[flagset]; if (featureFlagNames) { @@ -124,6 +132,7 @@ test('EVALUATOR - Multiple evaluations at once / should return right labels, tre }); test('EVALUATOR - Multiple evaluations at once by flagsets / should return right labels, treatments and configs if storage returns without errors.', async function () { + const expectedOutput = { config: { treatment: 'on', label: 'in segment all', @@ -134,29 +143,54 @@ test('EVALUATOR - Multiple evaluations at once by flagsets / should return right }, }; - const multipleEvaluationAtOnce = await evaluateFeaturesByFlagSets( - loggerMock, - 'fake-key', - ['reg_and_config', 'arch_and_killed'], - null, - mockStorage, - ); + const getResultsByFlagsets = (flagSets: string[]) => { + return evaluateFeaturesByFlagSets( + loggerMock, + 'fake-key', + flagSets, + null, + mockStorage, + ); + }; + + let multipleResultsAtOnceByFlagSets = await getResultsByFlagsets(['delay']); + expect(multipleResultsAtOnceByFlagSets.elapsedMilliseconds).toBeGreaterThanOrEqual(86); // defined 86 ms delay for testing purposes in mocked storage + + + multipleResultsAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config', 'arch_and_killed']); + let multipleEvaluationAtOnceByFlagSets = multipleResultsAtOnceByFlagSets.evaluations; // assert evaluationWithConfig - expect(multipleEvaluationAtOnce['config']).toEqual(expectedOutput['config']); // If the split is retrieved successfully we should get the right evaluation result, label and config. + expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); // If the split is retrieved successfully we should get the right evaluation result, label and config. // @todo assert flagset not found - for input validations // assert regular - expect(multipleEvaluationAtOnce['regular']).toEqual({ ...expectedOutput['config'], config: null }); // If the split is retrieved successfully we should get the right evaluation result, label and config. If Split has no config it should have config equal null. + expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); // If the split is retrieved successfully we should get the right evaluation result, label and config. If Split has no config it should have config equal null. // assert killed - expect(multipleEvaluationAtOnce['killed']).toEqual({ ...expectedOutput['config'], treatment: 'off', config: null, label: LabelsConstants.SPLIT_KILLED }); + expect(multipleEvaluationAtOnceByFlagSets['killed']).toEqual({ ...expectedOutput['config'], treatment: 'off', config: null, label: LabelsConstants.SPLIT_KILLED }); // 'If the split is retrieved but is killed, we should get the right evaluation result, label and config. // assert archived - expect(multipleEvaluationAtOnce['archived']).toEqual({ ...expectedOutput['config'], treatment: 'control', label: LabelsConstants.SPLIT_ARCHIVED, config: null }); + expect(multipleEvaluationAtOnceByFlagSets['archived']).toEqual({ ...expectedOutput['config'], treatment: 'control', label: LabelsConstants.SPLIT_ARCHIVED, config: null }); // If the split is retrieved but is archived, we should get the right evaluation result, label and config. // assert not_existent_split not in evaluation if it is not related to defined flagsets - expect(multipleEvaluationAtOnce['not_existent_split']).toEqual(undefined); + expect(multipleEvaluationAtOnceByFlagSets['not_existent_split']).toEqual(undefined); + + multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets([]); + expect(multipleEvaluationAtOnceByFlagSets.evaluations).toEqual({}); + + multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config']).evaluations; + expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); // If the split is retrieved successfully we should get the right evaluation result, label and config. + // @todo assert flagset not found - for input validations + + // assert regular + expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); // If the split is retrieved successfully we should get the right evaluation result, label and config. If Split has no config it should have config equal null. + // assert killed + expect(multipleEvaluationAtOnceByFlagSets['killed']).toEqual(undefined); + // 'If the split is retrieved but is killed, we should get the right evaluation result, label and config. + + // assert archived + expect(multipleEvaluationAtOnceByFlagSets['archived']).toEqual(undefined); }); diff --git a/src/evaluator/index.ts b/src/evaluator/index.ts index 8c9c1810..31311d20 100644 --- a/src/evaluator/index.ts +++ b/src/evaluator/index.ts @@ -4,10 +4,11 @@ import * as LabelsConstants from '../utils/labels'; import { CONTROL } from '../utils/constants'; import { ISplit, MaybeThenable } from '../dtos/types'; import { IStorageAsync, IStorageSync } from '../storages/types'; -import { IEvaluationResult } from './types'; +import { IByFlagSetsResult, IEvaluationResult } from './types'; import { SplitIO } from '../types'; import { ILogger } from '../logger/types'; -import { setToArray } from '../utils/lang/sets'; +import { ISet, setToArray } from '../utils/lang/sets'; +import { timer } from '../utils/timeTracker/timer'; const treatmentException = { treatment: CONTROL, @@ -94,8 +95,10 @@ export function evaluateFeaturesByFlagSets( flagsets: string[], attributes: SplitIO.Attributes | undefined, storage: IStorageSync | IStorageAsync, -): MaybeThenable> { - let storedFlagNames; +): MaybeThenable { + const stopTimer = timer(Date.now); + let elapsedMilliseconds: number; + let storedFlagNames: MaybeThenable>; // get ff by flagsets try { @@ -103,9 +106,30 @@ export function evaluateFeaturesByFlagSets( } catch (e) { // Exception on sync `getSplits` storage. Not possible ATM with InMemory and InLocal storages. // @todo - review exception - return treatmentsException(flagsets); + elapsedMilliseconds = stopTimer(); + return {evaluations:{}, elapsedMilliseconds}; + } + + const evaluatedFeatures = getByFlagSetsEvaluations(log, key, storedFlagNames, attributes, storage); + + if (thenable(evaluatedFeatures)) { + return evaluatedFeatures.then((evaluations) => { + elapsedMilliseconds = stopTimer(); + return {evaluations, elapsedMilliseconds}; + }); } + elapsedMilliseconds = stopTimer(); + return {evaluations: evaluatedFeatures, elapsedMilliseconds}; +} + +function getByFlagSetsEvaluations( + log: ILogger, + key: SplitIO.SplitKey, + storedFlagNames: MaybeThenable>, + attributes: SplitIO.Attributes | undefined, + storage: IStorageSync | IStorageAsync, +): MaybeThenable> { return thenable(storedFlagNames) ? storedFlagNames.then((splitNames) => evaluateFeatures(log, key, setToArray(splitNames), attributes, storage)) : evaluateFeatures(log, key, setToArray(storedFlagNames), attributes, storage); diff --git a/src/evaluator/types.ts b/src/evaluator/types.ts index 54078b5b..56d00290 100644 --- a/src/evaluator/types.ts +++ b/src/evaluator/types.ts @@ -27,6 +27,11 @@ export interface IEvaluation { export type IEvaluationResult = IEvaluation & { treatment: string } +export interface IByFlagSetsResult { + evaluations: Record, + elapsedMilliseconds: number +} + export type ISplitEvaluator = (log: ILogger, key: SplitIO.SplitKey, splitName: string, attributes: SplitIO.Attributes | undefined, storage: IStorageSync | IStorageAsync) => MaybeThenable export type IEvaluator = (key: SplitIO.SplitKey, seed: number, trafficAllocation?: number, trafficAllocationSeed?: number, attributes?: SplitIO.Attributes, splitEvaluator?: ISplitEvaluator) => MaybeThenable diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index c6afc365..8e4148bb 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -5,7 +5,7 @@ import { validateSplitExistance } from '../utils/inputValidation/splitExistance' import { validateTrafficTypeExistance } from '../utils/inputValidation/trafficTypeExistance'; import { SDK_NOT_READY } from '../utils/labels'; import { CONTROL, TREATMENT, TREATMENTS, TREATMENT_WITH_CONFIG, TREATMENTS_WITH_CONFIG, TRACK } from '../utils/constants'; -import { IEvaluationResult } from '../evaluator/types'; +import { IByFlagSetsResult, IEvaluationResult } from '../evaluator/types'; import { SplitIO, ImpressionDTO } from '../types'; import { IMPRESSION, IMPRESSION_QUEUEING } from '../logger/constants'; import { ISdkFactoryContext } from '../sdkFactory/types'; @@ -84,11 +84,12 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl function getTreatmentsByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false) { const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS); - const wrapUp = (evaluationResults: Record) => { + const wrapUp = (evaluationResults: IByFlagSetsResult) => { const queue: ImpressionDTO[] = []; const treatments: Record = {}; - Object.keys(evaluationResults).forEach(featureFlagName => { - treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, `getTreatmentsByFlagSets${withConfig ? 'WithConfig' : ''}`, queue); + const evaluations = evaluationResults.evaluations; + Object.keys(evaluations).forEach(featureFlagName => { + treatments[featureFlagName] = processEvaluation(evaluations[featureFlagName], featureFlagName, key, attributes, withConfig, `getTreatmentsByFlagSets${withConfig ? 'WithConfig' : ''}`, queue); }); impressionsTracker.track(queue, attributes); @@ -96,9 +97,11 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl return treatments; }; + const emptyEvaluationByFlagSet = {evaluations: {}, elapsedMilliseconds: 0}; + const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ? evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage) : - isStorageSync(settings) ? {} : Promise.resolve({}); // Promisify if async + isStorageSync(settings) ? emptyEvaluationByFlagSet : Promise.resolve(emptyEvaluationByFlagSet); // Promisify if async return thenable(evaluations) ? evaluations.then((res) => wrapUp(res)) : wrapUp(evaluations); } From d77011d05adc7616a09ef0f67b2b31766debe822 Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Sat, 23 Sep 2023 00:05:20 -0300 Subject: [PATCH 4/6] remove comments --- src/evaluator/__tests__/evaluate-features.spec.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/evaluator/__tests__/evaluate-features.spec.ts b/src/evaluator/__tests__/evaluate-features.spec.ts index 0d6116bc..c914599d 100644 --- a/src/evaluator/__tests__/evaluate-features.spec.ts +++ b/src/evaluator/__tests__/evaluate-features.spec.ts @@ -181,16 +181,9 @@ test('EVALUATOR - Multiple evaluations at once by flagsets / should return right expect(multipleEvaluationAtOnceByFlagSets.evaluations).toEqual({}); multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config']).evaluations; - expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); // If the split is retrieved successfully we should get the right evaluation result, label and config. - // @todo assert flagset not found - for input validations - - // assert regular - expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); // If the split is retrieved successfully we should get the right evaluation result, label and config. If Split has no config it should have config equal null. - // assert killed + expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); + expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); expect(multipleEvaluationAtOnceByFlagSets['killed']).toEqual(undefined); - // 'If the split is retrieved but is killed, we should get the right evaluation result, label and config. - - // assert archived expect(multipleEvaluationAtOnceByFlagSets['archived']).toEqual(undefined); }); From 43035bc09f019cfecb2af954c63ca89cf5917b6e Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Sat, 23 Sep 2023 00:27:10 -0300 Subject: [PATCH 5/6] Refactor - remove unnecessary function --- src/evaluator/index.ts | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/evaluator/index.ts b/src/evaluator/index.ts index 31311d20..da4b22e2 100644 --- a/src/evaluator/index.ts +++ b/src/evaluator/index.ts @@ -97,21 +97,24 @@ export function evaluateFeaturesByFlagSets( storage: IStorageSync | IStorageAsync, ): MaybeThenable { const stopTimer = timer(Date.now); - let elapsedMilliseconds: number; + let elapsedMilliseconds: number = 0; let storedFlagNames: MaybeThenable>; - // get ff by flagsets + // get features by flagsets try { storedFlagNames = storage.splits.getNamesByFlagSets(flagsets); } catch (e) { - // Exception on sync `getSplits` storage. Not possible ATM with InMemory and InLocal storages. - // @todo - review exception + // return empty evaluations elapsedMilliseconds = stopTimer(); return {evaluations:{}, elapsedMilliseconds}; } - const evaluatedFeatures = getByFlagSetsEvaluations(log, key, storedFlagNames, attributes, storage); + // evaluate related features + const evaluatedFeatures = thenable(storedFlagNames) ? + storedFlagNames.then((splitNames) => evaluateFeatures(log, key, setToArray(splitNames), attributes, storage)) : + evaluateFeatures(log, key, setToArray(storedFlagNames), attributes, storage); + // craft IByFlagSetsResult adding elapsedMilliseconds property if (thenable(evaluatedFeatures)) { return evaluatedFeatures.then((evaluations) => { elapsedMilliseconds = stopTimer(); @@ -122,20 +125,6 @@ export function evaluateFeaturesByFlagSets( return {evaluations: evaluatedFeatures, elapsedMilliseconds}; } - -function getByFlagSetsEvaluations( - log: ILogger, - key: SplitIO.SplitKey, - storedFlagNames: MaybeThenable>, - attributes: SplitIO.Attributes | undefined, - storage: IStorageSync | IStorageAsync, -): MaybeThenable> { - return thenable(storedFlagNames) ? - storedFlagNames.then((splitNames) => evaluateFeatures(log, key, setToArray(splitNames), attributes, storage)) : - evaluateFeatures(log, key, setToArray(storedFlagNames), attributes, storage); - -} - function getEvaluation( log: ILogger, splitJSON: ISplit | null, From fb7e99fbbcc9edc4ade8ec1e6f04f73f0ca8fda1 Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Tue, 26 Sep 2023 18:43:50 -0300 Subject: [PATCH 6/6] Fix telemetry --- .../__tests__/evaluate-features.spec.ts | 23 ++++--------- src/evaluator/index.ts | 28 ++++------------ src/evaluator/types.ts | 5 --- src/sdkClient/client.ts | 29 ++++++++--------- src/sdkClient/clientInputValidation.ts | 8 ++--- src/storages/KeyBuilderSS.ts | 4 +++ src/sync/submitters/types.ts | 6 +++- src/types.ts | 32 +++++++++---------- src/utils/constants/index.ts | 4 +++ 9 files changed, 60 insertions(+), 79 deletions(-) diff --git a/src/evaluator/__tests__/evaluate-features.spec.ts b/src/evaluator/__tests__/evaluate-features.spec.ts index c914599d..3b81a37b 100644 --- a/src/evaluator/__tests__/evaluate-features.spec.ts +++ b/src/evaluator/__tests__/evaluate-features.spec.ts @@ -39,14 +39,6 @@ const mockStorage = { }, getNamesByFlagSets(flagSets) { let toReturn = new _Set([]); - // Forced thenable delayed response for testing purposes - if (flagSets[0] === 'delay') { - return new Promise((resolve) => { - setTimeout(() => { - resolve(toReturn); - }, 86); - }); - } flagSets.forEach(flagset => { const featureFlagNames = flagSetsMock[flagset]; if (featureFlagNames) { @@ -131,7 +123,7 @@ test('EVALUATOR - Multiple evaluations at once / should return right labels, tre }); -test('EVALUATOR - Multiple evaluations at once by flagsets / should return right labels, treatments and configs if storage returns without errors.', async function () { +test('EVALUATOR - Multiple evaluations at once by flag sets / should return right labels, treatments and configs if storage returns without errors.', async function () { const expectedOutput = { config: { @@ -153,16 +145,13 @@ test('EVALUATOR - Multiple evaluations at once by flagsets / should return right ); }; - let multipleResultsAtOnceByFlagSets = await getResultsByFlagsets(['delay']); - expect(multipleResultsAtOnceByFlagSets.elapsedMilliseconds).toBeGreaterThanOrEqual(86); // defined 86 ms delay for testing purposes in mocked storage - multipleResultsAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config', 'arch_and_killed']); - let multipleEvaluationAtOnceByFlagSets = multipleResultsAtOnceByFlagSets.evaluations; + let multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config', 'arch_and_killed']); // assert evaluationWithConfig expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); // If the split is retrieved successfully we should get the right evaluation result, label and config. - // @todo assert flagset not found - for input validations + // @todo assert flag set not found - for input validations // assert regular expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); // If the split is retrieved successfully we should get the right evaluation result, label and config. If Split has no config it should have config equal null. @@ -174,13 +163,13 @@ test('EVALUATOR - Multiple evaluations at once by flagsets / should return right expect(multipleEvaluationAtOnceByFlagSets['archived']).toEqual({ ...expectedOutput['config'], treatment: 'control', label: LabelsConstants.SPLIT_ARCHIVED, config: null }); // If the split is retrieved but is archived, we should get the right evaluation result, label and config. - // assert not_existent_split not in evaluation if it is not related to defined flagsets + // assert not_existent_split not in evaluation if it is not related to defined flag sets expect(multipleEvaluationAtOnceByFlagSets['not_existent_split']).toEqual(undefined); multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets([]); - expect(multipleEvaluationAtOnceByFlagSets.evaluations).toEqual({}); + expect(multipleEvaluationAtOnceByFlagSets).toEqual({}); - multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config']).evaluations; + multipleEvaluationAtOnceByFlagSets = await getResultsByFlagsets(['reg_and_config']); expect(multipleEvaluationAtOnceByFlagSets['config']).toEqual(expectedOutput['config']); expect(multipleEvaluationAtOnceByFlagSets['regular']).toEqual({ ...expectedOutput['config'], config: null }); expect(multipleEvaluationAtOnceByFlagSets['killed']).toEqual(undefined); diff --git a/src/evaluator/index.ts b/src/evaluator/index.ts index da4b22e2..70dad68a 100644 --- a/src/evaluator/index.ts +++ b/src/evaluator/index.ts @@ -4,11 +4,10 @@ import * as LabelsConstants from '../utils/labels'; import { CONTROL } from '../utils/constants'; import { ISplit, MaybeThenable } from '../dtos/types'; import { IStorageAsync, IStorageSync } from '../storages/types'; -import { IByFlagSetsResult, IEvaluationResult } from './types'; +import { IEvaluationResult } from './types'; import { SplitIO } from '../types'; import { ILogger } from '../logger/types'; import { ISet, setToArray } from '../utils/lang/sets'; -import { timer } from '../utils/timeTracker/timer'; const treatmentException = { treatment: CONTROL, @@ -92,37 +91,24 @@ export function evaluateFeatures( export function evaluateFeaturesByFlagSets( log: ILogger, key: SplitIO.SplitKey, - flagsets: string[], + flagSets: string[], attributes: SplitIO.Attributes | undefined, storage: IStorageSync | IStorageAsync, -): MaybeThenable { - const stopTimer = timer(Date.now); - let elapsedMilliseconds: number = 0; +): MaybeThenable> { let storedFlagNames: MaybeThenable>; - // get features by flagsets + // get features by flag sets try { - storedFlagNames = storage.splits.getNamesByFlagSets(flagsets); + storedFlagNames = storage.splits.getNamesByFlagSets(flagSets); } catch (e) { // return empty evaluations - elapsedMilliseconds = stopTimer(); - return {evaluations:{}, elapsedMilliseconds}; + return {}; } // evaluate related features - const evaluatedFeatures = thenable(storedFlagNames) ? + return thenable(storedFlagNames) ? storedFlagNames.then((splitNames) => evaluateFeatures(log, key, setToArray(splitNames), attributes, storage)) : evaluateFeatures(log, key, setToArray(storedFlagNames), attributes, storage); - - // craft IByFlagSetsResult adding elapsedMilliseconds property - if (thenable(evaluatedFeatures)) { - return evaluatedFeatures.then((evaluations) => { - elapsedMilliseconds = stopTimer(); - return {evaluations, elapsedMilliseconds}; - }); - } - elapsedMilliseconds = stopTimer(); - return {evaluations: evaluatedFeatures, elapsedMilliseconds}; } function getEvaluation( diff --git a/src/evaluator/types.ts b/src/evaluator/types.ts index 56d00290..54078b5b 100644 --- a/src/evaluator/types.ts +++ b/src/evaluator/types.ts @@ -27,11 +27,6 @@ export interface IEvaluation { export type IEvaluationResult = IEvaluation & { treatment: string } -export interface IByFlagSetsResult { - evaluations: Record, - elapsedMilliseconds: number -} - export type ISplitEvaluator = (log: ILogger, key: SplitIO.SplitKey, splitName: string, attributes: SplitIO.Attributes | undefined, storage: IStorageSync | IStorageAsync) => MaybeThenable export type IEvaluator = (key: SplitIO.SplitKey, seed: number, trafficAllocation?: number, trafficAllocationSeed?: number, attributes?: SplitIO.Attributes, splitEvaluator?: ISplitEvaluator) => MaybeThenable diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 8e4148bb..31aa5f74 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -4,12 +4,13 @@ import { getMatching, getBucketing } from '../utils/key'; import { validateSplitExistance } from '../utils/inputValidation/splitExistance'; import { validateTrafficTypeExistance } from '../utils/inputValidation/trafficTypeExistance'; import { SDK_NOT_READY } from '../utils/labels'; -import { CONTROL, TREATMENT, TREATMENTS, TREATMENT_WITH_CONFIG, TREATMENTS_WITH_CONFIG, TRACK } from '../utils/constants'; -import { IByFlagSetsResult, IEvaluationResult } from '../evaluator/types'; +import { CONTROL, TREATMENT, TREATMENTS, TREATMENT_WITH_CONFIG, TREATMENTS_WITH_CONFIG, TRACK, TREATMENTS_WITH_CONFIG_BY_FLAGSETS, TREATMENTS_BY_FLAGSETS, TREATMENTS_BY_FLAGSET, TREATMENTS_WITH_CONFIG_BY_FLAGSET } from '../utils/constants'; +import { IEvaluationResult } from '../evaluator/types'; import { SplitIO, ImpressionDTO } from '../types'; import { IMPRESSION, IMPRESSION_QUEUEING } from '../logger/constants'; import { ISdkFactoryContext } from '../sdkFactory/types'; import { isStorageSync } from '../trackers/impressionObserver/utils'; +import { Method } from '../sync/submitters/types'; const treatmentNotReady = { treatment: CONTROL, label: SDK_NOT_READY }; @@ -81,13 +82,13 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl return getTreatments(key, featureFlagNames, attributes, true); } - function getTreatmentsByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false) { - const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS); + function getTreatmentsByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined, withConfig = false, method: Method = TREATMENTS_BY_FLAGSETS) { + const stopTelemetryTracker = telemetryTracker.trackEval(method); - const wrapUp = (evaluationResults: IByFlagSetsResult) => { + const wrapUp = (evaluationResults: Record) => { const queue: ImpressionDTO[] = []; const treatments: Record = {}; - const evaluations = evaluationResults.evaluations; + const evaluations = evaluationResults; Object.keys(evaluations).forEach(featureFlagName => { treatments[featureFlagName] = processEvaluation(evaluations[featureFlagName], featureFlagName, key, attributes, withConfig, `getTreatmentsByFlagSets${withConfig ? 'WithConfig' : ''}`, queue); }); @@ -97,25 +98,23 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl return treatments; }; - const emptyEvaluationByFlagSet = {evaluations: {}, elapsedMilliseconds: 0}; - const evaluations = readinessManager.isReady() || readinessManager.isReadyFromCache() ? evaluateFeaturesByFlagSets(log, key, flagSetNames, attributes, storage) : - isStorageSync(settings) ? emptyEvaluationByFlagSet : Promise.resolve(emptyEvaluationByFlagSet); // Promisify if async + isStorageSync(settings) ? {} : Promise.resolve({}); // Promisify if async return thenable(evaluations) ? evaluations.then((res) => wrapUp(res)) : wrapUp(evaluations); } - function getTreatmentsWithConfigByFlagSets(key: SplitIO.SplitKey, featureFlagNames: string[], attributes: SplitIO.Attributes | undefined) { - return getTreatmentsByFlagSets(key, featureFlagNames, attributes, true); + function getTreatmentsWithConfigByFlagSets(key: SplitIO.SplitKey, flagSetNames: string[], attributes: SplitIO.Attributes | undefined) { + return getTreatmentsByFlagSets(key, flagSetNames, attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSETS); } - function getTreatmentsByFlagSet(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined) { - return getTreatmentsByFlagSets(key, [featureFlagName], attributes); + function getTreatmentsByFlagSet(key: SplitIO.SplitKey, flagSetName: string, attributes: SplitIO.Attributes | undefined) { + return getTreatmentsByFlagSets(key, [flagSetName], attributes, false, TREATMENTS_BY_FLAGSET); } - function getTreatmentsWithConfigByFlagSet(key: SplitIO.SplitKey, featureFlagName: string, attributes: SplitIO.Attributes | undefined) { - return getTreatmentsByFlagSets(key, [featureFlagName], attributes, true); + function getTreatmentsWithConfigByFlagSet(key: SplitIO.SplitKey, flagSetName: string, attributes: SplitIO.Attributes | undefined) { + return getTreatmentsByFlagSets(key, [flagSetName], attributes, true, TREATMENTS_WITH_CONFIG_BY_FLAGSET); } // Internal function diff --git a/src/sdkClient/clientInputValidation.ts b/src/sdkClient/clientInputValidation.ts index 0161a43d..94edfafb 100644 --- a/src/sdkClient/clientInputValidation.ts +++ b/src/sdkClient/clientInputValidation.ts @@ -125,8 +125,8 @@ export function clientInputValidationDecorator = { ts: 'treatments', tc: 'treatmentWithConfig', tcs: 'treatmentsWithConfig', + tf: 'treatmentsByFlagSet', + tfs: 'treatmentsByFlagSets', + tcf: 'treatmentsWithConfigByFlagSet', + tcfs: 'treatmentsWithConfigByFlagSets', tr: 'track' }; diff --git a/src/sync/submitters/types.ts b/src/sync/submitters/types.ts index 5b7ff0c4..440f466a 100644 --- a/src/sync/submitters/types.ts +++ b/src/sync/submitters/types.ts @@ -124,7 +124,11 @@ export type TREATMENTS = 'ts'; export type TREATMENT_WITH_CONFIG = 'tc'; export type TREATMENTS_WITH_CONFIG = 'tcs'; export type TRACK = 'tr'; -export type Method = TREATMENT | TREATMENTS | TREATMENT_WITH_CONFIG | TREATMENTS_WITH_CONFIG | TRACK; +export type TREATMENTS_BY_FLAGSET = 'tf' +export type TREATMENTS_BY_FLAGSETS = 'tfs' +export type TREATMENTS_WITH_CONFIG_BY_FLAGSET = 'tcf' +export type TREATMENTS_WITH_CONFIG_BY_FLAGSETS = 'tcfs' +export type Method = TREATMENT | TREATMENTS | TREATMENT_WITH_CONFIG | TREATMENTS_WITH_CONFIG | TRACK | TREATMENTS_BY_FLAGSET | TREATMENTS_BY_FLAGSETS | TREATMENTS_WITH_CONFIG_BY_FLAGSET | TREATMENTS_WITH_CONFIG_BY_FLAGSETS; export type MethodLatencies = Partial>>; diff --git a/src/types.ts b/src/types.ts index 0bc0ea24..2fb862c8 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1161,41 +1161,41 @@ export namespace SplitIO { */ getTreatmentsWithConfig(key: SplitKey, featureFlagNames: string[], attributes?: Attributes): AsyncTreatmentsWithConfig, /** - * Returns a Treatments value, which will be (or eventually be) an object map with the treatments for the features related to the given flagset. + * Returns a Treatments value, which will be (or eventually be) an object map with the treatments for the features related to the given flag set. * For usage on NodeJS as we don't have only one key. * @function getTreatmentsByFlagSet * @param {string} key - The string key representing the consumer. - * @param {string} flagSet - The flagSet name we want to get the treatments. + * @param {string} flagSet - The flag set name we want to get the treatments. * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. * @returns {Treatments} The map with all the TreatmentWithConfig objects */ getTreatmentsByFlagSet(key: SplitKey, flagSet: string, attributes?: Attributes): AsyncTreatments, /** - * Returns a TreatmentWithConfig value, which will be (or eventually be) an object with both treatment and config string for features related to the given flagset. + * Returns a TreatmentWithConfig value, which will be (or eventually be) an object with both treatment and config string for features related to the given flag set. * For usage on NodeJS as we don't have only one key. * @function getTreatmentsWithConfigByFlagSet * @param {string} key - The string key representing the consumer. - * @param {string} flagSet - The flagSet name we want to get the treatments. + * @param {string} flagSet - The flag set name we want to get the treatments. * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. * @returns {Treatments} The map with all the TreatmentWithConfig objects */ getTreatmentsWithConfigByFlagSet(key: SplitKey, flagSet: string, attributes?: Attributes): AsyncTreatmentsWithConfig, /** - * Returns a Treatments value, which will be (or eventually be) an object map with the treatments for the feature flags related to the given flagSets. + * Returns a Treatments value, which will be (or eventually be) an object map with the treatments for the feature flags related to the given flag sets. * For usage on NodeJS as we don't have only one key. * @function getTreatmentsByFlagSets * @param {string} key - The string key representing the consumer. - * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Array} flagSets - An array of the flag set names we want to get the treatments. * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. * @returns {Treatments} The map with all the TreatmentWithConfig objects */ getTreatmentsByFlagSets(key: SplitKey, flagSets: string[], attributes?: Attributes): AsyncTreatments, /** - * Returns a TreatmentWithConfig value, which will be (or eventually be) an object with both treatment and config string for the feature flags related to the given flagSets. + * Returns a TreatmentWithConfig value, which will be (or eventually be) an object with both treatment and config string for the feature flags related to the given flag sets. * For usage on NodeJS as we don't have only one key. * @function getTreatmentsWithConfigByFlagSets * @param {string} key - The string key representing the consumer. - * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Array} flagSets - An array of the flag set names we want to get the treatments. * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. * @returns {Treatments} The map with all the TreatmentWithConfig objects */ @@ -1251,37 +1251,37 @@ export namespace SplitIO { */ getTreatmentsWithConfig(featureFlagNames: string[], attributes?: Attributes): TreatmentsWithConfig, /** - * Returns a Treatments value, which is an object map with the treatments for the feature flags related to the given flagSet. + * Returns a Treatments value, which is an object map with the treatments for the feature flags related to the given flag set. * @function getTreatmentsByFlagSet * @param {string} key - The string key representing the consumer. - * @param {string} flagSet - The flagSet name we want to get the treatments. + * @param {string} flagSet - The flag set name we want to get the treatments. * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. * @returns {Treatments} The map with all the TreatmentWithConfig objects */ getTreatmentsByFlagSet(key: SplitKey, flagSet: string, attributes?: Attributes): Treatments, /** - * Returns a TreatmentsWithConfig value, which is an object map with the TreatmentWithConfig (an object with both treatment and config string) for the feature flags related to the given flagSet. + * Returns a TreatmentsWithConfig value, which is an object map with the TreatmentWithConfig (an object with both treatment and config string) for the feature flags related to the given flag set. * @function getTreatmentsWithConfigByFlagSet * @param {string} key - The string key representing the consumer. - * @param {string} flagSet - The flagSet name we want to get the treatments. + * @param {string} flagSet - The flag set name we want to get the treatments. * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. * @returns {Treatments} The map with all the TreatmentWithConfig objects */ getTreatmentsWithConfigByFlagSet(key: SplitKey, flagSet: string, attributes?: Attributes): TreatmentsWithConfig, /** - * Returns a Returns a Treatments value, which is an object with both treatment and config string for to the feature flags related to the given flagSets. + * Returns a Returns a Treatments value, which is an object with both treatment and config string for to the feature flags related to the given flag sets. * @function getTreatmentsByFlagSets * @param {string} key - The string key representing the consumer. - * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Array} flagSets - An array of the flag set names we want to get the treatments. * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. * @returns {Treatments} The map with all the TreatmentWithConfig objects */ getTreatmentsByFlagSets(key: SplitKey, flagSets: string[], attributes?: Attributes): Treatments, /** - * Returns a TreatmentsWithConfig value, which is an object map with the TreatmentWithConfig (an object with both treatment and config string) for the feature flags related to the given flagSets. + * Returns a TreatmentsWithConfig value, which is an object map with the TreatmentWithConfig (an object with both treatment and config string) for the feature flags related to the given flag sets. * @function getTreatmentsWithConfigByFlagSets * @param {string} key - The string key representing the consumer. - * @param {Array} flagSets - An array of the flagSet names we want to get the treatments. + * @param {Array} flagSets - An array of the flag set names we want to get the treatments. * @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. * @returns {Treatments} The map with all the TreatmentWithConfig objects */ diff --git a/src/utils/constants/index.ts b/src/utils/constants/index.ts index c55984a3..18dc1ccb 100644 --- a/src/utils/constants/index.ts +++ b/src/utils/constants/index.ts @@ -65,6 +65,10 @@ export const TREATMENT = 't'; export const TREATMENTS = 'ts'; export const TREATMENT_WITH_CONFIG = 'tc'; export const TREATMENTS_WITH_CONFIG = 'tcs'; +export const TREATMENTS_BY_FLAGSET = 'tf'; +export const TREATMENTS_BY_FLAGSETS = 'tfs'; +export const TREATMENTS_WITH_CONFIG_BY_FLAGSET = 'tcf'; +export const TREATMENTS_WITH_CONFIG_BY_FLAGSETS = 'tcfs'; export const TRACK = 'tr'; export const CONNECTION_ESTABLISHED = 0;