RFC: Feature Flags Naming Conventions / Maintenance

dzucconi commented 1 year ago

Feature flags are permutation & complexity time bombs and, as such, there should be some urgency to get them out of the codebase. They have their purpose and we shouldn’t be afraid of using them — but I think they hang around the codebase and in Unleash for far too long.

 There’s currently no real process for maintaining them and due to the way we’ve implemented both creation and exposing them to the application, we have no way of knowing what’s active and in use or who created them in the first place.

Naming Convention

{team}_{feature-name}

Examples

grow_progressive-onboarding
fx_new-price-filter
tx_bank-account-balance-check

The prefix of the team makes it easy to understand who to talk to when interfacing with a flag in a codebase you’re working in.

Avoid when naming:

Language like enable, disable. Assume that the flag enables the feature.
Language like show, hide. Ditto.
Language like for-web. Include what systems this flag concerns in the description.
Dates. If dates are relevant then include that information in the flag's description.

Descriptions

Include a description of the functionality that the flag enables. Include additional metadata specifying the systems it targets, the team that owns it and the developer who created it.

The additional metadata here attempts to make maintenance easier, allowing for understanding which systems might be affected by the removal or modification of a flag.

Example

systems: force; owner: GROW; creator: Damon Zucconi

Maintenance

Proposal

When creating a feature flag, concurrently you should also create a ticket to remove it.

How is this RFC resolved?

Agree on naming convention and name all flags accordingly going forward
Create tickets for the removal of all other flags

damassi commented 1 year ago

Worth noting that retiring Echo is on the horizon within MoPlat, and who knows what future team might add unleash to an app without an announcement, so we should keep the app field in the name.

In tools.artsy.net, we can add some additional validation here, and update the messaging here.

rquartararo commented 1 year ago

Nice. Love the naming convention idea! On the TX team, we use Unleash flags in Force, but also Exchange, Pulse, and Volt. So our flag "private_sales_checkout" would need to remain active as it's being used in Volt and Exchange.

gkartalis commented 1 year ago

also in favor of this, I remember when we started using unleash also suggested something similar, so for me having the prefix of the team / repo makes total sense 👍

WillemdeKleijne commented 1 year ago

Good initiative! I'm a big fan of the 'avoid' section.

Regarding the maintenance / end-of-life of specific feature flags, I think creating a ticket is too free-form. More productized solutions for a/b-testing and rolling out features allow the user to set an 'expiry date'. With Unleash already highlighting which flags are active, could we agree on removing stale flags after a certain while?

dzucconi commented 1 year ago

What would the expiry date do though?

Unleash doesn't accurately report which flags are active because, due to the way it's integrated right now, we request all the flags.

damassi commented 1 year ago

@dzucconi - might be worthwhile to expand or do a follow-up on how we could revise Force's implementation so that we don't request everything on every request. Might be simple to fix.

joeyAghion commented 1 year ago

Most of this sounds reasonable. This bit runs contrary to my thinking:

Do not share flags across different apps. If you need a flag for the same feature in both Volt and Force, then create separate flags for each application.

Toggling platform-wide features on/off, segmenting consistently for split tests, or granting access based on other criteria (user, environment, subscription tier, etc.) was always the intent of this shared feature-toggling service. We tend to roll out features to one client at a time, but I'd rather our conventions not assume a toggle applies to one client.

dzucconi commented 1 year ago

@joeyAghion what about listing the relevant apps in the prefix: force-volt-exchange_tx_private-sales-checkout?

I suppose I find it surprising that a feature would have to be toggled across multiple apps but I see your point.

rquartararo commented 1 year ago

@dzucconi TX frequently works across multiple apps and we could separate them by app, but this would make it challenging to keep user id access consistent across all the flags. Adding all apps in the prefix as you suggested would be my preference.

kajatiger commented 1 year ago

I really like the idea and the proposal, except that the underscore-dash notation looks a bit weird, but I understand the sense behind it. I just wish there was a prettier way to write this. Depending on in which programming language we are writing the names I would prefer only underscore or something else.

MounirDhahri commented 1 year ago

Thanks for raising this @dzucconi . I totally agree with this. I took a look at all the CX available flags and we removed all code related to them.

About the flags naming, I don't have any strong preferences there, only suggestion I have is to ensure the naming by either checking the name added on Forque or even better generate the name using the UI. For example if you select, CX, Force, My Collection the feature flag created will be CX-Force-My-Collection

kathytafel commented 1 year ago

Agreed with most everything but also seconding the point that there can be a flag in multiple apps - and especially when it's to do with experiments you want the certitude of referring to the same set. In a previous life, we enabled PMs to also turn flags on and off. I guess with the new admin, it's easy enough to add these kinds of controls so you don't need to let them at unleash. I very much like the approach of create the ticket to remove as part of the completion. I was once at a talk where Uber talked about the amount of dead code behind unused feature flags and it's indeed a cognitive burden and they have a tool for that. I think given our size just good hygiene probably suffices.

dblandin commented 1 year ago

what about listing the relevant apps in the prefix

I like the team prefix. Seems like an immediate win to have each flag/toggle explicit owned by a product team and documented within the flag/toggle name.

Unleash also supports toggle descriptions and tags. I wonder if we could include the intended system consumers in the description (systems: force, gravity) or as individual tags (system:force, system:gravity)? That might also give some flexibility to add/remove a system as the use of a shared flag expands/contracts.

I'm all for documenting these things somewhere but I'm not yet sold on the constraint that they have be documented within the flag/toggle name itself if we have other mechanisms available.

dzucconi commented 1 year ago

Thanks to everyone for the feedback. I've updated the description incorporating it. I'm going to close this for now and:

I've already created tickets for the removal of every flag GROW is concerned with and removed and archived any inactive flags. I'd encourage you to do the same for your respective teams.
I'll open a PR in Force to the feature flagging doc covering these guidelines.
I'll assign myself a ticket to incorporate these guidelines in the Forque interface for creating flags.
I'll assign myself a ticket to incorporate descriptions in the flag table in Forque.

artsy / README