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

Single evaluation endpoint for variant and boolean types #2507

Open
1 task done
jalaziz opened this issue Dec 8, 2023 · 4 comments
Open
1 task done

Single evaluation endpoint for variant and boolean types #2507

jalaziz opened this issue Dec 8, 2023 · 4 comments
Labels
proposal Just putting it out there xl As big as it gets

Comments

@jalaziz
Copy link
Contributor

jalaziz commented Dec 8, 2023

Problem

Splitting boolean and variant feature flag types on the server side makes sense, but having different APIs makes this more complicated client-side as you have to know what type of flag you're evaluating.

While generally you would know, variant flags can easily act as boolean flags, so there's some crossover.

The issue arises when you try to create friendly wrappers around the Flipt client.

In our case, we we're developing a wrapper with a few "friendly" methods that automatically pass the evaluation context. This include methods like IsEnabled, GetAttachment, and Evaluate. In all but the attachment case, the caller doesn't need to know the type of flag.

Ideal Solution

While we could use the batch evaluation API for this, it would be nice to be able to evaluate a single flag and have the response indicate the type. Many of the fields are already shared between the different flag types (with the notable exceptions of segmentKeys, variantKeys, and variantAttachment).

I realize this ask may actually be moving things backwards in a way, but it was easier to build higher level wrapper when there was only one evaluation endpoint.

Search

  • I searched for other open and closed issues before opening this

Additional Context

No response

@GeorgeMac
Copy link
Contributor

It definitely wouldn't be hard to add an overloaded return type here. We could continue with the pattern we've started, but use oneOf in proto to support an evaluate for an arbitrary flag type. That would unblock your usecase here.

Just to clarify the decision to move to a more strict set of evaluation types. The mindset here was (in the spirit of type safety) we were assuming clients would (want to) know a specific type on the consumer side of the equation. As in, you're usually expecting one type at the callsight e.g. if flag is enabled go one way or the other. Overloading the contract can lead to code becoming brittle with all the edge cases a flag could end up being. The variant flag type is a bit like this in itself. It represents a boolean, a string or an additional arbitrary blob (attachment). Whereas, we thought we would move to a more concrete set of typed flags (boolean to start, then maybe int and string and so on), for a more predictable and constrained experience as a client consumer. I think e.g. OpenFeature models flags in this way too (ConfigCat from my past experience did this too).

Regardless though, if you find value in more free-form flag types I think both worlds can be supported 💯 .

Aside (felt relevant): We're actually thinking of creating a space in the API for generic configuration with definable schema. The mindset being that e.g. a boolean flag, or a set of variants could be simply expressed via a schema. Then Flipt will enforce that schema while trying to set values and clients can both expect or even be generated from that schema.

@markphelps markphelps added proposal Just putting it out there xl As big as it gets labels Dec 12, 2023
@jalaziz
Copy link
Contributor Author

jalaziz commented Dec 12, 2023

The mindset here was (in the spirit of type safety) we were assuming clients would (want to) know a specific type on the consumer side of the equation.

I think this definitely makes sense, but it depends on who the client is and how they're using flags.

In my experience, we generally want to use flags to answer one of three things:

  1. Is this flag enabled or not?
  2. Is this flag enabled? If so, give me it's attachment value.
  3. Give me the value for this flag.

2 & 3 could be combined into one depending on how the attachment is defined (for example, a nil value could mean disabled, but it's generally better to be explicit).

When it comes to type safety, there's certainly a lot of value in the value type of the flag being type safe, but the issue here is that determining whether a flag is enabled or not is now split across multiple endpoints that are different based on the value type of the flag.

I do realize variant flags will return matched instead of enabled, but that's fundamentally the same thing.

Also, you could argue that if you are checking if a variant flag is enabled or not, then you also likely want to know the segment / attachment and would also like to avoid additional API calls.

All this being said, it really feels like the type safety should be handled at the client level and not the API level. What I mean by this is that I should be able to call a simple API to evaluate the flag against some context, determine whether its enabled/matched, determine its type, and get its value. Then, client-side, I should be able to lookup the type-safe value of the flag.

My reasoning behind this is:

  1. IsEnabled is universal to all flag types. If I'm building a client wrapper, I really shouldn't care what the type of the flag is.
  2. Related to 1, in my experience, context is independent of flag type, so it's pretty natural to build some sort of middleware or client wrapper that injects evaluation context from overall runtime environment. Having different endpoints makes this more difficult on the consumer.

Circling back to type safety, if we agree it mainly applies to the value type of the flag, then we're likely saying that the client should error out when you request a value type that doesn't match. This can generally be done in most languages, and definitely with proto and oneOf.

For example, I personally would much prefer:

flag, err := client.Evaluate("foo", context)
if flag.IsEnabled:
    val, err := flag.StringValue()

over

flag, err := client.EvaluateString("foo", context)
if flag.IsEnabled:
    val := flag.Value()

Yes, it's an additional error check (in Golang), BUT it does allow me to branch off the type of the value if I want to support something dynamic without multiple API calls. Ultimately, it's more flexible.

Aside (felt relevant): We're actually thinking of creating a space in the API for generic configuration with definable schema. The mindset being that e.g. a boolean flag, or a set of variants could be simply expressed via a schema. Then Flipt will enforce that schema while trying to set values and clients can both expect or even be generated from that schema.

That would be amazing. We're actually playing around with Go generics to try and a client wrapper to try and enforce a schema for our attachments. Have the schema defined as part of the flag is super valuable, especially for validation purposes.

@GeorgeMac
Copy link
Contributor

I totally see where you're coming from. FF's are kind of a variable (flag) that is stored in a remote service. Where the server itself can't protect the client side code at compile time, because they're completely decoupled. So I can see that attempting to make this a server side concern still produces a runtime exception, regardless if you make a type-safe client-side call, it can be a mismatched type with the server-side definition.

In lieu of actually being able to have client side type-checked against the server side definitions, it is perhaps more appropriate to make a runtime type check and coerce or adjust on the client side. 👍

Another aside (😂): To make server-side validation more complete we would likely have to bring in CI time validation of client-side types, based on the server-side definitions. This is actually something we started down with https://github.com/flipt-io/ffs , but we have put on the shelf for now. Mark got as far as ensuring flags exists for both Go and Typescript I think. You could imagine taking it to the next step to ensure that they're of the expected type.

Glad to hear our general configuration ideas resonate. We will morph the design ideas into GH issues soon I suspect, it is hot on the agenda.

@jalaziz
Copy link
Contributor Author

jalaziz commented Dec 14, 2023

We just ran into something that I think is related to the broader conversation we're having here.

We're using boolean flags with segments to allow us to control whether the flag is enabled or disabled by environment.

We started to enable segments, only to realize we were still getting a false value back because the flag itself was disabled at the top level. While this ultimately makes sense, I think it adds further nuance to how flags should be treated (even boolean flags).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Just putting it out there xl As big as it gets
Projects
Status: Proposed
Development

No branches or pull requests

3 participants