Merge branch 'feat/node-migrations' into rlamb/merge-perf-updates-mig…

…rations

packages/shared/sdk-server/__tests__/MigrationOpTracker.test.ts

@@ -1,4 +1,4 @@
-import { LDConsistencyCheck, LDMigrationStage } from '../src';
+import { LDMigrationStage } from '../src';
 import { LDMigrationOrigin } from '../src/api/LDMigration';
 import MigrationOpTracker from '../src/MigrationOpTracker';
 
@@ -160,7 +160,7 @@ it('includes if the result was consistent', () => {
     },
   );
   tracker.op('read');
-  tracker.consistency(LDConsistencyCheck.Consistent);
+  tracker.consistency(() => true);
   tracker.invoked('old');
   tracker.invoked('new');
 
@@ -185,7 +185,7 @@ it('includes if the result was inconsistent', () => {
   tracker.op('read');
   tracker.invoked('old');
   tracker.invoked('new');
-  tracker.consistency(LDConsistencyCheck.Inconsistent);
+  tracker.consistency(() => false);
 
   const event = tracker.createEvent();
   expect(event?.measurements).toContainEqual({

packages/shared/sdk-server/src/LDClientImpl.ts

@@ -18,9 +18,9 @@ import {
   LDFeatureStore,
   LDFlagsState,
   LDFlagsStateOptions,
-  LDMigrationDetail,
   LDMigrationOpEvent,
   LDMigrationStage,
+  LDMigrationVariation,
   LDOptions,
   LDStreamProcessor,
 } from './api';
@@ -305,7 +305,7 @@ export default class LDClientImpl implements LDClient {
     key: string,
     context: LDContext,
     defaultValue: LDMigrationStage,
-  ): Promise<LDMigrationDetail> {
+  ): Promise<LDMigrationVariation> {
     const convertedContext = Context.fromLDContext(context);
     return new Promise((resolve) => {
       this.evaluateIfPossible(
@@ -328,8 +328,6 @@ export default class LDClientImpl implements LDClient {
             };
             resolve({
               value: defaultValue,
-              reason,
-              checkRatio,
               tracker: new MigrationOpTracker(
                 key,
                 contextKeys,
@@ -344,9 +342,7 @@ export default class LDClientImpl implements LDClient {
             return;
           }
           resolve({
-            ...detail,
             value: detail.value as LDMigrationStage,
-            checkRatio,
             tracker: new MigrationOpTracker(
               key,
               contextKeys,

packages/shared/sdk-server/src/Migration.ts

@@ -1,6 +1,6 @@
-import { internal, LDContext } from '@launchdarkly/js-sdk-common';
+import { LDContext } from '@launchdarkly/js-sdk-common';
 
-import { LDClient, LDConsistencyCheck, LDMigrationStage, LDMigrationTracker } from './api';
+import { LDClient, LDMigrationStage, LDMigrationTracker } from './api';
 import {
   LDMigration,
   LDMigrationOrigin,
@@ -17,8 +17,6 @@ import {
   LDSerialExecution,
 } from './api/options/LDMigrationOptions';
 
-const { shouldSample } = internal;
-
 type MultipleReadResult<TMigrationRead> = {
   fromOld: LDMigrationReadResult<TMigrationRead>;
   fromNew: LDMigrationReadResult<TMigrationRead>;
@@ -88,7 +86,6 @@ export function LDMigrationError(error: Error): { success: false; error: Error }
 interface MigrationContext<TPayload> {
   payload?: TPayload;
   tracker: LDMigrationTracker;
-  checkRatio?: number;
 }
 
 /**
@@ -239,7 +236,6 @@ export default class Migration<
     const res = await this.readTable[stage.value]({
       payload,
       tracker: stage.tracker,
-      checkRatio: stage.checkRatio,
     });
     stage.tracker.op('read');
     this.sendEvent(stage.tracker);
@@ -274,13 +270,13 @@ export default class Migration<
     oldValue: LDMethodResult<TMigrationRead>,
     newValue: LDMethodResult<TMigrationRead>,
   ) {
-    if (this.config.check && shouldSample(context.checkRatio ?? 1)) {
-      if (oldValue.success && newValue.success) {
-        const res = this.config.check(oldValue.result, newValue.result);
-        context.tracker.consistency(
-          res ? LDConsistencyCheck.Consistent : LDConsistencyCheck.Inconsistent,
-        );
-      }
+    if (!this.config.check) {
+      return;
+    }
+
+    if (oldValue.success && newValue.success) {
+      // Check is validated before this point, so it is force unwrapped.
+      context.tracker.consistency(() => this.config.check!(oldValue.result, newValue.result));
     }
   }
 

packages/shared/sdk-server/src/MigrationOpTracker.ts

@@ -1,4 +1,4 @@
-import { LDEvaluationReason, LDLogger } from '@launchdarkly/js-sdk-common';
+import { internal, LDEvaluationReason, LDLogger } from '@launchdarkly/js-sdk-common';
 
 import { LDMigrationStage, LDMigrationTracker } from './api';
 import {
@@ -55,8 +55,11 @@ export default class MigrationOpTracker implements LDMigrationTracker {
     this.errors[origin] = true;
   }
 
-  consistency(result: LDConsistencyCheck) {
-    this.consistencyCheck = result;
+  consistency(check: () => boolean) {
+    if (internal.shouldSample(this.checkRatio ?? 1)) {
+      const res = check();
+      this.consistencyCheck = res ? LDConsistencyCheck.Consistent : LDConsistencyCheck.Inconsistent;
+    }
   }
 
   latency(origin: LDMigrationOrigin, value: number) {

packages/shared/sdk-server/src/api/LDClient.ts

@@ -1,6 +1,6 @@
 import { LDContext, LDEvaluationDetail, LDFlagValue } from '@launchdarkly/js-sdk-common';
 
-import { LDMigrationDetail, LDMigrationOpEvent } from './data';
+import { LDMigrationOpEvent, LDMigrationVariation } from './data';
 import { LDFlagsState } from './data/LDFlagsState';
 import { LDFlagsStateOptions } from './data/LDFlagsStateOptions';
 import { LDMigrationStage } from './data/LDMigrationStage';
@@ -135,13 +135,13 @@ export interface LDClient {
    * @param defaultValue The default value of the flag, to be used if the value is not available
    *   from LaunchDarkly.
    * @returns
-   *   A Promise which will be resolved with the result (as an{@link LDMigrationDetail}).
+   *   A Promise which will be resolved with the result (as an{@link LDMigrationVariation}).
    */
   variationMigration(
     key: string,
     context: LDContext,
     defaultValue: LDMigrationStage,
-  ): Promise<LDMigrationDetail>;
+  ): Promise<LDMigrationVariation>;
 
   /**
    * Builds an object that encapsulates the state of all feature flags for a given context.

packages/shared/sdk-server/src/api/data/LDMigrationDetail.ts → packages/shared/sdk-server/src/api/data/LDMigrationVariation.ts

@@ -1,5 +1,3 @@
-import { LDEvaluationReason } from '@launchdarkly/js-sdk-common';
-
 import { LDMigrationOrigin } from '../LDMigration';
 import { LDMigrationOp, LDMigrationOpEvent } from './LDMigrationOpEvent';
 import { LDMigrationStage } from './LDMigrationStage';
@@ -32,11 +30,21 @@ export interface LDMigrationTracker {
   error(origin: LDMigrationOrigin): void;
 
   /**
-   * Report the result of a consistency check.
+   * Check the consistency of a read result. This method should be invoked if the `check` function
+   * is defined for the migration and both reads ("new"/"old") were done.
+   *
+   * The function will use the checkRatio to determine if the check should be executed, and it
+   * will record the result.
    *
-   * @param result The result of the check.
+   * Example calling the check function from the migration config.
+   * ```
+   * context.tracker.consistency(() => config.check!(oldValue.result, newValue.result));
+   * ```
+   *
+   * @param check The function which executes the check. This is not the `check` function from the
+   * migration options, but instead should be a parameter-less function that calls that function.
    */
-  consistency(result: LDConsistencyCheck): void;
+  consistency(check: () => boolean): void;
 
   /**
    * Call this to report that an origin was invoked (executed). There are some situations where the
@@ -64,40 +72,17 @@ export interface LDMigrationTracker {
 }
 
 /**
- * Detailed information about a migration variation.
+ * Migration value and tracker.
  */
-export interface LDMigrationDetail {
+export interface LDMigrationVariation {
   /**
    * The result of the flag evaluation. This will be either one of the flag's variations or
-   * the default value that was passed to `LDClient.variationDetail`.
+   * the default value that was passed to `LDClient.variationMigration`.
    */
   value: LDMigrationStage;
 
   /**
-   * The index of the returned value within the flag's list of variations, e.g. 0 for the
-   * first variation-- or `null` if the default value was returned.
-   */
-  variationIndex?: number | null;
-
-  /**
-   * An object describing the main factor that influenced the flag evaluation value.
-   */
-  reason: LDEvaluationReason;
-
-  /**
-   * A tracker which which can be used to generate analytics for the migration.
+   * A tracker which can be used to generate analytics for the migration.
    */
   tracker: LDMigrationTracker;
-
-  /**
-   * When present represents a 1 in X ratio indicating the probability that a given operation
-   * should have its consistency checked. A 1 indicates that it should always be sampled and
-   * 0 indicates that it never should be sampled.
-   */
-  checkRatio?: number;
-
-  /**
-   * Sampling ratio for the migration event. Defaults to 1 if not specified.
-   */
-  samplingRatio?: number;
 }

packages/shared/sdk-server/src/api/data/index.ts

@@ -1,5 +1,5 @@
 export * from './LDFlagsStateOptions';
 export * from './LDFlagsState';
 export * from './LDMigrationStage';
-export * from './LDMigrationDetail';
 export * from './LDMigrationOpEvent';
+export * from './LDMigrationVariation';