feat(flags): roll new flags to 1% of cloud customers #30905
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements a hash-based rollout for PreviewFlagsV2, enabling controlled exposure by rolling out new flags to 1% of cloud customers.
- Updated
/frontend/src/loadPostHogJS.tsxto include a newPREVIEW_FLAGS_V2_CONFIGfor toggling preview flags. - Introduced a stable hash-based rollout function in
/frontend/src/lib/utils.tsxthat respects inclusion/exclusion sets. - Added tests in
/frontend/src/lib/utils.test.tsto confirm behavior under various rollout scenarios, including percentage-based rollouts and explicit overrides.
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
frontend/src/lib/utils.tsx
Outdated
| excludedHashes?: Set<string> | ||
| } | ||
|
|
||
| function hashString(str: string): string { |
There was a problem hiding this comment.
Just out of curiosity, why did you hand roll a hash function? The purpose was to hide the api keys. I’m not familiar enough with your implementation to understand if it’s one-way secure.
There was a problem hiding this comment.
I wanted to do something simple; but you're right, I should pick a tried-and-true implementation. I've updated it.
There was a problem hiding this comment.
Ha! I'm allergic to hand-rolled crypto. 😛
|
Size Change: +1.54 MB (+15.14%) Total Size: 11.7 MB
|
| @@ -25,6 +26,11 @@ const configWithSentry = (config: Partial<PostHogConfig>): Partial<PostHogConfig | |||
|
|
|||
| export function loadPostHogJS(): void { | |||
| if (window.JS_POSTHOG_API_KEY) { | |||
| const PREVIEW_FLAGS_V2_CONFIG = { | |||
| rolloutPercentage: 1, | |||
| includedHashes: new Set(['593cb24f9928bab39ec383c06c908481880d5099']), | |||
There was a problem hiding this comment.
thinking about this more... I almost feel like I should pass this value in as a list of environment variables or something, feels like a better idea than committing hashed tokens (even if they are one-way secure).
@haacked what do you think? I could go either way – environment variables is more work bc more plumbing but arguably more secure.
There was a problem hiding this comment.
Yeah, it makes it easier if we have to back out, right?
There was a problem hiding this comment.
yup you were right the whole time; I was just tired when I implemented this. After I got home and thought about it and read your comment I was like "huh I'm being an idiot"
There was a problem hiding this comment.
You're too hard on yourself. 😆
Problem
we're scaling the new
/flagsservice and we want to do it intentionally. For starters, I want to start rolling out traffic to our customers on theus.posthog.comcloud instance – this way, if there's funky behavior, it just affects our customers, not our customers' customers.Changes
The way I'm doing this is to basically hash the feature flag keys and then compare those values with a rollout percentage. This way we can guarantee that the same API keys see the same rollout value without hard-coding anything.
This rollout function also takes in Sets that can contain hashed values to explicitly include or exclude, which override the rollout percentage. The idea here is that if want to roll specific customers on or off, we can easily do that. I've done it for our public API key, for example (this isn't just an example though, I want to turn this on for us)
Does this work well for both Cloud and self-hosted?
should work the same for both
How did you test this code?
wrote tests for hashing function, and then