Skip to content

Commit

Permalink
Merge branch 'feat/node-migrations' into rlamb/sc-218354/migration-fa…
Browse files Browse the repository at this point in the history 
…ctory-method
  • Loading branch information
@kinyoklion
kinyoklion authored Sep 25, 2023
2 parents 20f5334 + 9c5f402 commit 84b095c
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 7 deletions.
183 changes: 183 additions & 0 deletions packages/shared/sdk-server/__tests__/MigrationOpTracker.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -250,6 +327,7 @@ it('can handle exceptions thrown in the consistency check method', () => {
undefined,
undefined,
undefined,
undefined,
logger,
);
tracker.op('read');
Expand All @@ -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();
},
);
2 changes: 2 additions & 0 deletions packages/shared/sdk-server/src/LDClientImpl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ export default class LDClientImpl implements LDClient {
reason,
checkRatio,
undefined,
undefined,
samplingRatio,
),
});
Expand All @@ -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,
),
});
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/sdk-server/src/MigrationOpEventConversion.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
22 changes: 15 additions & 7 deletions packages/shared/sdk-server/src/MigrationOpTracker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default class MigrationOpTracker implements LDMigrationTracker {
new: false,
};

private consistencyCheck?: LDConsistencyCheck;
private consistencyCheck: LDConsistencyCheck = LDConsistencyCheck.NotChecked;

private latencyMeasurement = {
old: NaN,
Expand All @@ -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,
) {}
Expand Down Expand Up @@ -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',
Expand All @@ -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,
Expand All @@ -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[]) {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/sdk-server/src/api/data/LDMigrationOpEvent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface LDMigrationEvaluation {
value: LDMigrationStage;
default: LDMigrationStage;
variation?: number;
version?: number;
reason: LDEvaluationReason;
}

Expand Down

0 comments on commit 84b095c

Please sign in to comment.