docs: plan for provider.setConfiguration offline init (FFL-2666)#1326
docs: plan for provider.setConfiguration offline init (FFL-2666)#1326btthomas wants to merge 22 commits into
Conversation
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
aarsilv
left a comment
There was a problem hiding this comment.
Nice work! 💪 Digging what I'm seeing! I've attempted to answer your open questions. Feel free to get a head start on prototyping this out while people comment on your plan. 🐎
|
|
||
| ## Open items (non-blocking) | ||
|
|
||
| - Check with the flags team whether the `PrecomputedFlag`-style shape seen in the sample |
There was a problem hiding this comment.
The CDN currently returns the backend PrecomputedAssignments / PrecomputedFlag shape
There was a problem hiding this comment.
So is my inlined example above correct?
| - Check with the flags team whether the `PrecomputedFlag`-style shape seen in the sample | ||
| (`example.json`) is stable across environments/versions, vs the OpenFeature-aligned proposal | ||
| page (FFL-2687). | ||
| - Precomputed context-mismatch behavior: default value vs `PROVIDER_NOT_READY` vs error |
There was a problem hiding this comment.
If an error we should return default value but with a PROVIDER_NOT_READY code (@dd-oleksii can fact check me here)
There was a problem hiding this comment.
Above, you just said we should have a PROVIDER_ERROR on mismatch. Which is preferred?
| page (FFL-2687). | ||
| - Precomputed context-mismatch behavior: default value vs `PROVIDER_NOT_READY` vs error | ||
| (RFC open question). | ||
| - `setConfiguration` sync vs `Promise`-returning (JS-only work is synchronous, but a Promise |
There was a problem hiding this comment.
I don't see a huge downside to setConfiguration being asynchronous as I imagine being called infrequently and in a non blocking way.
There was a problem hiding this comment.
Our mobile customers are more focused on startup performance, correct? So that means we should make setConfiguration async, so they can do other app init things while our config is loaded, correct? Is that a good or bad assumption?
| - Precomputed context-mismatch behavior: default value vs `PROVIDER_NOT_READY` vs error | ||
| (RFC open question). | ||
| - `setConfiguration` sync vs `Promise`-returning (JS-only work is synchronous, but a Promise | ||
| keeps parity with `setEvaluationContext` and forward-compat for rules). |
There was a problem hiding this comment.
Side note, I am concerned about this being async due to race conditions. See: https://docs.google.com/document/d/12i0O__eHWWj1q5qSXID7VILiWxacHclLef-Y5oSea58/edit?disco=AAAB_VOdxLw
There was a problem hiding this comment.
That comment is related to dynamic (multi-user ruleset) when setContext passes in a new context. Does that concern apply here? Do we need to get it right in this stack of PRs or can it wait until we implement the dynamic context offline init?
| - **Obfuscation is not supported** in the DD precomputed format (the sample response carries | ||
| `obfuscated: false`) — parsing is plain JSON → object mapping (no key hashing, no | ||
| base64/salt decoding). |
There was a problem hiding this comment.
Future us! Since the current edge compute doesn't do this I think its fine to punt to sometime in the future (or never)
| Customers may call `setConfiguration` and `setEvaluationContext` in either order. The | ||
| `FlagsClient` holds the **loaded configuration** (carrying its embedded `context`) and the | ||
| **active evaluation context** independently. The servable `flagsCache` is only populated | ||
| when the two **match**; the match is re-validated on **both** calls. On mismatch, values are |
There was a problem hiding this comment.
On mismatch, values are
not served.
Should we instead return coded default value with a PROVIDER_ERROR code?
There was a problem hiding this comment.
errorCode: INVALID_CONTEXT is in the OpenFeature spec. That's the closest we have to "mismatch".
| configuration is present: record the context without invoking the native fetch and without | ||
| overwriting the config-populated cache. This is a JS-only change and lives in FFL-2688. | ||
|
|
||
| ## Work breakdown |
There was a problem hiding this comment.
Is plan to unleash agents for stacked PRs here? Note you can have agents use tools like git machete or jj--useful for propagating changes up the stack when iterating on lower down PRs.
There was a problem hiding this comment.
That is exactly my plan. A stack of PRs optimized for review while we can test the top of the stack end-to-end.
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 3a284a7 | Docs | Datadog PR Page | Give us feedback! |
| fetches + parses and returns a **serialized snapshot** (`Record<string, FlagCacheEntry>`). | ||
| 4. JS caches the snapshot in `flagsCache`. **All evaluation already happens in JS** against | ||
| that cache. |
There was a problem hiding this comment.
aside: while we're here, shall we rename FlagCacheEntry and flagsCache? (because it's not a cache and calling it cache is confusing)
I believe in other sdks we call it either "precomputed flag" or "flag configuration"
| - **Avoid the `FlagsConfiguration` name collision.** That identifier is already the `enable()` | ||
| options type (`packages/core/src/flags/types.ts`). Name the parsed-wire config type distinctly | ||
| — this plan uses **`ParsedFlagsConfiguration`** — so `configurationFromString` / | ||
| `setConfiguration` don't overload the existing type. |
There was a problem hiding this comment.
"Parsed" seems like a bad describer here — it's not something that we want to highlight. Also having both FlagsConfiguration and an unrelated ParsedFlagsConfiguration makes it way more confusing. I don't have a good suggestion though 😢 having FlagsConfiguration already used is unfortunate
There was a problem hiding this comment.
What should we do? FlagsConfiguration is already defined/exported as the parameter for the public enable() method.
| - Add a **`fetchPolicy`** (`ALWAYS` default / `NEVER` / `ON_MISMATCH`) set at `enable()` with a | ||
| per-`getClient()` override, so customers can turn off the fetch-on-`setEvaluationContext`. | ||
| Offline init needs `NEVER`; see [Fetch policy](#fetch-policy). |
There was a problem hiding this comment.
I'd start with ALWAYS and NEVER. "On mismatch" is not well-defined yet (e.g. we don't have a way to produce context-agnostic config). We might also add "poll" later.
Proposing to go with the minimal set of options for now because it's easy to add options in the future but it's hard to remove them
| - **Port, don't depend.** `openfeature-js-client` is not a dependency here (only upstream | ||
| `@openfeature/core` + `@openfeature/web-sdk` are). Port its small pure helpers — | ||
| `wire.ts` (`configurationFromString`/`configurationToString`) and `configMatchesContext` — | ||
| into RN core rather than depending on the package, whose provider/exposure-logging would | ||
| bypass RN's native RUM/exposure path. **But for the future rules (UFC) evaluator and any | ||
| obfuscation logic, prefer *depending* on a shared platform-agnostic core (e.g. | ||
| `@datadog/flagging-core`) rather than re-porting** — hand-porting an evolving evaluator risks | ||
| silent assignment drift. Keep the ported helpers behind a thin internal module boundary so a | ||
| later port → dependency swap stays contained. |
There was a problem hiding this comment.
Consider moving precomputed configuration types, configuration wire parsing/serializing functions, and precomputed "evaluator" into core
UPD: it looks like they are already there — anything that prevents us from reusing the core?
There was a problem hiding this comment.
anything that prevents us from reusing the core?
I think @sameerank is going to get everything into core this week, so we can import it for dynamic evaluation (the workstream after the current one).
| > ⚠️ **Two documented formats exist.** The Confluence *PrecomputedConfiguration format* | ||
| > page ([5141791092](https://datadoghq.atlassian.net/wiki/spaces/PANA/pages/5141791092/PrecomputedConfiguration+format)) | ||
| > describes an OpenFeature-aligned shape (`type` + `resolution.flagMetadata.experiment`). | ||
| > The sample CDN response and the shipped `openfeature-js-client` instead use the | ||
| > `PrecomputedFlag`-style shape above, which matches RN's `FlagCacheEntry`. **This plan | ||
| > assumes the `PrecomputedFlag`-style shape.** Since `variationValue` is the **typed** value, | ||
| > the decoder maps it to RN's `FlagCacheEntry.value` and derives the string | ||
| > `variationValue`/`variationType` that native Android exposure tracking expects. Still worth | ||
| > checking with the flags team whether the shape is stable across environments and versions. |
There was a problem hiding this comment.
Could you update the wiki to align it with actual returned type?
| In the offline flow the customer still calls `setEvaluationContext` (needed for context matching | ||
| and to hand a context to `trackEvaluation`), so under `fetchPolicy: NEVER` this path records the | ||
| context without invoking the native fetch and without overwriting the config-populated cache. |
There was a problem hiding this comment.
hm... I'm not familiar with RN implementation. Does native side have any state? if native setEvaluationContext returns a configuration, does it actually set anything?
There was a problem hiding this comment.
Native side does save state. I made a note that we should think about whether setConfiguration persists (or doesn't) to disk: 7b8af9c
|
|
||
| So the native read path only does *fetch → parse → return a snapshot*, and native exposure | ||
| tracking reconstructs what it needs from the per-flag data JS passes it. This means offline | ||
| init can be done **entirely in JS with no native changes**: parse a supplied configuration |
There was a problem hiding this comment.
I don't think this is correct. Not propagating the flags configuration to the native SDK level means that hybrid RN apps that want to fetch the flags config from the native SDK won't be able to, since we only set it at JS level.
This could be considered a non-blocker but it is definitely a gap that will need to be acknowledged and solved later down the line.
Also, if we also want to provide users with offline flags persistence we will still need to store this on disk, meaning that we will also need to communicate this to the native SDK.
I think we should keep this in mind going forward. The FFlags SDK on RN needs to be kept in sync with its native counterpart.
There was a problem hiding this comment.
Also, if we also want to provide users with offline flags persistence we will still need to store this on disk, meaning that we will also need to communicate this to the native SDK.
I disagree. When we say "Offline precomputed" we mean that the customer is supplying the precomputed variants to the SDK to initialize. In that case, we don't need to store it because the customer can just supply the same rules again, right?
The FFlags SDK on RN needs to be kept in sync with its native counterpart.
Why? What benefits do we get? Why not keep the RN SDK kept in sync with Browser? Seems easy since we can do them both in javascript and skip the native part.
There was a problem hiding this comment.
I disagree. When we say "Offline precomputed" we mean that the customer is supplying the precomputed variants to the SDK to initialize. In that case, we don't need to store it because the customer can just supply the same rules again, right?
For that use case yes, it's not necessary. But I assume that at some point a client will want to have a working offline mode that works out of the box, meaning that the last set of flags fetched from Datadog are persisted even if the app goes offline, without them having to worry of persisting those flags on disk on their own. (Growthbook SDK for RN provides this: https://docs.growthbook.io/lib/react-native#caching)
Why? What benefits do we get? Why not keep the RN SDK kept in sync with Browser? Seems easy since we can do them both in javascript and skip the native part.
Because as I mentioned earlier, doing this will break hybrid apps relying on the FFlags module. If the flags configuration is kept at JS level and not propagated to the underlying native SDK, any native code on that hybrid app that asks for the Flags configuration will get an empty or unsynced state.
Adds provider_set_configuration.md describing the plan to expose a JS setConfiguration path so a customer-supplied precomputed ConfigurationWire loads and evaluates exactly like CDN-fetched assignments. Planning only, no implementation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ConfigurationWire is version 1 (per updated spec and shipped wire.ts), and the envelope now reserves a server/rules branch alongside precomputed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Parser returns an empty config on invalid input or unknown version, matching the shipped wire.ts. Predictable failure surfaces at the provider layer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ancy Adopt the openfeature-js-client PrecomputedFlag shape (matches RN's FlagCacheEntry, decoder ~1:1) instead of the OpenFeature-aligned proposal page. Flags the two-format discrepancy to confirm with the flags team. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…epend openfeature-js-client is not a dependency here, so port its pure wire.ts and configMatchesContext helpers into RN core to preserve the native RUM/exposure path. Documents the context-agnostic matching nuance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Nothing here is confirmed or decided yet, so rename the Approach and Wire format headings to avoid implying finality. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A staging /precompute-assignments sample (example.json) shows the PrecomputedFlag-style shape (typed variationValue, RFC3339 createdAt, obfuscated:false), matching RN's FlagCacheEntry. Update the format block and notes to reference it and soften remaining certainty wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Explains that native exposure/RUM tracking is per-flag and parameter-driven (no dependency on a prior CDN fetch), so offline init stays in JS. Notes that FlagsClient.setEvaluationContext currently triggers a native CDN fetch and overwrites the cache, so it must branch when an offline config is loaded (FFL-2688). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a Non-goals section (fetchPrecomputedConfiguration, precomputeConfiguration, rules-based eval out of scope), note the fetch source could be the Datadog/Fastly edge CDN or a customer proxy, and specify context mismatch as PROVIDER_ERROR plus an INVALID_CONTEXT evaluation error code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
setEvaluationContext always fetches from the CDN today with no toggle. Add a fetchPolicy (ALWAYS default / NEVER / ON_MISMATCH) set at enable() with a per-getClient() override; build ALWAYS + NEVER now and defer ON_MISMATCH. Tracked as FFL-2718. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Group the seven sub-tasks into four stacked, self-contained PRs (unit tests co-located, no trailing test-only PR) and document the ordering rationale. FFL-2691 is repurposed to RUM FIT integration/e2e coverage (offline-loaded flag -> RUM parity). Jira encodes the order via Blocks links. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The parsed-wire config type must not reuse FlagsConfiguration, which already names the enable() options type. Use ParsedFlagsConfiguration for the configurationFromString/setConfiguration surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a configuration-kind/evaluation-mode concept chosen by which wire branch is populated (precomputed vs future server/rules), keep fetchPolicy as the network axis only, and gate the context match/mismatch-error on the precomputed kind so a future rules config (context-agnostic) is not rejected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The decoder must read attributes.obfuscated and fail predictably (unsupported -> PROVIDER_ERROR) instead of mis-mapping hashed keys as flag names if the CDN ever enables obfuscation. This is the seam for a future obfuscated format. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ator (M6, M5) Pass fetchPolicy inside an options object so it can grow (ttl, staleWhileRevalidate) without an API break, and record that the future rules (UFC) evaluator should be depended on from a shared core rather than re-ported, to avoid silent assignment drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t-fixes) Capture the plan-review silent-failure risks as a per-subtask acceptance checklist: variationValue stringification, wire-context normalization before matching, NEVER sets context synchronously, provider event ordering, and the iOS int/double parity note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an "Executing a step" section: what to read (this doc, the RFCs, the Jira ticket + acceptance checklist), where to ground in code/formats, the required shape of the per-step implementation plan (explicit files, signatures, return types, tests), the guardrails, and a reusable prompt. Lets a fresh-context agent execute any PR step consistently. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Name each step branch blake.thomas/FFL-2666-PR{N}, based on the previous
step's branch. PR1 branches off this planning branch so every step inherits
the plan. Add a Branch column to the delivery table.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Oleksii Shmalko <oleksii.shmalko@datadoghq.com>
A config loaded via setConfiguration is in-memory only today. Capture as an open question whether to offer a persist option for offline last-known-values across cold starts (wire vs parsed, JS-side store vs native flagsDataStore, auto-load + fetchPolicy interaction, staleness). Likely a fast-follow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The wire variationType union now includes integer and float, so widen the FFL-2687 validation to boolean|string|number|integer|float|object and note that integer/float map to a JS number for value while the original variationType string is preserved for Android's exposure round-trip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deliver never-fetch as a dedicated offline OpenFeature provider rather than a fetchPolicy flag. Its initialize/onContextChange use FlagsClient.setEvaluationContextWithoutFetching and never hit the CDN. Simpler core, no online/offline mixing, all-JS (no native changes), and the same provider extends to future offline dynamic rules. Collapse the delivery to three stacked PRs (PR2 = core offline API incl. the no-fetch method, PR3 = OfflineProvider + exports) and update the work breakdown and acceptance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
340691b to
3a284a7
Compare
Offline feature flags — related PRs
FlagsClient.setConfiguration): feat(flags): FlagsClient.setConfiguration + context matching [PR2] (FFL-2688) #1331Summary
Adds
provider_set_configuration.md, the design doc for offline feature-flag initialization in the React Native SDK: letting a customer load a flag configuration they fetched themselves and evaluate it exactly as if it came from the edge CDN, viaand a dedicated offline OpenFeature provider that never touches the network. Documentation only — no code.
It covers the scope (precomputed, static single-context) and non-goals, the
ConfigurationWire/precomputed formats, context matching, the offline provider, and how the work breaks down. See the doc for detail.