feat: create offlinePrecomputedInit #117

sameerank · 2024-12-05T17:36:03Z

labels: mergeable

Fixes: #issue

Motivation and Context

Description

Adds a method for offlinePrecomputedInit

Depends on Eppo-exp/js-sdk-common#163 because we need a way to persist the subject key and attributes without relying on request parameters, since the offline init doesn't make requests

How has this been tested?

sameerank · 2024-12-09T08:10:30Z

package.json

@@ -59,7 +59,7 @@
    "webpack-cli": "^4.10.0"
  },
  "dependencies": {
-    "@eppo/js-client-sdk-common": "^4.5.4"
+    "@eppo/js-client-sdk-common": "^4.6.1"


Depends on this to be published Eppo-exp/js-sdk-common#163

sameerank · 2024-12-09T08:54:42Z

src/i-client-config.ts

+ * Configuration for Eppo precomputed client initialization
+ * @public
+ */
+export interface IPrecomputedClientConfig extends IBaseRequestConfig {


Breaking change to the interface of precomputedInit but assuming it is safe because we are currently the only users of this function

sameerank · 2024-12-09T08:57:48Z

Marking as ready for review because I expect the tests to pass once the js common dependency is published

aarsilv

Pieces coming together 💪

src/index.ts

leoromanovsky

I haven't reviewed this yet but since it has a ✅ please remember to test this build by compiled the production package.

…computed-client

sameerank · 2024-12-11T06:42:44Z

Makefile

@@ -27,14 +27,15 @@ help: Makefile
 testDataDir := test/data/
 tempDir := ${testDataDir}temp/
 gitDataDir := ${tempDir}sdk-test-data/
-branchName := main
+branchName := sameeran/ff-3687-obfuscated-precomputed-json


Temporarily using this branch for testing until Eppo-exp/sdk-test-data#89 gets an approval 🙏

sameerank · 2024-12-11T06:48:24Z

src/index.spec.ts

+  let precomputedConfigurationWire: string;
+
+  beforeAll(() => {
+    precomputedConfigurationWire = readMockPrecomputedResponse(MOCK_PRECOMPUTED_RESPONSE_FILE);


Currently, tests use the unobfuscated, unstringified ConfigurationWire format https://github.com/Eppo-exp/sdk-test-data/blob/8ee55117b7c7f5d02b2997629af0f0fa93e4e97c/configuration-wire/precomputed-v1.json

In a follow up PR I will add support for parsing json strings and handling obfuscated fields

sameerank · 2024-12-11T06:58:16Z

src/index.ts

+export interface IConfigurationWire {
+  version: number;
+  precomputed: {
+    subjectKey: string;
+    subjectAttributes: Record<string, AttributeType>;
+    fetchedAt: string;
+    response: {
+      createdAt: string;
+      format: FormatEnum;
+      obfuscated: boolean;
+      environment: Environment;
+      flags: Record<string, PrecomputedFlag>;
+    };
+  };
+}


Will replace with an exported interface from Eppo-exp/js-sdk-common#160

src/index.ts

…computed-client

sameerank · 2024-12-16T07:32:44Z

src/index.spec.ts

@@ -1205,7 +1214,7 @@ describe('EppoClient config', () => {
    const retryManager = eventDispatcher['retryManager'];
    const batchProcessor = eventDispatcher['batchProcessor'];
    expect(eventDispatcher['deliveryIntervalMs']).toEqual(1);
-    expect(batchProcessor['batchSize']).toEqual(5);
+    expect(batchProcessor['batchSize']).toEqual(100);


cc @felipecsl I had to change this because 5 is below the minimum allowed batch size

https://github.com/Eppo-exp/js-sdk-common/blob/b1048a32587a809bed351af07185592c9f774150/src/events/batch-event-processor.ts#L4-L13

const MIN_BATCH_SIZE = 100; const MAX_BATCH_SIZE = 10_000; export default class BatchEventProcessor { private readonly batchSize: number; constructor(private readonly eventQueue: NamedEventQueue<Event>, batchSize: number) { // clamp batch size between min and max this.batchSize = Math.max(MIN_BATCH_SIZE, Math.min(MAX_BATCH_SIZE, batchSize)); }

sameerank · 2024-12-16T22:16:46Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "@eppo/js-client-sdk",
-  "version": "3.8.3",
+  "version": "3.9.0-alpha.0",


⚠️ Do not publish this version for dogfooding until

The edge compute endpoint is published with updated hashing

The corresponding md5 hasher in js-client-sdk-common is updated https://github.com/Eppo-exp/js-sdk-common/pull/164/files#r1887291803 (.appendBinary --> .append)

And 4.7.0-alpha.1 version of js-client-sdk-common has been released and the dependency in this file has been updated to match (also see: https://github.com/Eppo-exp/js-sdk-common/pull/164/files#r1887650834)

sameerank · 2025-01-07T20:34:21Z

src/index.ts

+      contextAttributes ?? { numericAttributes: {}, categoricalAttributes: {} },
+    );
+
+    EppoPrecomputedJSClient.instance.setSubjectSaltAndPrecomputedFlagStore(


Based on these PR comments

Eppo-exp/js-sdk-common#164 (comment)
Eppo-exp/js-sdk-common#164 (comment)

I'll see if I can make this method private/not use it here and have these set through the constructor

…computed-client

src/index.ts

rasendubi · 2025-01-09T15:21:26Z

src/index.ts

+  const configurationWire: IConfigurationWire = JSON.parse(config.precomputedConfigurationWire);
+  if (!configurationWire.precomputed) {
+    applicationLogger.error('Invalid precomputed configuration wire');
+    return EppoPrecomputedJSClient.instance;
+  }
+  const { subjectKey, subjectAttributes, response } = configurationWire.precomputed;
+  const parsedResponse: IPrecomputedConfigurationResponse = JSON.parse(response);


Is there any reason all this parsing is done in client library and not common?

It's due to the approach taken. The same EppoPrecomputedClient is used for online (via precomputedInit) vs offline mode (via offlinePrecomputedInit)

The EppoPrecomputedClient's constructor knows which mode we're using based on the provided initialization options for the client instance. Both precomputedFlagStore and subject need to be accessible for initializing in the online mode: the user may want to select a specific storage option (e.g. chrome.storage) for their use case and subject is required to know what to precompute. We're reusing these same inputs for offline mode, hence the parsing.

I could add an option to pass in the configuration wire string to be parsed by the common library, which would overwrite the contents of precomputedFlagStore, but making subject optional or to overwrite it seems like doing a lot in EppoPrecomputedClient's constructor.

docs/js-client-sdk.iclientconfig.eventingestionconfig.md

src/index.ts

rasendubi · 2025-01-09T15:33:29Z

src/index.ts

+      subjectAttributes: subjectAttributes ?? ({} as Attributes),
+    };
+
+    EppoPrecomputedJSClient.instance = new EppoPrecomputedJSClient({


major: before potentially overwriting previous instance, we should shut it down. (and warn of re-initialization)

I think I'm unclear about what "shut it down" means here. The client is offline so polling/making requests isn't at play. Also the two instance variables being set by the constructor (subject and precomputedFlagStore) are private.

I could make these public so I can clear them here. Or I can add a public method to EppoPrecomputedClient in the common sdk to clear these. I can't do it in the constructor because that only has access to the new instance, not the one being replaced.

My attempt at a private shutdown function

function shutdownEppoPrecomputedClient() { if (EppoPrecomputedJSClient.instance) { EppoPrecomputedJSClient.instance.stopPolling(); EppoPrecomputedJSClient.initialized = false; applicationLogger.warn(`${loggerPrefix} Precomputed client is being re-initialized.`); } }

The configuration store is a private variable in EppoPrecomputedClient so I can't clear it here, nor does it come with any helpers to clear its contents. Also the store we use at the moment is memory-only, so I don't know if persisting across sessions is possible, e.g. if a page refresh is the reason for re-initializing

I think I'm unclear about what "shut it down" means here.

I mostly meant stopping polling. Most SDKs have a method for stopping all background activities.

The client is offline so polling/making requests isn't at play.

The fact that we're initializing offline doesn't mean that the previous client was in offline mode too

package.json

src/index.spec.ts

sameerank added 2 commits December 5, 2024 09:28

Create offlinePrecomputedInit method

ecf184d

Adjust config init format to have a precompute object

80340ca

sameerank commented Dec 9, 2024

View reviewed changes

sameerank added 3 commits December 9, 2024 00:11

v3.9.0

ce4bf21

Bump minor version

3fdd774

Test for offlinePrecomputedInit

2cbe3e5

sameerank commented Dec 9, 2024

View reviewed changes

sameerank marked this pull request as ready for review December 9, 2024 08:57

sameerank requested review from aarsilv and leoromanovsky December 9, 2024 08:57

aarsilv approved these changes Dec 9, 2024

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

aarsilv assigned leoromanovsky Dec 9, 2024

Fix tests

acbafaa

sameerank commented Dec 9, 2024

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

felipecsl reviewed Dec 9, 2024

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

sameerank added 2 commits December 10, 2024 15:53

Update common version

e3f35a5

Docs update

b0f73a7

leoromanovsky reviewed Dec 11, 2024

View reviewed changes

sameerank added 2 commits December 10, 2024 20:29

Merge branch 'main' into sameeran/ff-3649-offline-init-method-for-pre…

723728b

…computed-client

Update offline precomputed init to use the configuration wire format

90d725e

sameerank commented Dec 11, 2024

View reviewed changes

Remove unused IPrecompute import

c8c6a55

sameerank commented Dec 11, 2024

View reviewed changes

sameerank requested a review from typotter December 11, 2024 07:06

sameerank added 2 commits December 10, 2024 23:11

Add a todo comment

9216ca7

Delete ds store

8b3f8c4

typotter reviewed Dec 11, 2024

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

Merge branch 'main' into sameeran/ff-3649-offline-init-method-for-pre…

409019a

…computed-client

sameerank commented Dec 16, 2024

View reviewed changes

sameerank added 4 commits December 16, 2024 12:19

Change test data branch back to main

6f32a79

v3.9.0

9b4e404

v3.9.0-alpha.0

c2f0c78

Update lock file

ee3409f

sameerank commented Dec 16, 2024

View reviewed changes

sameerank assigned typotter and unassigned leoromanovsky Dec 16, 2024

rasendubi self-requested a review December 17, 2024 19:59

sameerank commented Jan 7, 2025

View reviewed changes

sameerank added 3 commits January 8, 2025 02:34

Merge branch 'main' into sameeran/ff-3649-offline-init-method-for-pre…

3410428

…computed-client

Update to use constructor instead of setters

3320634

Update js sdk common dependency

f31efd8

sameerank mentioned this pull request Jan 9, 2025

chore: confirm getPrecomputedAssignments functionality in node Eppo-exp/node-server-sdk#89

Merged

rasendubi reviewed Jan 9, 2025

View reviewed changes

aarsilv reviewed Jan 9, 2025

View reviewed changes

package.json Outdated Show resolved Hide resolved

src/index.spec.ts Outdated Show resolved Hide resolved

src/index.spec.ts Show resolved Hide resolved

sameerank added 11 commits January 10, 2025 10:15

Update common sdk dependency to 4.8.0-alpha.1

3f5a2bb

Comply with throwOnFailedInitialization

5f24951

Remove unneeded reset the static instance before each test

72c6ac3

Make documentation consistent with the interface

5c3712e

Remove unnecessary casting to Attribute

d71e589

Don't include logger prefix in thrown error

e094f8e

Add a shutdown function and test

27b413b

Remove comment

70a0667

v3.9.2-alpha.0

a2db546

Remove application logger input

a53962d

Fix test

8612f58

sameerank merged commit b55bfe2 into main Jan 11, 2025
8 checks passed

sameerank deleted the sameeran/ff-3649-offline-init-method-for-precomputed-client branch January 11, 2025 01:18

sameerank mentioned this pull request Jan 16, 2025

fix: getprecomputedinstance should always return an instance #151

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create offlinePrecomputedInit #117

feat: create offlinePrecomputedInit #117

sameerank commented Dec 5, 2024 •

edited

Loading

sameerank Dec 9, 2024

sameerank Dec 9, 2024

sameerank commented Dec 9, 2024

aarsilv left a comment

leoromanovsky left a comment

sameerank Dec 11, 2024

sameerank Dec 11, 2024

sameerank Dec 11, 2024

sameerank Dec 16, 2024 •

edited

Loading

sameerank Dec 16, 2024 •

edited

Loading

sameerank Jan 7, 2025

rasendubi Jan 9, 2025

sameerank Jan 10, 2025

rasendubi Jan 9, 2025

sameerank Jan 10, 2025

sameerank Jan 10, 2025 •

edited

Loading

rasendubi Jan 16, 2025 •

edited

Loading

feat: create offlinePrecomputedInit #117

feat: create offlinePrecomputedInit #117

Conversation

sameerank commented Dec 5, 2024 • 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

sameerank commented Dec 9, 2024

aarsilv left a comment

Choose a reason for hiding this comment

leoromanovsky 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

sameerank Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

sameerank Dec 16, 2024 • edited Loading

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 Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

rasendubi Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

sameerank commented Dec 5, 2024 •

edited

Loading

sameerank Dec 16, 2024 •

edited

Loading

sameerank Dec 16, 2024 •

edited

Loading

sameerank Jan 10, 2025 •

edited

Loading

rasendubi Jan 16, 2025 •

edited

Loading