sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.11k stars 1.29k forks source link

Link to feature flag #29420

Open rrhyne opened 2 years ago

rrhyne commented 2 years ago

It is not possible to activate a feature flag for a particular unregistered user. This means the only method of testing feature flagged features is an A/B test. It would be helpful for user testing sessions if links could set a local variable that enables a particular feature flag.

vdavid commented 2 years ago

@rrhyne, questions:

rrhyne commented 2 years ago

@vdavid this handbook page will be helpful:

https://docs.sourcegraph.com/dev/how-to/use_feature_flags

Yes, I did mean local storage, so that the feature flag setting may persist to aid in multi-page situations and repeat visits. There is a local storage component that handles quite a few tasks.

Re available in production, your last point is interesting. This is probably a discussion for a wider group, but I could see us potentially not wanting to allow just anyone to turn on any given feature flag.

vdavid commented 2 years ago

Feature flags: Great, thanks for the link!

Local storage: Excellent, all clear.

Environment/visibility: I'll need a bit more guidance on this @rrhyne – I struggle a bit with finding the right audiences for my questions at this point. My main concern is that I may unnecessarily disturb others. So, what is the right channel for this question to reach the right "wider group"?

My guesses: a.) #dev-chat b.) #growth-and-integrations-internal c.) Something else

On my own, I'd probably go with a.) #dev-chat to make sure that I reach all the people with a security perspective. But I'm reluctant to send my message to a channel with 195 people without checking with someone first.

Also, I'm unsure who the final decision-maker is in this question. Am I right in supposing that after listening to opinions from the right people, I am free to decide whether to put this out publicly or restrict it?

erzhtor commented 2 years ago

@rrhyne @vdavid, I'm adding a similar feature in https://github.com/sourcegraph/sourcegraph/pull/29037, mainly for force testing feature.

Basically, the user will be able to override in DevTools as localStorage.setItem('getting-started-tour', true) which will take precedence over remote feature flag value.

Let me, please, know if this resolves this issue or not.

rrhyne commented 2 years ago

@erzhtor, your solution would fix part of the problem, but requiring the user to set a specific flag via the console will not unlock the ability to easily test through usertesting.com or similar solutions.

camdencheek commented 2 years ago

@vdavid Hi! Happy to answer questions on feature flag stuff. Sorry about the slow response -- busy day yesterday 😄

My main concern is that I may unnecessarily disturb others.

Don't worry too much about that. I've found people here are good at aggressively muting channels they are not interested in, and will gently redirect you if the message is put in the wrong place. No need to slow yourself down by worrying over which channel to post it in. If your questions are general, and don't belong to a specific team, #dev-chat is always a good bet. In this case, I'd probably be the one to respond in #dev-chat, so I'm happy to just answer questions directly here.

Also, I'm unsure who the final decision-maker is in this question. Am I right in supposing that after listening to opinions from the right people, I am free to decide whether to put this out publicly or restrict it?

Yes, you are right that the decision is ultimately yours. Keeping in line with our "High agency" value, the person owning the feature is responsible for:

We've got a pretty strong culture of trust around here, so you're trusted to make a decision if you're confident in the correct path forward, and you're also trusted to reach out for help if you're not confident. Hope that is helpful 🙂

camdencheek commented 2 years ago

Did you have any feature flag questions that weren't answered by that doc?

camdencheek commented 2 years ago

also, cc @limitedmage because you helped implement the client side code for feature flags, as well temporary user settings, which may be relevant here.

erzhtor commented 2 years ago

@rrhyne, I see, thanks for clarifying.

I think I can add a logic to auto-set localStorage through URL query parameters:

WDYT?

vdavid commented 2 years ago

@camdencheek, thanks! That answers many of my questions and soothes my concerns! :) One thing that's still unclear for me is the security aspect: I haven't checked the list of our current feature flags, but I suspect that letting anyone on the internet turn on/off any feature flag they discover is (or will be) a security concern. Any thoughts on this?

@erzhtor, I'm completely happy if you implement this as you're already there in the code. I'm pretty confident that I'll find some other meaningful issues on the Growth board to tinker with.

camdencheek commented 2 years ago

I suspect that letting anyone on the internet turn on/off any feature flag they discover is (or will be) a security concern

We should not be putting any security-sensitive code behind feature flags anyways. On the client side, overriding a feature flag was always possible with a debugger. Server-side, it required a site-admin to set a feature flag for the user, but this is more because it is incomplete than intentional. We eventually want to allow users to override their own feature flags, even if they are not site admin.

TL;DR, letting a user override their own feature flags for a session should be fine. However, we should probably document that feature flags should not be used to protect sensitive code/data.

erzhtor commented 2 years ago

I haven't checked the list of our current feature flags, but I suspect that letting anyone on the internet turn on/off any feature flag they discover is (or will be) a security concern. Any thoughts on this?

@vdavid, good question. Thanks for bringing this up. I don't think that this should be a security concern since the user:

  1. have to know the exact feature key
  2. Feature flags are either way to rollout (AB test) something gradually or the ability to turn on/off UI changes (when we talk about the web), and we should not rely on feature flags to hide some sensitive data.

But, happy to hear other thoughts!