Skip to content
This repository has been archived by the owner on May 6, 2021. It is now read-only.

Idler should use the fabric8-toggles-service to interact with feature toggle server #83

Open
hferentschik opened this issue Jan 8, 2018 · 15 comments
Milestone

Comments

@hferentschik
Copy link
Contributor

Right now the Idler communicates directly with the Unleash server to determine whether a given user has the idler feature enabled. Should there be an internal service (aka fabric8-toggles-service) which abstracts from the underlying Unleash instance? This could, for example, be the fabric8-toggles-service.

Right now, however, the endpoint of fabric8-toggles-service is bound to the logged in user. A new endpoint would be needed which allows to also specify a user id.

To me, it seems like a good idea to use fabric8-toggles-service for all access to the underlying Unleash server. Unleash becomes more of an implementation detail. Also, fabric8-toggles-service could offer some convenience API endpoints to get a list of users with a specific feature enabled.

@vpavlin, @xcoulon - WDYT?

@hferentschik hferentschik changed the title Should Idler use the fabric8-toggles-service to interact with feature toggle server Should Idler use the fabric8-toggles-service to interact with feature toggle server? Jan 8, 2018
@vpavlin
Copy link
Member

vpavlin commented Jan 8, 2018

We could work with the token potentially - we can obtain a token for a user when we first encounter his OpenShift build/dc change, cache it and then use that to get the feature service response

@hferentschik
Copy link
Contributor Author

Another important abstraction I am after is being able to check for a feature without knowing the underlying strategy. This should be transparent to the caller. I am not interested in what Unleash strategy is used under the hood. In fact, afaict a given feature can be enabled by multiple strategies. I just want to know whether for a given user x a specific feature y is enabled. Everything else should be transparent.

Right now, the fabric8-jenkins-idler is built around the userIs strategy whereas fabric8-toggles-service uses strategy based on level. The toggle service should allow for dynamically switching the type of strategy.

As said, Unleash seems to already support this. The API to verify whether a feature is enabled is generic in terms which parameters are passed and multiple parameters can be passed as well. Depending on the strategy/strategies configured for a feature, the required parameters are extracted dynamically. There are basically two ways to go about this:

  • When a call is made to detect whether a feature is enabled for a given user, we first query the for the strategies configured for this feature and prepare the context for the IsEnabled call accordingly
  • Generally put all potential feature determination parameters into the context (uid, level, etc) and let Unleash do the work

@xcoulon WDYT?

@xcoulon
Copy link

xcoulon commented Jan 9, 2018

yes, I think we could expand the current toggles service to make it more generic and support multiple properties retrieved from the user's profile in auth, in order to apply some strategies (enable by email, enable by feature level, etc.)

@hferentschik
Copy link
Contributor Author

yes, I think we could expand the current toggles service to make it more generic and support multiple properties retrieved from the user's profile in auth, in order to apply some strategies (enable by email, enable by feature level, etc

That's my thinking as well. Making fabric8-toggles-service more generic in terms of its IsEnabled API call, is the first step. Moving forward I see fabric8-toggles-service as a natural place to implement things like "give me the list of users for which a given feature is enabled".

@xcoulon, as discussed we also need another API call which takes userId and feature name. Calls to this API method should be only allowed using a service account token.

@xcoulon What is the next step here? Create some issues within fabric8-toggles-service?

@hferentschik hferentschik changed the title Should Idler use the fabric8-toggles-service to interact with feature toggle server? Idler should use the fabric8-toggles-service to interact with feature toggle server Jan 9, 2018
@aslakknutsen
Copy link
Contributor

@hferentschik @xcoulon Just to note: isEnabled in the unleash client is a local check only. The rules are synced in an async pull model from the unleash server. Making an isEnabled call on fabric8-toggles-service for all services to rely on means this moved from being a local process 'if' statement to a network call.

@corinnekrych
Copy link

@aslakknutsen
Copy link
Contributor

@corinnekrych That's a different thing, but it highlights another issue: Unleash keep track of which apps are using which toggles etc, you lose that in the suggested approach above.

@hferentschik
Copy link
Contributor Author

hferentschik commented Jan 9, 2018

Just to note: isEnabled in the unleash client is a local check only. The rules are synced in an async pull model from the unleash server.

Interesting, I wish the Unleash docs would be a bit clearer here. Also, there are several bugs in the client (at least the Go client). Hence my thought of having to deal with Unleash onlu in one place instead of every service doing their own thing, potentially making the same mistakes over and over again.

@aslakknutsen, so are you saying that each service should always use the Unleash API directly? Then what's the purpose of the fabric8-toggles-service in the first place?

Making an isEnabled call on fabric8-toggles-service for all services to rely on means this moved from being a local process 'if' statement to a network call.

Sure, but you do it once and then keep it locally, right? I see the benefits of the async Unleash client model, but imo there are several benefits in having our own toggle service as well:

  • abstract from Unleash (making it an impl detail)
  • prevent making the same implementation errors related to Unleash in multiple services
  • ability to add additional functionality (list of users with a given toggle enabled)

ATM, I am more leaning towards _ fabric8-toggles-service_ being some gateway to the underlying feature toggle management, over using Unleash directly. Which way should we go?

@hferentschik
Copy link
Contributor Author

Unleash keep track of which apps are using which toggles etc, you lose that in the suggested approach above.

Is this information relevant for us? If it is one might be able to pass it along to the call to fabric8-toggles-service?

@xcoulon
Copy link

xcoulon commented Jan 9, 2018

isEnabled in the unleash client is a local check only. The rules are synced in an async pull model from the unleash server. Making an isEnabled call on fabric8-toggles-service for all services to rely on means this moved from being a local process 'if' statement to a network call.

@aslakknutsen fair point!

@hferentschik
Copy link
Contributor Author

@xcoulon so what is the intent of fabric8-toggles-service in the first place?

@aslakknutsen
Copy link
Contributor

aslakknutsen commented Jan 9, 2018

Also, there are several bugs in the client (at least the Go client).

Example?

Then what's the purpose of the _ fabric8-toggles-service_ in the first place?

Originally only to expose Unleash in a UI consumable way and an API to control the groups/memberships.

Sure, but you do it once and then keep it locally, right?

Local cache per user per toggle? Unleash assumes correctly that 'most' will be off. Only 'on' rules are synced.

@aslakknutsen
Copy link
Contributor

aslakknutsen commented Jan 9, 2018

Is this information relevant for us? If it is one might be able to pass it along to the call to fabric8-toggles-service?

Yes, it's absolutely relevant. It tracks if anyone is still using it, when it's being used, who is using it etc etc. When there are 100s of toggles defined, this matters.

@xcoulon
Copy link

xcoulon commented Jan 9, 2018

@hardy: initial intent for fabric8-toggles-service is to provide an endpoint for the UI to enable/disable features. UI cannot talk to unleash directly because you have to use a client which cannot ship in the UI: it has to be server-side.

@hferentschik
Copy link
Contributor Author

Yes, it's absolutely relevant. It tracks if anyone is still using it, when it's being used, who is using it etc etc. When there are 100s of toggles defined, this matters.

Sure. As said, nothing stops us to pass along the app name as well. It seems the app name is somehow part of the Unleash client initialization, but that we could work around I guess.

I guess the big elephant in the room is whether we can agree on whether it makes sense to have something like fabric8-toggles-service as a facade towards Unleash. I can see several advantages (see #83 (comment)) which needs to be weighed against the async model the Unleash native clients work.

@xcoulon

initial intent for fabric8-toggles-service is to provide an endpoint for the UI to enable/disable features. UI cannot talk to unleash directly because you have to use a client which cannot ship in the UI: it has to be server-side.

Got you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants