Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VPN-5855 - Part 2 - Update TaskGetFeatureList to use new Guardian endpoint and scheduling #8956

Merged
merged 11 commits into from
May 6, 2024

Conversation

brizental
Copy link
Contributor

The /featurelist Guardian enpoint is being update to also return experimental features from Nimbus, see https://github.com/mozilla-services/guardian-website/pull/1688.

This pull request updates TaskGetFeatureList to use the new endpoint. The new enpoint

  1. is a POST endpoint,
  2. expects either an Authorization header from which to derive the experimenter id or the experimenter id to be in the body of the request

This pull request also updates the scheduling of the calls to this endpoint. Previously this endpoint would only be called once the user was logged in, on a timer. Now it should be called

  1. As soon as possible when the application is started
  2. Regardless of being logged in or not
  3. Periodically
  4. Whenever the experimenter id changes

About the experimenter id

The experimenter id is used by Nimbus to calculate the experimental features that should be enable and with which parameters.

As described in https://github.com/mozilla-mobile/mozilla-vpn-client/blob/main/docs/rfcs/0006-cirrus-experiments.md, we will use the FxA id as the experimenter id. However, if the user is logged out we will generate a random UUID and use that instead.

Therefore, whenever the user logged in state changes TaskGetFeatureList needs to be re-run with a different experimenter id. Note that the way this is implemented, the logged out experimenter id is only calculated once i.e. it doesn't get recalculated everytime the user logs out. As I write this I realize we should probably rotate it as well when telemetry is disabled -- will send a commit with that.

@brizental brizental changed the title VPN-5855 - Update TaskGetFeatureList to use new Guardian endpoint and scheduling VPN-5855 - Part 2 - Update TaskGetFeatureList to use new Guardian endpoint and scheduling Jan 15, 2024
@brizental brizental force-pushed the 5855-get-experiments branch from 0be6513 to 2deb52e Compare January 17, 2024 10:17
src/taskscheduler.h Show resolved Hide resolved
src/app.cpp Show resolved Hide resolved
src/feature/taskgetfeaturelistworker.h Show resolved Hide resolved
src/networkrequest.h Show resolved Hide resolved
Copy link
Contributor

@vinocher vinocher left a comment

Choose a reason for hiding this comment

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

Looks good!

@brizental brizental force-pushed the 5855-get-experiments branch from 6193930 to e9ebe70 Compare January 25, 2024 15:08
@brizental
Copy link
Contributor Author

Merging this is blocked until https://github.com/mozilla-services/guardian-website/pull/1688 is merged and released to staging.

@brizental brizental added the ⛔️ do not merge ⛔️ This PR should not be included in the current/next release label Jan 25, 2024
@brizental brizental force-pushed the 5855-get-experiments branch 2 times, most recently from 1f25f30 to ed37b95 Compare February 5, 2024 15:29
@brizental brizental removed the ⛔️ do not merge ⛔️ This PR should not be included in the current/next release label Feb 5, 2024
@brizental brizental enabled auto-merge (squash) February 5, 2024 15:31
@github-actions github-actions bot added the 🛬 Landing This PR is marked as "auto-merge" label Feb 5, 2024
@brizental brizental force-pushed the 5855-get-experiments branch from ed37b95 to 3fff341 Compare May 2, 2024 15:04
@brizental brizental disabled auto-merge May 3, 2024 08:11
@brizental brizental force-pushed the 5855-get-experiments branch from c412078 to 85a66e5 Compare May 3, 2024 08:13
@brizental brizental enabled auto-merge (squash) May 3, 2024 08:50
@brizental brizental merged commit d5fcb1a into main May 6, 2024
120 of 121 checks passed
@brizental brizental deleted the 5855-get-experiments branch May 6, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛬 Landing This PR is marked as "auto-merge"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants