chore: add a requirement to have an optional init option in providers…#74
Merged
jonathannorris merged 1 commit intoopen-feature:cursor/offline-context-local-storage-4fb5from Apr 13, 2026
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the ADR for local storage in static-context providers to include an optional 'persisted-cache key prefix' configuration. This change aims to prevent storage key collisions when multiple provider instances share the same storage partition, such as a single web origin. The feedback focuses on improving the clarity and logical consistency of the document, specifically by refining the description of how the prefix is used during lookup and ensuring it is correctly categorized as a configuration option rather than a static-context input. Additionally, it was noted that the implementation guidelines should be updated to reflect this new requirement.
| During initialization, a provider should follow a cache-first approach: | ||
|
|
||
| 1. Attempt to load a matching persisted bulk evaluation from local storage (matching `cacheKeyHash`). | ||
| 1. Attempt to load a matching persisted bulk evaluation from local storage (matching `cacheKeyHash`, and the same persisted-cache key prefix the instance was configured with, if any). |
Contributor
There was a problem hiding this comment.
The phrasing "matching ... (matching ...)" is redundant. Additionally, since the prefix is used to construct the storage key (as defined in the Decision section), it is more accurate to describe it as being used for the lookup rather than being "matched" against the loaded content.
Suggested improvement:
- Attempt to load a persisted bulk evaluation from local storage using the configured persisted-cache key prefix (if any) and the
cacheKeyHash.
|
|
||
| Providers should allow applications to disable the default persistence behavior, for example with a `disableLocalCache` option, or replace the storage backend when platform requirements or policy constraints require it. | ||
|
|
||
| When applications configure **more than one** static-context provider against the same underlying storage (same browser origin, shared app container, and so on), providers should expose an optional **persisted-cache key prefix** (name may vary by SDK, for example `persistedCacheKeyPrefix` or `localCacheKeyPrefix`). Applications set a distinct prefix per provider instance so persisted entries are namespaced and instances do not load or overwrite each other’s bulk evaluations. |
Contributor
There was a problem hiding this comment.
Since this ADR now requires providers to expose a persisted-cache key prefix option, the guideline/static-context-provider.md file should be updated to include this in its "Configuration" section to maintain consistency between the decision and the implementation guidelines.
toddbaert
approved these changes
Apr 10, 2026
Member
toddbaert
left a comment
toddbaert
left a comment
There was a problem hiding this comment.
Approved with a nit, up to you if you want to change it.
You will need to do a git commit --amend --signoff && git push -f to fix the DCO issue.
jsalaber
force-pushed
the
cursor/offline-context-local-storage-4fb5
branch
from
54fc225 to
ce09511
Compare
… for prefixing the cache key Signed-off-by: Salaber <jason.salaber@dynatrace.com>
jsalaber
force-pushed
the
cursor/offline-context-local-storage-4fb5
branch
from
ce09511 to
e7e72cc
Compare
jonathannorris
approved these changes
Apr 13, 2026
jonathannorris
merged commit 1aaba80
into
open-feature:cursor/offline-context-local-storage-4fb5
3 checks passed
jonathannorris
pushed a commit
that referenced
this pull request
Apr 13, 2026
… for prefixing the cache key (#74) Signed-off-by: Jason Salaber <jason.salaber@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
jonathannorris
pushed a commit
that referenced
this pull request
Apr 13, 2026
… for prefixing the cache key (#74) Signed-off-by: Jason Salaber <jason.salaber@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
jonathannorris
added a commit
that referenced
this pull request
Apr 23, 2026
) * Add ADR for offline local storage cache Co-authored-by: Jonathan Norris <jonathannorris@users.noreply.github.com> Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * Renumber local storage ADR to 0009 Co-authored-by: Jonathan Norris <jonathannorris@users.noreply.github.com> Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): refine static-context local persistence proposal Clarify ADR 0009 with provider behavior, persistence examples, and implementation guidance for local cached bulk evaluations. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): clarify fallback semantics in ADR 0009 Clarify initialization flow, explain the persisted timestamp, and define temporary server failures as eligible for persisted fallback. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): clarify cache key guidance in ADR 0009 Specify the cacheKeyHash formula and restore explicit open questions for reviewer feedback. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): add disableLocalCache option to ADR 0009 Document an explicit provider option for turning off persisted local storage. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): simplify cache key to hash(targetingKey) in ADR 0009 Drop authToken from cache key derivation and replace sha256 with generic hash(), per reviewer feedback. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): add CACHED evaluation reason and remove resolved open question Specify CACHED as the evaluation reason when serving from persisted storage. Remove fallback scope open question since the decision section already addresses it. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): strengthen fallback language to must not in ADR 0009 Use must not for auth/config error fallback to prevent masking real problems. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): remove platform constraint from negative consequences Local storage availability is a platform constraint, not a consequence of the proposal. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): make security/privacy consequence more concrete Specify that flag values are stored in plaintext and accessible to same-origin code or compromised devices. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): remove storage model implementation details from ADR 0009 The specific storage key and record model are implementation details. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): clean up implementation notes and mermaid diagram Remove redundant implementation notes that overlap with the decision section. Simplify mermaid diagram initialize call to use context. Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): rewrite ADR 0009 for cache-first initialization Replace fallback-on-failure with cache-first initialization pattern aligned with vendor SDKs (LaunchDarkly, Statsig, DevCycle, Eppo). Provider loads from persisted cache immediately on startup, refreshes from network in background, and emits PROVIDER_CONFIGURATION_CHANGED when fresh values arrive. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): add cache TTL as open question in ADR 0009 Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): improve precision of provider lifecycle semantics in ADR 0009 Fix PROVIDER_FATAL to PROVIDER_ERROR with fatal error code per spec. Add rationale for READY vs STALE on cache-hit startup. Clarify cache key tradeoff (targetingKey vs full context). Note existing provider implementations will need lifecycle refactors. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): clear persisted cache on auth/config errors in ADR 0009 On the cache-hit path, if the background refresh fails with 401/403/400, the provider continues serving cached values for the current session but clears the persisted entry. This ensures the next cold start uses the cache-miss path, making auth errors immediately visible. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): clarify background refresh cancellation and first cold start behavior Providers should cancel in-flight background refresh when onContextChanged() is called. Document that cache-first only applies after the first successful evaluation is persisted. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): add cache key namespace as open question in ADR 0009 Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): refine cache namespace open question to use auth token hash Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * Revert "refactor: make endpoint.origin optional, default to OFREP base URL" This reverts commit b69eafd. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * chore: add a requirement to have an optional init option in providers for prefixing the cache key (#74) Signed-off-by: Jason Salaber <jason.salaber@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): keep persisted cache on auth errors, rely on TTL for expiry Don't clear the persisted cache on 401/403/400 errors. The cache TTL is responsible for eventual expiry. This avoids degrading subsequent cold starts to defaults while auth errors are investigated. Aligns with vendor SDK behavior (DevCycle keeps cache through auth errors with a 30-day TTL). Moved TTL from open question to implementation recommendation. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): recommend cacheKeyPrefix for multi-provider namespacing Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): implement cacheKeyPrefix for multi-provider namespacing Move cacheKeyPrefix from open question to decision. Providers support an optional cacheKeyPrefix config option; when set, the cache key becomes hash(cacheKeyPrefix + targetingKey). Standardize terminology across the ADR. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): restore cacheKeyPrefix open question with answer Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): add version field to example and rename file to kebab-case Add version field to persisted entry example JSON for schema versioning. Rename ADR file from camelCase to kebab-case to match convention. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * chore: reset non-ADR-0009 files to main Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): use delimiter in cacheKeyPrefix hash to prevent collisions Use hash(cacheKeyPrefix + ":" + targetingKey) to avoid ambiguous concatenation where different prefix/key pairs produce the same hash. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): add implementation note for storage write failures If persisting fails, continue with fresh in-memory values. The old persisted entry remains for the next cold start. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): introduce cacheMode option with network-first mode for SPAs Replace the disableLocalCache boolean with a three-value cacheMode enum: cache-first (default), network-first, and disabled. network-first awaits the initial network request and falls back to cached values only on network errors or 5xx responses, which fits SPA use cases that block rendering on flag evaluation. Document timeoutMs tuning guidance for network-first, add alternatives considered, and update initialization flow and mermaid diagram notes. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): rename cache-first to local-cache-first for clarity Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): simplify network-first description to reference existing behavior Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): explain network-first as existing behavior plus offline fallback Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): add section title and description before initialization diagram Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): clarify TTL-expired entries must not be served Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): split initialization sequence diagram into three smaller diagrams Replace the single nested-alt diagram with three focused ones: cache hit with background refresh, cache miss, and the normal polling cycle. Addresses review feedback that nested alts were hard to read. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): reframe cache clearing as implementation detail, focus on TTL invariant Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> * docs(adr): require both logging and PROVIDER_ERROR emit on auth/config errors Per review feedback, the cache-hit path should both log the error and emit a PROVIDER_ERROR event when a background refresh fails with auth or config errors. Updates the decision text, cache matching section, and the sequence diagram. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> --------- Signed-off-by: Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com> Signed-off-by: Jason Salaber <jason.salaber@dynatrace.com> Co-authored-by: Jonathan Norris <jonathannorris@users.noreply.github.com> Co-authored-by: Jason Salaber <jason.salaber@dynatrace.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR