diff --git a/packages/shared/sdk-server/__tests__/MigrationOpTracker.test.ts b/packages/shared/sdk-server/__tests__/MigrationOpTracker.test.ts index 1cb0dd280..8346aa2b5 100644 --- a/packages/shared/sdk-server/__tests__/MigrationOpTracker.test.ts +++ b/packages/shared/sdk-server/__tests__/MigrationOpTracker.test.ts @@ -58,6 +58,83 @@ it('generates an event if the minimal requirements are met.', () => { }); }); +it('can include the variation in the event', () => { + const tracker = new MigrationOpTracker( + 'flag', + { user: 'bob' }, + LDMigrationStage.Off, + LDMigrationStage.Off, + { + kind: 'FALLTHROUGH', + }, + undefined, + 1, + ); + + tracker.op('write'); + tracker.invoked('old'); + + expect(tracker.createEvent()).toMatchObject({ + contextKeys: { user: 'bob' }, + evaluation: { + default: 'off', + key: 'flag', + reason: { kind: 'FALLTHROUGH' }, + value: 'off', + variation: 1, + }, + kind: 'migration_op', + measurements: [ + { + key: 'invoked', + values: { + old: true, + }, + }, + ], + operation: 'write', + }); +}); + +it('can include the version in the event', () => { + const tracker = new MigrationOpTracker( + 'flag', + { user: 'bob' }, + LDMigrationStage.Off, + LDMigrationStage.Off, + { + kind: 'FALLTHROUGH', + }, + undefined, + undefined, + 2, + ); + + tracker.op('write'); + tracker.invoked('old'); + + expect(tracker.createEvent()).toMatchObject({ + contextKeys: { user: 'bob' }, + evaluation: { + default: 'off', + key: 'flag', + reason: { kind: 'FALLTHROUGH' }, + value: 'off', + version: 2, + }, + kind: 'migration_op', + measurements: [ + { + key: 'invoked', + values: { + old: true, + }, + }, + ], + operation: 'write', + }); +}); + it('includes errors if at least one is set', () => { const tracker = new MigrationOpTracker( 'flag', @@ -250,6 +327,7 @@ it('can handle exceptions thrown in the consistency check method', () => { undefined, undefined, undefined, + undefined, logger, ); tracker.op('read'); @@ -265,3 +343,108 @@ it('can handle exceptions thrown in the consistency check method', () => { }, ]); }); + +it.each([ + [false, true, true, false], + [true, false, false, true], + [false, true, true, true], + [true, false, true, true], +])( + 'does not generate an event if latency measurement without correct invoked measurement' + + ' invoke old: %p invoke new: %p measure old: %p measure new: %p', + (invoke_old, invoke_new, measure_old, measure_new) => { + const tracker = new MigrationOpTracker( + 'flag', + { user: 'bob' }, + LDMigrationStage.Off, + LDMigrationStage.Off, + { + kind: 'FALLTHROUGH', + }, + ); + + tracker.op('write'); + if (invoke_old) { + tracker.invoked('old'); + } + if (invoke_new) { + tracker.invoked('new'); + } + if (measure_old) { + tracker.latency('old', 100); + } + if (measure_new) { + tracker.latency('new', 100); + } + + expect(tracker.createEvent()).toBeUndefined(); + }, +); + +it.each([ + [false, true, true, false], + [true, false, false, true], + [false, true, true, true], + [true, false, true, true], +])( + 'does not generate an event error measurement without correct invoked measurement' + + ' invoke old: %p invoke new: %p measure old: %p measure new: %p', + (invoke_old, invoke_new, measure_old, measure_new) => { + const tracker = new MigrationOpTracker( + 'flag', + { user: 'bob' }, + LDMigrationStage.Off, + LDMigrationStage.Off, + { + kind: 'FALLTHROUGH', + }, + ); + + tracker.op('write'); + if (invoke_old) { + tracker.invoked('old'); + } + if (invoke_new) { + tracker.invoked('new'); + } + if (measure_old) { + tracker.error('old'); + } + if (measure_new) { + tracker.error('new'); + } + + expect(tracker.createEvent()).toBeUndefined(); + }, +); + +it.each([ + [true, false, true], + [false, true, true], + [true, false, false], + [false, true, false], +])( + 'does not generate an event if there is a consistency measurement but both origins were not invoked' + + ' invoke old: %p invoke new: %p consistent: %p', + (invoke_old, invoke_new, consistent) => { + const tracker = new MigrationOpTracker( + 'flag', + { user: 'bob' }, + LDMigrationStage.Off, + LDMigrationStage.Off, + { + kind: 'FALLTHROUGH', + }, + ); + + tracker.op('write'); + if (invoke_old) { + tracker.invoked('old'); + } + if (invoke_new) { + tracker.invoked('new'); + } + tracker.consistency(() => consistent); + expect(tracker.createEvent()).toBeUndefined(); + }, +); diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 3b8590ad7..80c94f904 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -336,6 +336,7 @@ export default class LDClientImpl implements LDClient { reason, checkRatio, undefined, + undefined, samplingRatio, ), }); @@ -352,6 +353,7 @@ export default class LDClientImpl implements LDClient { checkRatio, // Can be null for compatibility reasons. detail.variationIndex === null ? undefined : detail.variationIndex, + flag?.version, samplingRatio, ), }); diff --git a/packages/shared/sdk-server/src/MigrationOpEventConversion.ts b/packages/shared/sdk-server/src/MigrationOpEventConversion.ts index f18348c30..75b6a1530 100644 --- a/packages/shared/sdk-server/src/MigrationOpEventConversion.ts +++ b/packages/shared/sdk-server/src/MigrationOpEventConversion.ts @@ -196,6 +196,10 @@ function validateEvaluation(evaluation: LDMigrationEvaluation): LDMigrationEvalu validated.variation = evaluation.variation; } + if (evaluation.version !== undefined && TypeValidators.Number.is(evaluation.version)) { + validated.version = evaluation.version; + } + return validated; } diff --git a/packages/shared/sdk-server/src/MigrationOpTracker.ts b/packages/shared/sdk-server/src/MigrationOpTracker.ts index 795572c00..9001e42fd 100644 --- a/packages/shared/sdk-server/src/MigrationOpTracker.ts +++ b/packages/shared/sdk-server/src/MigrationOpTracker.ts @@ -26,7 +26,7 @@ export default class MigrationOpTracker implements LDMigrationTracker { new: false, }; - private consistencyCheck?: LDConsistencyCheck; + private consistencyCheck: LDConsistencyCheck = LDConsistencyCheck.NotChecked; private latencyMeasurement = { old: NaN, @@ -43,6 +43,7 @@ export default class MigrationOpTracker implements LDMigrationTracker { private readonly reason: LDEvaluationReason, private readonly checkRatio?: number, private readonly variation?: number, + private readonly version?: number, private readonly samplingRatio?: number, private readonly logger?: LDLogger, ) {} @@ -101,13 +102,16 @@ export default class MigrationOpTracker implements LDMigrationTracker { return undefined; } + if (!this.measurementConsistencyCheck()) { + return undefined; + } + const measurements: LDMigrationMeasurement[] = []; this.populateInvoked(measurements); this.populateConsistency(measurements); this.populateLatency(measurements); this.populateErrors(measurements); - this.measurementConsistencyCheck(); return { kind: 'migration_op', @@ -120,6 +124,7 @@ export default class MigrationOpTracker implements LDMigrationTracker { default: this.defaultStage, reason: this.reason, variation: this.variation, + version: this.version, }, measurements, samplingRatio: this.samplingRatio ?? 1, @@ -145,32 +150,35 @@ export default class MigrationOpTracker implements LDMigrationTracker { ); } - private checkOriginEventConsistency(origin: LDMigrationOrigin) { + private checkOriginEventConsistency(origin: LDMigrationOrigin): boolean { if (this.wasInvoked[origin]) { - return; + return true; } // If the specific origin was not invoked, but it contains measurements, then // that is a problem. Check each measurement and log a message if it is present. if (!Number.isNaN(this.latencyMeasurement[origin])) { this.logger?.error(`${this.logTag()} ${this.latencyConsistencyMessage(origin)}`); + return false; } if (this.errors[origin]) { this.logger?.error(`${this.logTag()} ${this.errorConsistencyMessage(origin)}`); + return false; } if (this.consistencyCheck !== LDConsistencyCheck.NotChecked) { this.logger?.error(`${this.logTag()} ${this.consistencyCheckConsistencyMessage(origin)}`); + return false; } + return true; } /** * Check that the latency, error, consistency and invoked measurements are self-consistent. */ - private measurementConsistencyCheck() { - this.checkOriginEventConsistency('old'); - this.checkOriginEventConsistency('new'); + private measurementConsistencyCheck(): boolean { + return this.checkOriginEventConsistency('old') && this.checkOriginEventConsistency('new'); } private populateInvoked(measurements: LDMigrationMeasurement[]) { diff --git a/packages/shared/sdk-server/src/api/data/LDMigrationOpEvent.ts b/packages/shared/sdk-server/src/api/data/LDMigrationOpEvent.ts index c3e69d702..544a7b34c 100644 --- a/packages/shared/sdk-server/src/api/data/LDMigrationOpEvent.ts +++ b/packages/shared/sdk-server/src/api/data/LDMigrationOpEvent.ts @@ -13,6 +13,7 @@ export interface LDMigrationEvaluation { value: LDMigrationStage; default: LDMigrationStage; variation?: number; + version?: number; reason: LDEvaluationReason; }