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

Describe advanced local feature flag configuration #326

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MGibson1
Copy link
Member

Objective

I had cause to add test flag differences between authenticated and unauthenticated contexts.

In a local system, this means needing to expand our basic JSON definition considerably to define segments, rules, and flags. This documents the basics of how that works and provides an example to work from should others need to do the same.

@MGibson1 MGibson1 requested a review from a team as a code owner March 20, 2024 16:57
@bitwarden-bot
Copy link

bitwarden-bot commented Mar 20, 2024

Logo
Checkmarx One – Scan Summary & Details8d321325-ca01-4988-884c-f5996aef2c06

No New Or Fixed Issues Found

Copy link

cloudflare-workers-and-pages bot commented Mar 20, 2024

Deploying contributing-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5d663fd
Status: ✅  Deploy successful!
Preview URL: https://6655c975.contributing-docs.pages.dev
Branch Preview URL: https://ps-include-context-aware-fea.contributing-docs.pages.dev

View logs

@MGibson1 MGibson1 force-pushed the ps/include-context-aware-feature-flag-example branch from 05adfb8 to c910e54 Compare March 20, 2024 17:00
Copy link
Contributor

@withinfocus withinfocus left a comment

Choose a reason for hiding this comment

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

Pretty advanced use case to show, but perhaps it will help advanced needs.

@@ -1,8 +1,3 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ Why open this up to everyone? We do not want to encourage usage of this except internally.

Copy link
Member Author

Choose a reason for hiding this comment

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

The metric I use for internal vs external is whether or not special access is required. The feature flags is in the AGPL licensed section of the codebase and not blocked by any employee-specific secrets.

I don't see why we wouldn't want external contributors feature flagging their contributions, if it makes sense and they are able.

Copy link
Contributor

Choose a reason for hiding this comment

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

They're not able, since we'd have to register flags in LD. If you think an engineer would connect with them to do that then alright, but it's a bit of a jump from today.

There's sometimes an eagerness to adopt features in the community and what's happened is users start trying to put these developer overrides in and have problems, therefore increasing support needs. I want the impetus to be on our internal engineering team to quickly remove these in a follow-up release instead.

<summary>Context-aware feature flag JSON</summary>
<div>
<div>
The `flags.json` file can also define flags which respond to user context. Currently, only `UserId`and the `OrganizationId` of all organizations a user belongs to are included in our feature flagging context. The syntax for defining context-aware flags amounts to defining a `flag` object which specifies `variations` values and `rules` which are evaluated against the user context. A `fallthrough` object is also available to specify a default variation. Rules are represented by `segments`, which are defined in the same file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The `flags.json` file can also define flags which respond to user context. Currently, only `UserId`and the `OrganizationId` of all organizations a user belongs to are included in our feature flagging context. The syntax for defining context-aware flags amounts to defining a `flag` object which specifies `variations` values and `rules` which are evaluated against the user context. A `fallthrough` object is also available to specify a default variation. Rules are represented by `segments`, which are defined in the same file.
The `flags.json` file can also define flags that respond to user context. Currently, only `UserId` and the `OrganizationId` of all organizations a user belongs to are included in our feature flagging context. The syntax for defining context-aware flags amounts to defining a `flag` object that specifies `variations` values and `rules` which are then evaluated against the user context. A `fallthrough` object is also available to specify a default variation. Rules are represented by `segments`, which are defined in the same file.

Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ This is also violating our line lengths since it's in custom HTML.

Copy link
Contributor

Choose a reason for hiding this comment

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

❌ We support an addition service account context as well that's omitted.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, we actually have three contextKinds now that I look closer. I'll expand to include descriptions of data on each

docs/contributing/feature-flags.md Outdated Show resolved Hide resolved
docs/contributing/feature-flags.md Outdated Show resolved Hide resolved
Co-authored-by: Matt Bishop <matt@withinfocus.com>
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.

3 participants