Skip to content

Conversation

leoromanovsky
Copy link
Member

@leoromanovsky leoromanovsky commented May 4, 2024

… (FF-2048)


labels: mergeable

Fixes: #issue

Motivation and Context

Customers with very short sessions (for example, chrome extensions) experience many more CDN requests than those with longer sessions. It is their interest to have more control over the rate of refreshes and balance that against being tolerant against stale flags.

For example, an owner of a chrome extension may know that during the lifetime of a financial transaction on a website where the extension is used repeatedly, a cool down period of 2 minutes will allow for a CDN request on the first open of the chrome extension to get the latest flags, while pausing CDN requests for subsequent requests in that period whenever the extension is re-opened.

We will provide an isExpired API to the ConfigurationStore and only perform CDN requests when the store declares itself as expired.

The default case will likely be that isExpired returns true always and the SDK behavior will be unchanged.

Upstream clients will implement isExpired using the available APIs on their platforms and the desired policies. Paired with these changes will be support for a cooldown period which will be a user provided SDK init parameter that governs how often a client should be stale, even when the poller executes its callback.

chrome extension

class ChromeStorage {

  constructor(private cooldownSeconds: number) {}

  setEntries(..) {
  
     await chrome.storage.local.set('lastUpdated', new Date())
  }

  isExpired():Promise<boolean> {
    await lastUpdated = chrome.storage.local.get('lastUpdated')

    const currentTime = new Date();
    const timeSinceLastUpdate = currentTime.getTime() - lastUpdated.getTime();
    const cooldownMilliseconds = cooldownPeriodSeconds * 1000; // Convert cooldown to milliseconds

    return timeSinceLastUpdate >= cooldownMilliseconds;
  }

}

The other classes will implement the functions in similar ways, or choose to return whatever else is appropriate.

Description

Upstream clients will implement isExpired using the available APIs on their platforms.

How has this been tested?

New unit tests

Questions for reviewers

  1. There are several ways to "block" CDN requests and all of them have trade-offs

Ultimately the "cooldown period" upstream clients will implement is not exact.

This approach continues running the poller and it acts as the clock - cool down periods significantly shorter than the period of the poller don't make much sense. However they will still be useful for stopping the initial request from happening during repeated short sessions - such as with the use case of the chrome extension.

  1. No new SDK parameters are added to commons

The cooldown period will be implemented in the JS SDK for the configuration stores that need it.

  1. Does the word isExpired convey the correct meaning?

It's important to get it right so users who implement their own persistent store have an understanding of its role in the SDK's operation.

@leoromanovsky leoromanovsky marked this pull request as ready for review May 4, 2024 04:05
Copy link
Contributor

@schmit schmit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, I like your choice of providing this as an API.

On naming, I think isExpired is clear and best, though perhaps hasExpired has some merits too.

@schmit
Copy link
Contributor

schmit commented May 9, 2024

Btw don't forget to bump the version, I guess 3.1.0 is appropriate given the new functionality

@leoromanovsky leoromanovsky merged commit 17a33c7 into main May 9, 2024
@leoromanovsky leoromanovsky deleted the lr/ff-2048/cooldown branch May 9, 2024 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants