feat: Remove custom measurements.

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

@@ -382,10 +382,7 @@ describe('given an LDClient with test data', () => {
   });
 });
 
-// Out migrator doesn't create custom measurements. So we need an additional test to ensure
-// that custom measurements make it through the conversion process.
-
-it('can accept custom measurements', () => {
+it('ignores invalid measurement keys', () => {
   const inputEvent: LDMigrationOpEvent = {
     kind: 'migration_op',
     operation: 'read',
@@ -401,39 +398,20 @@ it('can accept custom measurements', () => {
     },
     measurements: [
       {
-        key: 'custom1',
-        kind: 'custom',
+        // @ts-ignore
+        key: 'bad',
         values: {
           old: 1,
           new: 2,
         },
       },
-      {
-        key: 'custom2',
-        kind: 'custom',
-        values: {
-          new: 2,
-        },
-      },
-      {
-        key: 'custom3',
-        kind: 'custom',
-        values: {
-          old: 2,
-        },
-      },
-      {
-        key: 'custom4',
-        kind: 'custom',
-        values: {},
-      },
     ],
   };
   const validatedEvent = MigrationOpEventConversion(inputEvent);
-  expect(validatedEvent).toEqual(inputEvent);
+  expect(validatedEvent).toEqual({ ...inputEvent, measurements: [] });
 });
 
-it('removes bad custom measurements', () => {
+it('invalid data types are filtered', () => {
   const inputEvent: LDMigrationOpEvent = {
     kind: 'migration_op',
     operation: 'read',
@@ -449,14 +427,26 @@ it('removes bad custom measurements', () => {
     },
     measurements: [
       {
-        key: 'custom1',
-        kind: 'custom',
+        key: 'latency_ms',
         values: {
           // @ts-ignore
           old: 'ham',
           new: 2,
         },
       },
+      {
+        key: 'consistent',
+        // @ts-ignore
+        value: undefined,
+      },
+      {
+        key: 'error',
+        values: {
+          // @ts-ignore
+          old: {},
+          new: 2,
+        },
+      },
     ],
   };
   const validatedEvent = MigrationOpEventConversion(inputEvent);

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

@@ -175,50 +175,3 @@ it('includes if the result was inconsistent', () => {
     samplingOdds: 0,
   });
 });
-
-it('allows for the addition of custom measurements', () => {
-  const tracker = new MigrationOpTracker(
-    'flag',
-    { user: 'bob' },
-    LDMigrationStage.Off,
-    LDMigrationStage.Off,
-    {
-      kind: 'FALLTHROUGH',
-    },
-  );
-  tracker.op('read');
-
-  tracker.custom({
-    kind: 'custom',
-    key: 'cats',
-    values: {
-      old: 3,
-      new: 10,
-    },
-  });
-
-  tracker.custom({
-    kind: 'custom',
-    key: 'badgers',
-    values: {
-      new: 10,
-    },
-  });
-
-  const event = tracker.createEvent();
-  expect(event?.measurements).toContainEqual({
-    key: 'cats',
-    kind: 'custom',
-    values: {
-      old: 3,
-      new: 10,
-    },
-  });
-  expect(event?.measurements).toContainEqual({
-    key: 'badgers',
-    kind: 'custom',
-    values: {
-      new: 10,
-    },
-  });
-});

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

@@ -2,7 +2,6 @@ import { internal, TypeValidators } from '@launchdarkly/js-sdk-common';
 
 import {
   LDMigrationConsistencyMeasurement,
-  LDMigrationCustomMeasurement,
   LDMigrationErrorMeasurement,
   LDMigrationEvaluation,
   LDMigrationLatencyMeasurement,
@@ -19,10 +18,6 @@ function isOperation(value: LDMigrationOp) {
   return value === 'read' || value === 'write';
 }
 
-function isCustomMeasurement(value: LDMigrationMeasurement): value is LDMigrationCustomMeasurement {
-  return (value as any).kind === 'custom';
-}
-
 function isLatencyMeasurement(
   value: LDMigrationMeasurement,
 ): value is LDMigrationLatencyMeasurement {
@@ -54,27 +49,13 @@ function areValidValues(values: { old?: number; new?: number }) {
 function validateMeasurement(
   measurement: LDMigrationMeasurement,
 ): LDMigrationMeasurement | undefined {
+  // Here we are protecting ourselves from JS callers. TypeScript says that
+  // it cannot be an empty string, but those using JS can do what they want.
+  // @ts-ignore
   if (!TypeValidators.String.is(measurement.key) || measurement.key === '') {
     return undefined;
   }
 
-  if (isCustomMeasurement(measurement)) {
-    if (!TypeValidators.Object.is(measurement.values)) {
-      return undefined;
-    }
-    if (!areValidValues(measurement.values)) {
-      return undefined;
-    }
-    return {
-      kind: measurement.kind,
-      key: measurement.key,
-      values: {
-        old: measurement.values.old,
-        new: measurement.values.new,
-      },
-    };
-  }
-
   if (isLatencyMeasurement(measurement)) {
     if (!TypeValidators.Object.is(measurement.values)) {
       return undefined;
@@ -121,6 +102,7 @@ function validateMeasurement(
     };
   }
 
+  // Not a supported measurement type.
   return undefined;
 }
 

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

@@ -3,7 +3,6 @@ import { LDEvaluationReason } from '@launchdarkly/js-sdk-common';
 import { LDMigrationStage, LDMigrationTracker } from './api';
 import {
   LDConsistencyCheck,
-  LDMigrationCustomMeasurement,
   LDMigrationMeasurement,
   LDMigrationOp,
   LDMigrationOpEvent,
@@ -29,8 +28,6 @@ export default class MigrationOpTracker implements LDMigrationTracker {
 
   private operation?: LDMigrationOp;
 
-  private customMeasurements: LDMigrationCustomMeasurement[] = [];
-
   constructor(
     private readonly flagKey: string,
     private readonly contextKeys: Record<string, string>,
@@ -56,13 +53,9 @@ export default class MigrationOpTracker implements LDMigrationTracker {
     this.latencyMeasurement[origin] = value;
   }
 
-  custom(measurement: LDMigrationCustomMeasurement) {
-    this.customMeasurements.push(measurement);
-  }
-
   createEvent(): LDMigrationOpEvent | undefined {
     if (this.operation && Object.keys(this.contextKeys).length) {
-      const measurements = [...this.customMeasurements];
+      const measurements: LDMigrationMeasurement[] = [];
 
       this.populateConsistency(measurements);
       this.populateLatency(measurements);

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

@@ -1,11 +1,7 @@
 import { LDEvaluationReason } from '@launchdarkly/js-sdk-common';
 
 import { LDMigrationOrigin } from '../LDMigration';
-import {
-  LDMigrationCustomMeasurement,
-  LDMigrationOp,
-  LDMigrationOpEvent,
-} from './LDMigrationOpEvent';
+import { LDMigrationOp, LDMigrationOpEvent } from './LDMigrationOpEvent';
 import { LDMigrationStage } from './LDMigrationStage';
 
 /**
@@ -52,14 +48,6 @@ export interface LDMigrationTracker {
    */
   latency(origin: LDMigrationOrigin, value: number): void;
 
-  /**
-   * Report a custom measurement. Unlike other methods on the tracker multiple custom
-   * measurements can be reported.
-   *
-   * @param measurement The custom measurement to track.
-   */
-  custom(measurement: LDMigrationCustomMeasurement): void;
-
   /**
    * Create a migration op event. If the event could not be created, because of a missing
    * operation, then undefined is returned.

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

@@ -16,15 +16,6 @@ export interface LDMigrationEvaluation {
   reason: LDEvaluationReason;
 }
 
-export interface LDMigrationCustomMeasurement {
-  kind: 'custom';
-  key: string;
-  values: {
-    old?: number;
-    new?: number;
-  };
-}
-
 export interface LDMigrationConsistencyMeasurement {
   key: 'consistent';
   value: number;
@@ -53,8 +44,7 @@ export interface LDMigrationErrorMeasurement {
 export type LDMigrationMeasurement =
   | LDMigrationLatencyMeasurement
   | LDMigrationErrorMeasurement
-  | LDMigrationConsistencyMeasurement
-  | LDMigrationCustomMeasurement;
+  | LDMigrationConsistencyMeasurement;
 
 /**
  * Event used to track information about a migration operation.