feat: variant on enabled toggle metrics (#495)

Unleash · Jul 20, 2023 · a0b38aa · a0b38aa
1 parent ef5d59c
commit a0b38aa
Show file tree

Hide file tree

Showing 8 changed files with 137 additions and 55 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "unleash-client",
-  "version": "4.1.0-beta.4",
+  "version": "4.1.0-beta.5",
   "description": "Unleash Client for Node",
   "license": "Apache-2.0",
   "main": "./lib/index.js",

diff --git a/src/client.ts b/src/client.ts
@@ -4,7 +4,7 @@ import { FeatureInterface } from './feature';
 import { RepositoryInterface } from './repository';
 import {
   Variant, VariantDefinition,
-  getDefaultVariant, selectVariant,
+  getDefaultVariant, selectVariant, VariantWithFeatureStatus,
 } from './variant';
 import { Context } from './context';
 import { Constraint, Segment, StrategyResult } from './strategy/strategy';
@@ -31,7 +31,6 @@ export default class UnleashClient extends EventEmitter {
       if (
         !strategy ||
         !strategy.name ||
-        typeof strategy.name !== 'string' ||
         !strategy.isEnabled ||
         typeof strategy.isEnabled !== 'function'
       ) {
@@ -150,7 +149,7 @@ export default class UnleashClient extends EventEmitter {
     }
   }
 
-  getVariant(name: string, context: Context, fallbackVariant?: Variant): Variant {
+  getVariant(name: string, context: Context, fallbackVariant?: Variant): VariantWithFeatureStatus {
     const feature = this.repository.getToggle(name);
     const variant = this.resolveVariant(feature, context, true, fallbackVariant);
     if (feature?.impressionData) {
@@ -171,7 +170,9 @@ export default class UnleashClient extends EventEmitter {
   // This function is intended to close an issue in the proxy where feature enabled
   // state gets checked twice when resolving a variant with random stickiness and
   // gradual rollout. This is not intended for general use, prefer getVariant instead
-  forceGetVariant(name: string, context: Context, fallbackVariant?: Variant): Variant {
+  forceGetVariant(name: string,
+                  context: Context,
+                  fallbackVariant?: Variant): VariantWithFeatureStatus {
     const feature = this.repository.getToggle(name);
     return this.resolveVariant(feature, context, true, fallbackVariant);
   }
@@ -181,40 +182,43 @@ export default class UnleashClient extends EventEmitter {
     context: Context,
     checkToggle: boolean,
     fallbackVariant?: Variant,
-  ): Variant {
+  ): VariantWithFeatureStatus {
     const fallback = fallbackVariant || getDefaultVariant();
 
     if (typeof feature === 'undefined') {
-      return fallback;
+      return { ...fallback, featureEnabled: false };
     }
 
+    let featureEnabled = !checkToggle;
     if (checkToggle) {
       const result = this.isFeatureEnabled(feature, context, () => !!fallbackVariant?.enabled);
+      featureEnabled = result.enabled;
 
       if (result.enabled && result.variant) {
-        return result.variant;
+        return { ...result.variant, featureEnabled };
       }
 
       if (!result.enabled) {
-        return fallback;
+        return { ...fallback, featureEnabled };
       }
     }
 
     if (!feature.variants ||
       !Array.isArray(feature.variants) ||
       feature.variants.length === 0) {
-      return fallback;
+      return { ...fallback, featureEnabled };
     }
 
     const variant: VariantDefinition | null = selectVariant(feature, context);
     if (variant === null) {
-      return fallback;
+      return { ...fallback, featureEnabled };
     }
 
     return {
       name: variant.name,
       payload: variant.payload,
       enabled: true,
+      featureEnabled,
     };
   }
 }
diff --git a/src/test/client.test.ts b/src/test/client.test.ts
@@ -248,6 +248,7 @@ test('should always return defaultVariant if missing variant', (t) => {
   const defaultVariant = {
     enabled: false,
     name: 'disabled',
+    featureEnabled: true
   };
   t.deepEqual(result, defaultVariant);
 
@@ -258,6 +259,7 @@ test('should always return defaultVariant if missing variant', (t) => {
       type: 'string',
       value: '',
     },
+    featureEnabled: true
   };
   const result2 = client.getVariant('feature-but-no-variant', {}, fallback);
 
@@ -349,6 +351,7 @@ test('should favor strategy variant over feature variant', (t) => {
       name: 'strategyVariantName',
       payload: { type: 'string', value: 'strategyVariantValue' },
       enabled: true,
+      featureEnabled: true
     },
   );
 });
@@ -378,7 +381,9 @@ test('should return disabled variant for non-matching strategy variant', (t) =>
   t.deepEqual(variant, {
       name: 'disabled',
       enabled: false,
+      featureEnabled: false,
     },
   );
 });
 
+
diff --git a/src/test/snapshots/client.test.ts.md b/src/test/snapshots/client.test.ts.md
@@ -10,6 +10,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant1',
       payload: {
         type: 'string',
@@ -21,6 +22,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant2',
       payload: {
         type: 'string',
@@ -32,6 +34,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant3',
       payload: {
         type: 'string',
@@ -45,6 +48,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant1',
       payload: {
         type: 'string',
@@ -56,6 +60,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant2',
       payload: {
         type: 'string',
@@ -67,6 +72,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant3',
       payload: {
         type: 'string',
@@ -80,6 +86,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant1',
       payload: {
         type: 'string',
@@ -91,6 +98,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant2',
       payload: {
         type: 'string',
@@ -102,6 +110,7 @@ Generated by [AVA](https://avajs.dev).
 
     {
       enabled: true,
+      featureEnabled: true,
       name: 'variant3',
       payload: {
         type: 'string',

diff --git a/src/test/snapshots/client.test.ts.snap b/src/test/snapshots/client.test.ts.snap
diff --git a/src/test/unleash.test.ts b/src/test/unleash.test.ts
@@ -725,7 +725,8 @@ test('should use provided bootstrap data', (t) =>
       },
     });
 
-    instance.on('error', () => {});
+    instance.on('error', () => {
+    });
 
     instance.on('ready', () => {
       t.true(instance.isEnabled('bootstrappedToggle') === true);
@@ -932,10 +933,12 @@ test('should allow custom repository', (t) =>
         getToggle: () => ({ name: 'test', enabled: true, strategies: [{ name: 'default' }] }),
         getToggles: () => [],
         getSegment: () => undefined,
-        stop: () => {},
+        stop: () => {
+        },
         // @ts-expect-error
         start: () => {
-          setInterval(() => {}, 1000);
+          setInterval(() => {
+          }, 1000);
         },
         // @ts-expect-error
         on: (evt, fun) => {
@@ -946,7 +949,8 @@ test('should allow custom repository', (t) =>
       },
     });
 
-    instance.on('error', () => {});
+    instance.on('error', () => {
+    });
 
     instance.on('ready', () => {
       t.true(instance.isEnabled('test') === true);
@@ -955,14 +959,10 @@ test('should allow custom repository', (t) =>
     });
   }));
 
-test('should report variant metrics', async (t) => {
+
+const metricsCapturingUnleash = (input: any) => {
   const url = getUrl();
-  const repository = new FakeRepo({
-    name: 'toggle-with-variants',
-    enabled: true,
-    strategies: [{ name: 'default', constraints: [] }],
-    variants: [{ name: 'toggle-variant', payload: { type: 'string', value: 'variant value' } }],
-  });
+  const repository = new FakeRepo(input);
   const instance = new Unleash({
     skipInstanceCountWarning: true,
     appName: 'foo',
@@ -978,11 +978,21 @@ test('should report variant metrics', async (t) => {
   instance.on(UnleashEvents.Sent, (data) => {
     capturedData.push(data);
   });
+  // @ts-expect-error
+  return { instance, capturedData };
+};
+
+test('should report variant metrics', async (t) => {
+  const { instance, capturedData } = metricsCapturingUnleash({
+    name: 'toggle-with-variants',
+    enabled: true,
+    strategies: [{ name: 'default', constraints: [] }],
+    variants: [{ name: 'toggle-variant', payload: { type: 'string', value: 'variant value' } }],
+  });
 
   instance.getVariant('toggle-with-variants');
 
   await instance.destroyWithFlush();
-  // @ts-expect-error
   t.deepEqual(capturedData[0].bucket.toggles, {
     'toggle-with-variants': {
       yes: 1,
@@ -991,3 +1001,46 @@ test('should report variant metrics', async (t) => {
     },
   });
 });
+
+test('should report disabled variant metrics', async (t) => {
+  const { instance, capturedData } = metricsCapturingUnleash({
+    name: 'toggle-without-variants',
+    enabled: true,
+    strategies: [{ name: 'default', constraints: [] }],
+    variants: [],
+  });
+
+  instance.getVariant('toggle-without-variants');
+
+  await instance.destroyWithFlush();
+  t.deepEqual(capturedData[0].bucket.toggles, {
+    'toggle-without-variants': {
+      yes: 1,
+      no: 0,
+      variants: { 'disabled': 1 },
+    },
+  });
+});
+
+test('should report disabled toggle metrics', async (t) => {
+  const { instance, capturedData } = metricsCapturingUnleash({
+    name: 'disabled-toggle',
+    enabled: false,
+    strategies: [{ name: 'default', constraints: [] }],
+    variants: [],
+  });
+
+  instance.getVariant('disabled-toggle');
+
+  await instance.destroyWithFlush();
+  t.deepEqual(capturedData[0].bucket.toggles, {
+    'disabled-toggle': {
+      yes: 0,
+      no: 1,
+      variants: { 'disabled': 1 },
+    },
+  });
+});
+
+
+