Typed API proposal

[toddbaert]

A few conversations and issues (https://github.com/open-feature/spec/issues/22), as well as the survey of some existing vendor SDKs (https://github.com/open-feature/spec/blob/main/research/api-comparision.md) suggest that typesafe functions/methods are likely a requirement for the OpenFeature SDK. Below I have a very simple proposal, with some in-line considerations, for some typesafe interfaces for getting flag values.

Pseudocode:

interface OpenFeatureClient {

  /**
  * Get a boolean flag value.
  *
  * Note: As discussed, we may want to expose a "friendlier" boolean flag method,
  * called something like "isEnabled()", which would be a shortcut for this method.
  */
  boolean getBooleanValue(string flagId, boolean defaultValue, Context? context);

  /**
  * Get a string flag value.
  */
  string getStringValue(String flagId, string defaultValue, Context? context);

  /**
  * Get a number flag value.
  */
  number getNumberValue(String flagId, number defaultValue, Context? context);

  /**
  * Get a object (JSON) flag value.
  *
  * Note: Generics support is not universal, so we may need other
  * mechanisms to support some degree of type-safety in languages without generics support.
  *
  * Note: Parsing is an interesting question here. Some SDKs simply return stringified JSON,
  * while others return parsed "map-like" objects in the case of JSON.
  * "Hooks" (https://github.com/open-feature/spec/issues/25), configured globally or per flag,
  * may provide a means to configure custom parsing of objects, but we may not want to do any parsing by default at all.
  * This question also has implications on dependencies. To keep the SDK as lightweight as possible, we likely wouldn't
  * want to implement parsing there, but instead require it to be done in the provider implementation.
  */
  T getObjectValue<T>(String flagId, T defaultValue, Context? context);
}

Open questions:

• If the type returned from the feature flag backend is not of the expected type, should the default be returned?
• should parsing of JSON be implemented? If so, should it be implemented in the provider? If not, would we stringify JSON values for feature flags SDKs that provide already parsed objects (as in the case of Harness)?

Feel free to poke any holes in this proposal you can, or alternatively, add your own proposal. I'll work on implementing something resulting from our discussion here in the SDK-research repo.

[agentgonzo]

If the type returned from the feature flag backend is not of the expected type, should the default be returned?

No, it should throw an error.

[agentgonzo]

I would suggest two improvements:

1. add a optional default value to the call that the user can pass in. It's handy for callers, and some SDKs support it, so may as well expose it to the API.

2. change the name of the methods from getXyzVariation to getXyzValue. To me, this is more intuitive. The term 'variation' seems to be specific to a couple of providers and as a new-user to this API, my first question would be "what is a variation?". To which the answer is "it's the retuned value from the flag" and everyone understands the meaning of value. You even call it a 'value' in the javadoc!

[agentgonzo]

flagId vs flagName. Ultimately, it probably doesn't matter as it will just be translated into whatever the underlying provider uses, but I'm curious as to what the different providers use for this. Generally, an ID will be a random string of hex that means nothing to the user, and the name has meaning. CloudBees flags use the name here rather than an ID. What do other providers use?

[toddbaert]

I would suggest two improvements:

1. add a optional default value to the call that the user can pass in. It's handy for callers, and some SDKs support it, so may as well expose it to the API.
2. change the name of the methods from getXyzVariation to getXyzValue. To me, this is more intuitive. The term 'variation' seems to be specific to a couple of providers and as a new-user to this API, my first question would be "what is a variation?". To which the answer is "it's the retuned value from the flag" and everyone understands the meaning of value. You even call it a 'value' in the javadoc!

Thanks @agentgonzo . I agree and I've updated the proposal with both these suggestions.

[beeme1mr]

If the type returned from the feature flag backend is not of the expected type, should the default be returned?

No, it should throw an error.

Hey @agentgonzo , are you suggesting that an error should bubble up to the application author? How does this work with dynamic values in Rollout?

I think we should avoid throwing errors that could affect the users application. However, we could still throw an error within the SDK and give application author's the ability to register an error callback as a lifecycle hook.

In general, I would expect to be able to allow a non-technical person to be able to make flag configuration changes in a UI without having to worry about breaking the application. Of course, the UI should probably prevent users from changing the return type once a flag has been created but that's out of our control.

[agentgonzo]

are you suggesting that an error should bubble up to the application author?

Yes. If the user is asking for a number, but in actual fact it's a string, then they should get an error. (because it's not a number).

How does this work with dynamic values in Rollout?

