docs: add ADR 0010 to disable default polling for static-context providers#69
docs: add ADR 0010 to disable default polling for static-context providers#69jonathannorris wants to merge 3 commits intomainfrom
Conversation
… providers Replace default timer-based polling with trigger-based refresh (initialization, context change, app foreground, SSE push). Closes #68. Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant change to how static-context providers refresh feature flag evaluations. It proposes disabling default timer-based polling in favor of event-driven mechanisms, such as initialization, context changes, app foreground/page visibility, and Server-Sent Events (SSE) pushes. This aims to enhance sticky bucketing consistency, reduce resource consumption on client devices, and align the OpenFeature Protocol with common practices observed in leading vendor SDKs. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces ADR 0010, which proposes to disable default timer-based polling for static-context providers in favor of event-driven updates. This is a well-reasoned and detailed proposal that aligns with common industry practices. My review includes a few suggestions to improve the accuracy and completeness of the ADR: correcting a potentially broken link to another ADR, fixing an incorrect API name for watchOS foreground detection, and recommending that a more concrete migration strategy for the proposed breaking change be included directly in the ADR.
| ## Open Questions | ||
|
|
||
| 1. Should the ADR specify a recommended cooldown duration for foreground re-fetches, or leave this to implementations? | ||
| 2. How should providers handle the transition for existing applications that depend on the current default polling behavior? Should there be a deprecation period or migration guide? |
There was a problem hiding this comment.
This ADR introduces a significant breaking change for existing applications. While this is acknowledged as a negative consequence and an open question, leaving the migration strategy entirely open is risky. The ADR should be more prescriptive and propose a concrete migration path to guide implementers and users.
For example, the ADR could recommend:
- Introducing this change in a new major version of the providers.
- Providing a temporary configuration flag to opt-out of the new behavior and retain polling, to be deprecated in a future version.
- A detailed migration guide in the SDK documentation explaining the change and how to opt back into polling if needed.
Providing a clear strategy will significantly improve the adoption of this change and reduce friction for existing users.
|
|
||
| Static-context providers should not poll on a fixed timer by default. Instead, providers should re-fetch bulk evaluations based on event-driven triggers: | ||
|
|
||
| 1. **Initialization** — as defined in [ADR-0009](0009-localStorageForStaticContextProviders.md), providers fetch bulk evaluations during initialization (or load from cache and refresh in the background). |
There was a problem hiding this comment.
The link to ADR-0009 appears to use a different naming convention (camelCase) than other ADRs in this repository (kebab-case). This might result in a broken link. Please verify the filename and update for consistency.
For example, other ADRs are named like 0005-polling-for-bulk-evaluation-changes.md. The link to ADR-0009 points to 0009-localStorageForStaticContextProviders.md. It should probably be 0009-local-storage-for-static-context-providers.md.
| |----------|-----------| | ||
| | iOS / tvOS | `UIApplication.willEnterForegroundNotification` | | ||
| | macOS | `NSApplication.willBecomeActiveNotification` | | ||
| | watchOS | `WKExtension.applicationWillEnterForegroundNotification` | |
There was a problem hiding this comment.
The mechanism specified for watchOS foreground detection, WKExtension.applicationWillEnterForegroundNotification, appears to be incorrect. The standard way to handle this is by implementing the applicationWillEnterForeground() method of the WKExtensionDelegate. If a notification-based approach is preferred, it would typically be a custom notification posted from this delegate method. Please verify and correct the specified mechanism for watchOS.
| | watchOS | `WKExtension.applicationWillEnterForegroundNotification` | | |
| | watchOS | `WKExtensionDelegate.applicationWillEnterForeground()` | |
… ADR 0010 Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
1 participant
Summary
Motivation
Current OFREP static-context providers poll on a fixed timer by default (JS: 30s, Swift: 30s, Kotlin: 5min). This breaks sticky bucketing expectations and wastes resources on mobile. The majority of vendor SDKs (DevCycle, LaunchDarkly, Statsig, Eppo) do not use timer-based polling as the default client-side refresh mechanism.
See the vendor comparison table in the issue for details.
Notes
Related Issues
Closes #68