add bandits configuration to offline bootstrap in precomputed client … #195

leoromanovsky · 2025-01-10T06:49:09Z

…(FF-3796)

labels: mergeable

Fixes: #issue

Motivation and Context

The precomputed clients needs to have two bandit evaluation functionalities:

request bandit evaluation from the server
receive offline init configuration from an external source (ex: nodejs server)

In both cases the SDK initializes in offline mode and is able to perform:

bandit precomputed evaluation
sends bandit logging events if a logger is defined

Not all bandit APIs are at parity with the local-evaluation client: specifically this PR only addeds getBanditAction, not details since those are not available in the API. Nor does it add getBestAction, which was previously developed for coinbase, since that method is not supported in the server.

Description

augments the network requester to add a bandit_actions parameter optionally. its signature matches the server API (https://github.com/Eppo-exp/eppo/blob/main/packages/edge-assignments/src/handlers/handler_assignments.rs#L30-L31). I am explicitly requiring a Record<string, ContextAttributes> instead of the more open-ended BanditActions
allows SDK initialization with bandit actions
adds getBanditAction API method

How has this been tested?

Our test sample data (https://github.com/Eppo-exp/sdk-test-data/blob/main/configuration-wire/precomputed-v1.json) needs precomputed bandits to be able to support testing against the shared data suite. For now adding just a simple case and in-parallel adding these.

leoromanovsky · 2025-01-10T22:39:19Z

src/client/eppo-precomputed-client.ts

    if (options.requestParameters) {
      // Online-mode
      this.requestParameters = options.requestParameters;
    } else {
      // Offline-mode
+
+      // Offline mode depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.
      if (!this.precomputedFlagStore.isInitialized()) {
        logger.error(
          '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',


use logger constant [Eppo SDK]

leoromanovsky · 2025-01-11T00:36:23Z

src/precomputed-requestor.ts

+    const promises: Promise<void>[] = [];
+    promises.push(
+      hydrateConfigurationStore(this.precomputedFlagStore, {
+        entries: precomputedResponse.flags,
+        environment: precomputedResponse.environment ?? { name: UNKNOWN_ENVIRONMENT_NAME },
+        createdAt: precomputedResponse.createdAt,
+        format: precomputedResponse.format,
+        salt: precomputedResponse.salt,
+      }),
+    );
+    if (this.precomputedBanditsStore) {
+      promises.push(
+        hydrateConfigurationStore(this.precomputedBanditsStore, {
+          entries: precomputedResponse.bandits,
+          environment: precomputedResponse.environment ?? { name: UNKNOWN_ENVIRONMENT_NAME },
+          createdAt: precomputedResponse.createdAt,
+          format: precomputedResponse.format,
+          salt: precomputedResponse.salt,
+        }),
+      );
+    }


when a bandit store is present - hydrate it

Confirming both stores use the same salt

js-sdk-common/src/configuration.ts

Lines 139 to 142 in 8a51588

this.salt = salt;

this.bandits = obfuscatePrecomputedBanditMap(this.salt, bandits);

this.flags = obfuscatePrecomputedFlags(this.salt, flags);

}

leoromanovsky · 2025-01-11T00:38:23Z

src/interfaces.ts

-export interface IPrecomputedBandit {
-  banditKey: string;
-  action: string;
+export type BasePrecomputedBandit = {


since we haven't published these interfaces publically, i took a moment to rewrite them to more explicitly mention which are obfuscated.

leoromanovsky · 2025-01-11T00:38:45Z

src/client/eppo-precomputed-client.ts

@@ -382,6 +488,40 @@ export default class EppoPrecomputedClient {
    }
  }

+  private logBanditAction(banditEvent: IBanditEvent): void {


directly copied from local client

leoromanovsky · 2025-01-11T00:39:22Z

src/client/eppo-precomputed-client.ts

+    // An evaluation details object is needed for the bandit event, but it is not used for anything else.
+    const evaluationDetails: IFlagEvaluationDetails = {
+      environmentName: '',
+      flagEvaluationCode: 'MATCH',
+      flagEvaluationDescription: 'Bandit action assigned',
+      variationKey: '',
+      variationValue: '',
+      banditKey: banditEvaluation.banditKey,
+      banditAction: banditEvaluation.action,
+      configFetchedAt: '',
+      configPublishedAt: '',
+      matchedRule: null,
+      matchedAllocation: null,
+      unmatchedAllocations: [],
+      unevaluatedAllocations: [],
+    };


unfortunately an evaluation detail is required for the callback even though we don't support details at this time

Unless there's some dependency preventing this, it would be more elegant to make evaluationDetails optional in IBanditEvent

Thanks for mentioning that - the assignment callback allows it to be null so I will normalize this.

export interface IAssignmentEvent { /** * The flag evaluation details. Null if the flag was precomputed. */ evaluationDetails: IFlagEvaluationDetails | null; }

sameerank

Lgtm. Please take a pass at the linter warnings before merging

sameerank · 2025-01-13T17:28:21Z

src/client/eppo-precomputed-client.ts

      // Offline-mode
+
+      // Offline mode depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.


Can combine these comments

Suggested change

// Offline-mode

// Offline mode depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.

// Offline-mode: depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.

sameerank · 2025-01-14T00:01:46Z

src/client/eppo-precomputed-client.ts

+    // An evaluation details object is needed for the bandit event, but it is not used for anything else.
+    const evaluationDetails: IFlagEvaluationDetails = {
+      environmentName: '',
+      flagEvaluationCode: 'MATCH',
+      flagEvaluationDescription: 'Bandit action assigned',
+      variationKey: '',
+      variationValue: '',
+      banditKey: banditEvaluation.banditKey,
+      banditAction: banditEvaluation.action,
+      configFetchedAt: '',
+      configPublishedAt: '',
+      matchedRule: null,
+      matchedAllocation: null,
+      unmatchedAllocations: [],
+      unevaluatedAllocations: [],
+    };


Unless there's some dependency preventing this, it would be more elegant to make evaluationDetails optional in IBanditEvent

sameerank · 2025-01-14T00:04:43Z

src/precomputed-requestor.ts

+    const promises: Promise<void>[] = [];
+    promises.push(
+      hydrateConfigurationStore(this.precomputedFlagStore, {
+        entries: precomputedResponse.flags,
+        environment: precomputedResponse.environment ?? { name: UNKNOWN_ENVIRONMENT_NAME },
+        createdAt: precomputedResponse.createdAt,
+        format: precomputedResponse.format,
+        salt: precomputedResponse.salt,
+      }),
+    );
+    if (this.precomputedBanditsStore) {
+      promises.push(
+        hydrateConfigurationStore(this.precomputedBanditsStore, {
+          entries: precomputedResponse.bandits,
+          environment: precomputedResponse.environment ?? { name: UNKNOWN_ENVIRONMENT_NAME },
+          createdAt: precomputedResponse.createdAt,
+          format: precomputedResponse.format,
+          salt: precomputedResponse.salt,
+        }),
+      );
+    }


Confirming both stores use the same salt

js-sdk-common/src/configuration.ts

Lines 139 to 142 in 8a51588

this.salt = salt;

this.bandits = obfuscatePrecomputedBanditMap(this.salt, bandits);

this.flags = obfuscatePrecomputedFlags(this.salt, flags);

}

sameerank · 2025-01-14T00:05:33Z

src/client/eppo-precomputed-client.spec.ts

+      },
+    });
+
+    const client = new EppoPrecomputedClient({


Unused variable

…(FF-3796)

leoromanovsky commented Jan 11, 2025

View reviewed changes

leoromanovsky requested review from rasendubi, sameerank and typotter January 11, 2025 01:08

sameerank approved these changes Jan 14, 2025

View reviewed changes

leoromanovsky added 7 commits January 14, 2025 13:01

add bandits configuration to offline bootstrap in precomputed client …

b19462e

…(FF-3796)

add specs

0abba39

add bandit functions

7851deb

remove details; change signature

1a553c8

payload

43dfb91

basic e2e test

49c2db2

4.9.0-alpha.1; fix test helper

be58b32

leoromanovsky force-pushed the lr/ff-3796 branch from 6503800 to be58b32 Compare January 15, 2025 02:32

leoromanovsky added 5 commits January 14, 2025 18:34

allow bandit evaluation reason to be null

919be85

fix lock

7d62bcf

fix registry

8bcde36

Merge branch 'main' into lr/ff-3796

b83e928

nullish

eac825a

leoromanovsky merged commit 3f84c6b into main Jan 15, 2025
8 checks passed

leoromanovsky deleted the lr/ff-3796 branch January 15, 2025 04:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add bandits configuration to offline bootstrap in precomputed client … #195

add bandits configuration to offline bootstrap in precomputed client … #195

leoromanovsky commented Jan 10, 2025 •

edited

Loading

leoromanovsky Jan 10, 2025

leoromanovsky Jan 11, 2025

sameerank Jan 14, 2025

leoromanovsky Jan 11, 2025

leoromanovsky Jan 11, 2025

leoromanovsky Jan 11, 2025

sameerank Jan 14, 2025

leoromanovsky Jan 15, 2025

sameerank left a comment

sameerank Jan 13, 2025

sameerank Jan 14, 2025

sameerank Jan 14, 2025

sameerank Jan 14, 2025

	this.salt = salt;
	this.bandits = obfuscatePrecomputedBanditMap(this.salt, bandits);
	this.flags = obfuscatePrecomputedFlags(this.salt, flags);
	}

		// Offline-mode

		// Offline mode depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.

add bandits configuration to offline bootstrap in precomputed client … #195

add bandits configuration to offline bootstrap in precomputed client … #195

Conversation

leoromanovsky commented Jan 10, 2025 • edited Loading

labels: mergeable

Motivation and Context

Description

How has this been tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sameerank left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leoromanovsky commented Jan 10, 2025 •

edited

Loading