That means that a different proxy instance has to run for each environment. We're using review environments like dev/123 (where 123 would be the MR number) and it's not feasible to spin up a new proxy instance for each of those.
Looking at the upstream Unleash Proxy docs, the Proxy API actually supports passing the environment as part of the context among other properties:
field name
type
lifetime
description
appName
string
static
the name of the application
environment
string
static
the environment the app is running in
userId
string
dynamic
an identifier for the current user
sessionId
string
dynamic
an identifier for the current session
remoteAddress
string
dynamic
an identifier for the current session
properties
Map<string, string>
dynamic
a key-value store of any data you want
However, the environment parameter is completely ignored in the request. Is there a reason that the proxy works differently when connected to GitLab? Am I missing something?
Sadly it looks like due to this limitation the proxy and therefore the entire FF feature isn't usable for us
Proposal
We introduce a new toggle in the Unleash Client Configuration modal. This b switches the behavior to use environment parameter for selecting feature flags instead of appName parameter.
Users who expect the environment param to be respected in GitLab Unleash API can enable the toggle. This toggle is enabled by default in all newly created projects. Existing projects have it disabled, but can enable it manually. In this case, we should show a caution that users should make sure the environment parameter is correctly set in the unleash client or proxy beforehand.
Technical details
We add use_unleash_environment column (boolean DEFAULT true NOT NULL) to operations_feature_flags_clients table.
Switch the flag filtering mechanism between toggle ON/OFF.
This has been implemented completely incorrectly. If I make our proxy use the default appName ('unleash-proxy'), setting any environment in a feature flag other than 'unleash-proxy' will make the flag return false.
We were very excited to setup Unleash w/ GitLab but given this issue others we have no way forward. Hoping that this can get worked on soon (I'll do it myself if someone can point out the right files).
Similar scenario here with one suggestion: if the environment field is currently being ignored by GitLab, could using that field as an override for appName be a path forward without breaking backward compatibility?
To add specifics: if a request comes in with only an appName value, then that is the environment (current behavior). However, if a request comes in with an appName and environment, the environment value takes precedence (new, slightly more compatible behavior).
I confirm that all Gitlab Feature Flag states/statuses are solely dependent upon the Unleash Docker Proxy UNLEASH_APP_NAME variable and therefore renders any use of the unleash-react-client variable unleash-appname useless.
The GitLab Unleash API uses the UNLEASH_APPNAME and UNLEASH_INSTANCEID HTTP headers to authenticate and to filter the flags by environment. The Proxy seems to provide this information on behalf of the client, and this is set at the time the proxy is started via environment variables/config. This means that the client can't use the same proxy to request other environment's flags, as this value is being sent to GitLab's Unleash APIs via the Proxy, and this is a static value.
My concern is this: [...] if the unleash-react-client variable isn't the variable being recognized by the Gitlab Feature Flag API, then independent Unleash Docker Proxies have to be instantiated with the proper UNLEASH_APP_NAME variable with then turns into a DevOps maintenance problem.
One concern is that this would be a breaking change for clients that still relies on the HTTP_UNLEASH_APPNAME. If they haven't configured environment parameter yet, it could fall back to default e.g. https://github.com/Unleash/unleash-client-ruby#list-of-arguments.
We should give heads-up to users in advance and gradually rollout the change. It's also reasonable to give a deprecation warning now and do the switching in the next GitLab major update %16.0.
Is there any way we can avoid the breaking change?
It's worth exploring if there is any way to avoid breaking change, like adding project-level setting to make the new behavior opt-in. @afontaine Do you have any options on this?
@vshushlin To be clear, overriding unleash_app_name by unleash_environment is the main breaking change. If a user hasn't configured these params correctly in client SDK, they would start seeing different feature flags in their apps. For instance:
Client SDK:
unleash_app_name: production
unleash_environment: default
GitLab Unleash API:
Before: Returns feature flags scoped by production environment name.
After: Returns feature flags scoped by default environment name.
"Supporting both approaches => dropping legacy in the future" is the second breaking change, which could happen in 17.0.
@cbalane I know we aren't actively working on Category:Feature Flags, but I think we should address this sooner rather than in a year. (introducing support for HTTP_UNLEASH_ENVIRONMENT somewhen in early 16.*, and then the breaking change in 17.0 sounds fine to me)
@cbalane I have a premium customer with over 80 users that is currently experiencing difficulties with this issue. They would like to use GitLab for their Feature Flags and informed us that was a reason they purchased GitLab to begin with. They've been struggling with this issue for months, and might look to a 3rd party to resolve it. Is there an updated ETC for the FF to be completed and released to customers?
This issue describes a problem to solve, and it's confirmed.
This issue describes a proposal that outlines a solution for the problem to solve.
This issue requires input from the Product Designer to determine a UX proposal.
This issue includes a UX proposal that is up-to-date and has been validated.
This issue requires a technical proposal (e.g. link to the code to be changed), and it's added and up-to-date.
[-] This issue could affect application performance, and the concern is explained in the issue description.
[-] This issue could affect application security, and the concern is explained in the issue description.
This issue is the smallest iteration possible and doesn't require further break down.
This issue is linked to related, blocking and blocked issues, and it's up-to-date.
This issue is labeled correctly.
This issue requires PoC, prototype or Technical Evaluation due to its high complexity, and it's already finished.
This issue describes a list of tasks or expected merge requests, therefore the weight is set and needs weight label is removed.
If both backend and frontend work are required, add ~backend-weight::* and ~frontend-weight::* labels as well.
@shinya.maeda let's make sure it's a toggle and not a checkbox, and we should probably do a phased default off -> default on -> on, disabled -> removed flow for the rollout.
It also seems the appName can be passed in during client creation. I wonder if 1) that works, and 2) we can update our docs before embarking on this bigger refactor?
@afontaine Thanks. I've updated the issue description to use Toggle instead.
and we should probably do a phased default off -> default on -> on, disabled -> removed flow for the rollout.
I'm not entirely sure what you mean by that, but the intent here is that we introduce an option to switch the behavior. We don't intend to finish the migration within the same milestone. We should create a follow-up issue to force-enable the switch in entire projects as a breaking change in 16.0 or 17.0.
It also seems the appName can be passed in during client creation. I wonder if 1) that works, and 2) we can update our docs before embarking on this bigger refactor?
Yes, this is a current workaround. However, I think users still are expecting environment to be respected instead of appName in general. I also feel it's semantically correct. Is there any reason that we should stick with appName still?
@shinya.maeda I suppose not, but I'm glad the workaround exists.
I had thought there was an activation strategy tied to environments as well, but this is not the case. We're free to proceed without worrying too much about clients
@shinya.maeda@afontaine - This proposal makes sense, I've gone ahead and mocked up what this could look like with a toggle on by default, but I would love to get technical writing's eyes on it as there are quite a few warnings on this modal (@rdickenson or @phillipwells let me know who should be taking on this review with the recent org change) which we might want to take a look at with this change:
As a side note, this last input box with the content above is pretty confusing:
I think we should have the text relating to it as part of the title text to the input box as shown in my mock above. It took me a while to realize these two things were related.
@emilybauman Thanks for the ping! I'm happy to pair with @rdickenson on some of the copy during the transition—it would certainly help me learn a bit more about release.
Some suggestions from me based on a first look:
There are definitely a lot of alerts on this page Although I wonder whether we can omit the warning entirely if the toggle is enabled?
For the toggle title, maybe something like Select feature flags by environment name could work? I think focusing on the action the toggle enables could be helpful here.
I agree, that last input could use a little work. How about a shorter title with help text to clarify?
To confirm, enter {Release Test Project}
Enter your project name to proceed. To cancel, close this modal.
is all fairly rough (and I'm definitely not the domain expert), so I'll let Russell give his two cents as well
@phillipwells - This is not an area I'm familiar with. Regardless, your suggestions look good to me. Especially focusing on the action the toggle enables.
@emilybauman - I know we're focused on the toggle, but as Phillip said earlier, there are a lot of warnings on this modal.
I may be going outside the intended scope, but I see the following issues with this modal and its content:
It's titled "Configure feature flags", but the action button is labeled "Regenerate instance ID".
The phrase starting "Install a compatible..." implies the values are provided prior to the modal. I may be misreading this, though.
The instance ID is an input field, but there's also a "Regenerate instance ID" button. That has me confused.
Thanks for the feedback @phillipwells - this simplifies the modal quite a bit and I love the focus on the action for the toggle. I have removed the first warning as I agree it may not be needed anymore, but if anyone thinks otherwise let me know:
Overall the entire modal confuses me.
@rdickenson - Yes I had similar concerns with the entire modal. It took me a while to figure out what I was supposed to be doing because the main action was greyed out. I do believe some of these content changes might be outside the scope of this issue, but we can open up a follow-up to capture them all since I think it would be better to clear up what is actually going on within the modal. I'll let @shinya.maeda speak to this.
@afontaine - I think ultimately this might be the best way to go. This modal is trying to do too much, and I think the actions are getting a bit muddied. I'd be on board to break it down to:
Popover with basic, copyable content and some supporting info.
Model with more information.
Adjusting the reset action to be similar to resetting a registration token with a confirmation modal. To be honest I've never seen something handled in GitLab quite like the regenerate action is being handled here, so it left me scratching my head a bit
Perhaps we can open an issue to discuss and mock something up if it is out of scope for this issue. Or if we think we can do all the changes in this one issue, I'd be fine with that as well
@emilybauman@afontaine - I really like Andrew's proposed break down of the one modal into the 3 pieces. If we want to keep the MR's scope small, however, I think Phillip's proposed changes to the modal qualifies as an MVC.
I don't have a strong opinion on UI/UX for the MVC - as long as the toggle can be changed by users. That being said, please feel free to override the current propposal.
I'd be fine using this issue to handle some content changes proposed by @phillipwells as an MVC and I can go ahead and open up another to handle the improved 3 piece experience. I think the small changes we can make here with just content will already have good impact on the usability of this modal.
I can work today to clean the issue up with the updated modal and we can go from there.
@shinya.maeda thanks for driving this issue forward!
@nagyv-gitlab you may want to look at this closer, and maybe discuss more with @shinya.maeda. This may be one of the things to prioritize from the ~"group::release" domain even though it's related to feature flags.
@vshushlin@shinya.maeda I'm moving it to ~"configure::parked". I understand that you might not like this decision.
I want us to focus on Environment management and Kubernetes. This issue seems too big to be delivered in a single (or less) milestone and the work did not start on it yet. As a result, I rather not start it now. It would only decrease our focus and require context switching between different areas of the product.
@nagyv-gitlab is this an issue that would accept a community contribution? Not volunteering myself (yet), but the issue itself seems pretty well refined but the label Seeking community contributions is not applied..
@tmeijn It's likely rather big for a community contribution, and might need multiple MRs (perhaps even issues) to track it properly.
Nevertheless, if you (or company) would be interested in contributing, I'm supportive. I agree that it's quite well refined. The description to be updated with a summary of the design proposal thread. Otherwise, it looks to be very well refined and discussed.
This is an issue for us as well. The appName parameter is acting as the environment when Unleash is hosted by Gitlab:
this._unleash = initialize({ url: unleashUrl, instanceId: unleashInstanceId, appName: environmentName, // Local, Development, etc. this maps to what Gitlab calls "environment" in the feature flags interface });
This means we either need to create an individual proxy for each front-end that talks to Gitlab Unleash or we need to share all our feature flags in a singular repository and prefix the app name. Example: my-app-local, my-app-development, my-app2-local etc.
Please fix this if you can, we're having to hack around it in the current implementation.