In Rollout, the SDK will create the flags for you if they don't exist (ie, the flag creation is done by the SDK itself, so you're always going to have a flag of the correct type). Though I don't quite know what happens if you create the flag as a number, then change it to a string type. It's possible it's illegal, but I'll get back to you on that.

Of course, the UI should probably prevent users from changing the return type once a flag has been created but that's out of our control

It would be interesting to see what the different providers allow and don't allow here. CloudBees doesn't allow the user to change the type of a flag. IMO that would be a bad thing anyway (think of what would happen for a rolling upgrade where you have two versions of the code running at once - one expecting a number, one expecting a string, but in the provider platform, you can only have a flag defined as a singular type.

I think we should avoid throwing errors that could affect the users application.

I think you should throw an error if the developer does something wrong. In this case, if they are asking for a number, and the value is a string, I think an error is the correct thing to do. Consider what the languages themselves do. I think that Java throws an exception if you try to parse a garbage string into an int. Node just returns NaN. Fundamentally, asking for a number when it is a string is an error. we should treat it as such.

[dabeeeenster]

In Flagsmith, Flags are a combination of both a boolean (to define whether the flag is on or off) and a value. The value can be one of either

• string
• int
• boolean

Where a more complex data type is required (e.g. json, XML, float etc) users are expected to use string and do the type conversion themselves.

Values are not required.

So the main concern here is that Flagsmith flags can return two pieces of data, and both could be boolean!

I'm not sure how other vendors approach this, but I think it would be important to provide a generic way of interfacing with the flag provider in the event that the interface defined here doesn't map to the provider.

[toddbaert]

@agentgonzo I see your points, and generally, I agree with the statement:

I think you should throw an error if the developer does something wrong.

I think that a developer could be "not wrong" about a flag value today, but wrong tomorrow. Maybe they switch providers (maybe they went from local env-var, or a future cloud native solution, to a full-feature Saas solution such as Rollout) or some providers allow flag type changes.

The resultant necessary error handling seems like it could add a lot of complexity for something that can be reasonably treated with a fallback (the default).

[toddbaert]

I'm not sure how other vendors approach this, but I think it would be important to provide a generic way of interfacing with the flag provider in the event that the interface defined here doesn't map to the provider.

Good point @dabeeeenster

We already have a concept of a "Context" (see the glossary) which perhaps we could also return as part of a larger flagValue wrapper in other methods (getFlagValueWithContext, similar to something Split has as well: getTreatmentWithConfig ). This could contain arbitrary additional data specific to the provider, in a structure the provider defines.

[agentgonzo]

It's true. It could be handled by resorting to the default value. Both approaches work. Resorting to a default value can make it harder for the developer to debug though (if they call the wrong method and sit there scratching their head as to why the flag is returning the default value). We could always log a warning and then return the default value if you think raising an error is the wrong thing to do here.

What do others think?

At this stage, we can probably pick on and see how we get on. If we wanted to go super-complex, you could even define a switch in the SDK setup for strict mode at a global level or something like that: strict mode on => throw an error if the types don't match. strict mode off => log a warning and return the default value

[moredip]

Just want to call out that we should be careful to not blindly adopt the otel philosophy of never throwing an error (and never logging?). This philosophy makes sense in the context of o11y, but I'm not 100% sure it does in the context of feature flagging.

I think at the very least we should be logging errors if someone does things like asking for a boolean value from a string-typed flag. Fast, clear feedback is a useful thing.

[toddbaert]

Just want to call out that we should be careful to not blindly adopt the otel philosophy of never throwing an error (and never logging?). This philosophy makes sense in the context of o11y, but I'm not 100% sure it does in the context of feature flagging.

I think at the very least we should be logging errors if someone does things like asking for a boolean value from a string-typed flag. Fast, clear feedback is a useful thing.

I see your point, I think the discussion is valid. I also agree the decision with OTel is quite obvious (never throw) compared to our use case here.

For me, it comes down to this: What is the worse case for either decision?

Never throw: worst case I can think of, is a application author spends time confused about why a feature is defaulted when it should be some other expected value.

Throw: some change in the flag backend is now resulting in 500s (especially relevant for languages which dont force exception handling).

I think at the very least we should be logging errors if someone does things like asking for a boolean value from a string-typed flag. Fast, clear feedback is a useful thing.

I certainly agree with this. We could also support throwing in error() lifecycle event hooks if we end up going with that proposal.

[moredip]

Throw: some change in the flag backend is now resulting in 500s (especially relevant for languages which dont force exception handling).

The thing is, that's always a risk when you make a change to a flag in the backend. It's a fundamental part of dynamic feature flag configuration - you can break stuff when you change the configuration. You could argue that if there's an option to avoid the 500 - by logging a warning and falling back to a default - then that's what you should do, but I don't think there's a clear cut winner here. There's a strong counter-argument that you should fail in a fast, clear way, rather than attempting to fall back a default code path which may well not have been exercised or tested for a long time, and could cause bigger issues than failing hard.

The more I think about it, the more I lean towards:

• allow the behavior to be customized "in user land" by the Application Integrator (the error hook seems really nice)
• definitely take a default position of logging a warning when the SDK is being asked to make a flagging decision that doesn't make sense (i.e. invalid type, missing flag)
• probably take a default position of throwing an error in that case

[justinabrahms]

flagId vs flagName

We use "flag key" for our system.

---

As for throwing behavior, my take is this: We should never throw errors and always return a default in the error case. We should still provide a mechanism for users to understand this is happening.

The way we're intending to handle this is by providing two APIs. One gets the raw value, one wraps it in a result type that lets you interrogate why we got that value. It tracks very closely with LaunchDarkly's API: java or js

interface Flag<T> {
  T getValue(key, ctx, default)
  EvaluationDetail<T> getEvaluationDetail(key, ctx, default)
}

interface EvaluationDetail<T> {
 T getValue()
 EvaluationDetail.KIND getReason()
 Optional<String> getErrorReason() // only populated if getReason() returns error type.
 // .. more methods like in linked docs
}

kind enum definition

[beeme1mr]

As for throwing behavior, my take is this: We should never throw errors and always return a default in the error case. We should still provide a mechanism for users to understand this is happening.

Yeah, I agree. Defaults are already used when various error conditions are detected. I would argue that an invalid return type shouldn't be treated any differently.

There's a strong counter-argument that you should fail in a fast, clear way, rather than attempting to fall back a default code path which may well not have been exercised or tested for a long time, and could cause bigger issues than failing hard.

If the default code path is that risky, you've likely had the flag in your code base too long. However, that's likely part of a larger discussion around the expected lifecycle of a feature flag.

[moredip]

If the default code path is that risky, you've likely had the flag in your code base too long.

This is 100% correct and also extremely common in the real world. IME, most teams using feature flags at scale are going to have some flags like these.

[toddbaert]

I think providing another set of similar methods to those proposed in my first post, which wrap the flag value in a container like @justinabrahms mentioned here, also provides a means of solving the problem of getting arbitrary additional provider-specific data @dabeeeenster mentioned here. I think I will take a first pass at this, and add it to the proposal.

Then I'll implement it in a branch in the SDK-research repo.

[patricioe]

Team,

Adding one more angle to this. Hopefully not too problematic.

Split.io support the addition of configuration to each flag variation. For example, we support:

var treatment = splitClient.getTreatment(userId, flagName)
var treatmentResult  = splitClient.getTreatmentWithConfig(userId, flagName)
// then treatmentResult.treatment or treatmentResult.config

source: https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#get-treatments-with-configurations

type TreatmentResult = {
    treatment: string,
    config: string | null
  };

How can this spec support or be friendly to this?

[toddbaert]

@patricioe , see this comment. We could use this wrapper to add arbitrary metadata, including the split config.

[patricioe]

@toddbaert makes sense. If there is an opportunity to model it other than in the context it would be probably preferred but if that is the only way, I'll take it.

[InTheCloudDan]

For LaunchDarkly we use flag key to describe the identifier being passed in since it is a unique string per project. And in the case of type error, we return the default value.

https://github.com/launchdarkly/go-server-sdk/blob/5adef9bac9b3c9545fb6fbff3e4ad3ff639ba390/ldclient.go#L840-L842

https://github.com/launchdarkly/go-server-sdk/blob/cfe6bbdda7a74c586988bd5e4d1e454f62b6a7bf/interfaces/client_interface.go#L14-L15

[toddbaert]

For LaunchDarkly we use flag key to describe the identifier being passed in since it is a unique string per project. And in the case of type error, we return the default value.

Thanks for that feedback!

[s-sen]

One option is that instead of separate APIs for the supported data types we consolidate on a single API using Generics

T Result<T> getValue(String flagKey, T defaultValue, Context? context);

interface Result<T> {
  T getValue();
  EvalDiagnostics getDiagnostics();
}

// used for logging or for figuring out questions such as 'why is the default value returned'
// without delving into vendor specific logs
interface EvalDiagnostics {
  Status getStatus();  // Status is an enum that indicates what the issue may be
  String getDetails(); // String that has more detail of the issue
}

As mentioned previously, the downside of this are as follows:

a. What is the story for languages that do not have generics support? Do we create a separate interface for these? b. If the feature flag value is a complex type e.g. a json string, then T can be supported by each vendor as a complex type specific to the vendor. In this case, the client code is forced to use vendor specific data type as shown below

Result<VendorComplexType> getValue("myflag", "MyVendorComplexTypeDefaultValue", myCtx); So this adds dependency on a specific vendor in the client code which we are trying to avoid.

One option is to mandate that T is a language specific type e.g. Boolean, Number, String or a named type that extends Object. When it is the named type, it has to support a standard API to retrieve the value as a Map<String,String>, and each vendor is responsible for implementing how to transform the underlying vendor specific object to a map of strings. This can cover complex json string as the feature flag value that can be translated into a map of strings without adding vendor specific dependency on client code.

[toddbaert]

@s-sen I would add one more downside here: In languages with no runtime type information (javascript), type-safety with this "entirely generic" pattern is particularly limited. In providers which have a typed API, we would have to "guess" which underlying typed method to call, or use the default value as a means of determining that. That seems problematic though, since the default may be the semantically "wrong" type, accidentally, leading to additional confusion.

[dabeeeenster]

I feel like ultimately this is going to come down to a trade off between vendor lock in and "good engineering/testability/type checking etc".

IMHO I think removing vendor lock in should be the primary concern, and it should be taken at the expense of computer-sciency type things.

Having said that, there are always going to be areas where different vendors have features that are just not compatible with each other and a migration with a single LOC change to redefine the flag provider is a panacea, but I still think its one that should be the primary goal of this project.

[toddbaert]

@dabeeeenster I think I generally agree. Can you elaborate on how that concern specifically relates to the SDK type conversation? I think the proposal above generally works across vendors and languages, and does address the lock-in concern.

Incidentally, I'm working today on our first pass at a Flagsmith provider, probably based on the v2-beta Flagsmith SDK. You will see it appear in the SDK-research repo, satisfying the interfaces defined above.

[dabeeeenster]

Yes sorry I was getting a bit philosophical 😁

So as an example, let's say a vendor provides a way of returning a rich data structure in their current SDK that can be interrogated and interacted with directly with the SDK. I don't think openfeature should make an attempt to provide complex interfaces to these rich data types. In this case, we should just return some sort of Object/String (depending on the idiom of the language) and then allow the developer to make the relevant type conversion within their code.

If a developer makes use of these data structres, they have sort of locked themselves out of working with a flag vendor that doesn't provide that similar rich data structure, but I don't think it should be in scope of openfeature to worry about that. This lock in/code is happening outside of the openfeature wrapper/sdk.

At some point there are going to be incompatibilities between vendors (more at the edges) that cant really be catered for without going down long rabbit holes.

Looking forward to seeing the Flagsmith adapter!

[toddbaert]

So as an example, let's say a vendor provides a way of returning a rich data structure in their current SDK that can be interrogated and interacted with directly with the SDK. I don't think openfeature should make an attempt to provide complex interfaces to these rich data types. In this case, we should just return some sort of Object/String (depending on the idiom of the language) and then allow the developer to make the relevant type conversion within their code.

Yes, this makes sense to me.

I think what we were most concerned about in this issue was being able to strongly-type returned flag values. I agree that things like diagnostic information, metadata, etc, should likely be exposed on "weakly typed" maps or other structures. I think the besides the "well-defined" flag types, the only thing we want to rigorously define is a standard set of attributes supplied as the user/context.

[s-sen]

If we are thinking of using a registered hook for diagnostics related data, IMO it should be separate from the OpenTelemetryHook that is being discussed in the other thread. The diagnostics info is an important tool for initial troubleshooting by client developers without having to delve into provider-specific logs.

If I am a client developer, I am only interested in the evaluated value of the flag and errors or other diagnostics that help me debug if I am not getting the expected result. Telemetry related implementation should be implemented by the provider and I will only look at provider generated logs if I am not seeing the correct telemetry related information e.g. is my new feature triggering a high error count.

With that in mind, we can create a separate hook for diagnostics that has a function that is triggered after evaluation. The client developer can implement this for diagnostics logging or debugging.

class OpenDiagnosticsHook implements OpenFeatureHook {
...
after (Context context, FlagKey flagkey, Diagnostics diagnostics) {
    // optionally handle diagnostics/errors with flag resolution in this life-cycle hook.
}
...
interface Diagnostics {
  Status getStatus();  // Status is an enum that indicates what the issue may be
  String getDetails(); // String that has more detail of the issue
}

[toddbaert]

I'm going to close this since it's captured in the current spec. Feel free to open PRs or further discussions if needed.