-
Notifications
You must be signed in to change notification settings - Fork 14
Variations and Variables #85
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @fahad19, great proposal! Most of what you proposed is already technically possible but may benefit from more user-friendly method names. For reference, the method names were originally inspired by this API comparison.
| const featureKey = "my_feature"; | ||
| const defaultState = false; | ||
|
|
||
| const isEnabled = client.isEnabled(featureKey, defaultState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you see this as syntactic sugar for the existing getBooleanValue method, or would this represent an entirely new value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we consider features to contain a single value only, then yes it's a syntatic sugar.
but if we wish to establish three separate kinds of values, like:
- flag status (boolean)
- variation (string)
- variables (key/value pairs with different types of values)
then .isEnabled() ends up with a meaning of its own, which evaluates the flag's status as boolean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like it would be very difficult to have something meaningful here cross-vendor. This feels more like the "why" that would be associated with the reason or meta-data associated with a flag evaluation. (The detailed evaluation reason would be "on").
I don't think there is a consistent idea of what "on" would be, or if all vendors would have such a concept.
| const featureKey = "my_experiment"; | ||
| const defaultVariation = "control"; | ||
|
|
||
| const variation = client.getVariation(featureKey, defaultVariation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this differ from the getStringValue method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the intent is different.
imagine having both variations and variables under the feature, which are of both string type.
in that case, how can we let the provider know what kind of value to compute?
| const variableKey = "showSidebar"; | ||
| const defaultValue = false; | ||
|
|
||
| const showSidebar = client.getVariableBoolean(featureKey, variableKey, defaultValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the concept, especially with the type enforcement at the variable level. We can do something similar, but your proposal may be a nicer dev experience.
Here's what's already possible.
const featureKey = "my_feature";
const defaultValue = { showSidebar: false };
const { showSidebar } = client.getObjectValue(featureKey, defaultValue);In TypeScript, it would even be possible to register a hook that validates the object before returning it to the client.
import { z } from "zod";
const myFeatureSchema= z.object({
showSidebar: z.boolean(),
});
type MyFeature = z.infer<typeof myFeatureSchema>;
const featureKey = "my_feature";
const defaultValue: MyFeature = { showSidebar: false };
class Validator {
constructor(private readonly zodSchema) {}
after(_, evaluationDetails) {
this.zodSchema.parse(evaluationDetails.value);
}
}
const { showSidebar } = client.getObjectValue<MyFeature >(featureKey, defaultValue, undefined, { hooks: [new Validator(myFeatureSchema)] });NOTE: This was written from memory and has yet to be tested.
The second example is fairly complex but would offer runtime variable validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
client.getObject() can work, but this will require the provider to compute all the variables together and return it as an object, which application can destruct from.
the proposed API in document suggests we evaluate individual variables scoped under each feature on-demand.
also when the method name specifically says getVariable*, it shows the intent and the kind of desired value as well. We want the "variable" to be evaluated with that specific key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I had discussed this in another OFEP or spec comment, but I think that the idea of a "scoped" feature could be added and would probably be more widely applicable.
Variables for this system seem closer to what variations may generally be from other systems (multiple types, potentially objects), and you would use several "flags" to accomplish something similar in those systems (or one structured flag containing the relevant data).
Here they are together in that a flag contains a few more states that are exposed.
If you were to imagine this at a very basic level it would be something like.
"myFeature" -> Variation (with on/off as details of that evaluation). Maybe this means some extra work is done.
"myFeature:showSideBar" -> Variable.
Being strings is for illustrative purpose, but if there is true need, then some more specific way could be devoted to support the idea.
Currently I think this is divergent enough that it would create a lot of dead API surface for most providers. There may be a way to make it into something more uniform though. How do we adjust the API so that two providers are not completely divergent from an API usage perspective?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you were to imagine this at a very basic level it would be something like.
"myFeature" -> Variation (with on/off as details of that evaluation). Maybe this means some extra work is done.
"myFeature:showSideBar" -> Variable.
Being strings is for illustrative purpose, but if there is true need, then some more specific way could be devoted to support the idea.
This is an interesting idea.
Currently I think this is divergent enough that it would create a lot of dead API surface for most providers.
I haven't had time yet to really dig into this proposal, but I think as it stands, this is my biggest concern with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the comments!
scoped feature would solve more than half of the challenges I am having creating a new provider.
Variables
It would also be more like naming convention based like you suggested, without requiring any further development from OpenFeature's side:
// regular
client.getBooleanValue("myFeatureKey");
// scoped
client.getBooleanValue("myFeatureKey:myVariableKey");Variations
for the variations part, OpenFeature provider-based API usage could be using myFeatureKey:variation.
another challenge would be emitting an "activation" event when an Experiment has been exposed to end users. For that the Context API can be abused a bit just to make it work somehow (just thinking out loud):
client.getStringValue("myFeatureKey:variation", "control", {
// ... rest of context
"$activate": true,
});isEnabled
given we adopted the convention as mentioned above, the proposed .isEnabled() method can be used as:
client.getBooleanValue("myFeatureKey");and it will not clash with any variables, because if variables were intended the user was expected to pass the key as myFeatureKey:someVariable instead.
Question
is this a convention that we wish to align on and document somewhere at OpenFeature level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could bubble up events using hooks. A hook can be configured to run after each flag evaluation and even be included in the provider itself. It's also possible to pass "hook hints" at evaluation time. These can be used in a hook to customize the behavior even further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A hook can be configured to run after each flag evaluation
this is what I am specifically trying to avoid. we should not emit an event at the time of evaluation.
the event should be emitted only when the application thinks the user has been exposed to that feature/experiment, manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey just adding my voice to this as we @ DevCycle have a somewhat similar hierarchy to what's described here, having Features as our top-level organizational object that contain multiple variables (flags) underneath: https://docs.devcycle.com/introduction/core-concepts/feature-hierarchy
https://docs.devcycle.com/sdk/server-side-sdks/node/node-openfeature
https://github.com/DevCycleHQ/js-sdks/blob/main/sdk/openfeature-nodejs-provider/src/DevCycleProvider.ts
We generally have learned that the vast majority of folks want to just interface with flags (variables) as the main interface to our SDKs. There are very few folks who need the information about the feature + variation data at run-time, mostly that data is just needed for reporting / eventing. That is certainly something we've struggled with ourselves with OpenFeature is how to get the variation + feature metadata attached to the resolution details.
I think a better approach here would be to figure out how to keep the core interface of the OpenFeature spec the same, where users are interfacing with different types of flags (variables), but exposing the feature + variation data as part of the Resolution Details / Detailed Evaluation data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey just adding my voice to this as we @ DevCycle have a somewhat similar hierarchy to what's described here, having Features as our top-level organizational object that contain multiple variables (flags) underneath: https://docs.devcycle.com/introduction/core-concepts/feature-hierarchy https://docs.devcycle.com/sdk/server-side-sdks/node/node-openfeature https://github.com/DevCycleHQ/js-sdks/blob/main/sdk/openfeature-nodejs-provider/src/DevCycleProvider.ts
We generally have learned that the vast majority of folks want to just interface with flags (variables) as the main interface to our SDKs. There are very few folks who need the information about the feature + variation data at run-time, mostly that data is just needed for reporting / eventing. That is certainly something we've struggled with ourselves with OpenFeature is how to get the variation + feature metadata attached to the resolution details.
I think a better approach here would be to figure out how to keep the core interface of the OpenFeature spec the same, where users are interfacing with different types of flags (variables), but exposing the feature + variation data as part of the Resolution Details / Detailed Evaluation data.
This is an interesting perspective, and an appealing idea, to me at least.
@fahad19 do you think we could add well-defined properties to the detailed evaluation methods that would help? value would still maintain its current semantics (variable in DevCycle language as @jonathannorris mentions), but we could perhaps add additional properties that would satisfy your use-case, while remaining coherent with systems that don't have such concepts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @toddbaert and @jonathannorris for all the inputs <3
I don't think the discussion here regarding activation will go anywhere, except for pushing it to the userland entirely.
I will close it with a summarized comment in a bit.
| - key (`String`) | ||
| - Value (`Boolean`, `String`, `Number`, `Object`) | ||
|
|
||
| **Note**: It is worthwhile to expand the `Number` type into more explicit types like `Integer` and `Double`, especially when dealing with SDKs in strongly typed languages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already supported in strongly typed languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove this line soon.
|
my 2 cents is that if there's a way we can achieve this in "userland" (e.g. using "foo:bar" key names with semantics that the provider understands) then we should strongly prefer that over changing/extending the spec |
@moredip: you are very much right in suggesting if we can a solution in the userland for scoped variations/variables like using I like this suggestion. and likely creating a new Featurevisor provider for OpenFeature following this pattern. my response would be the same if I were an OpenFeature maintainer. the only remaining thing for me is to figure out the "activation" event to be triggered at will, which is discussed in above thread. |
|
summary:
really appreciate all the inputs <3 I would have defended the OpenFeature API the same way to not have any further tweaks in the API to accommodate Featurevisor or any other solutions. will take the learnings from here and build the provider at some point. thanks everyone! 🙌 |
This PR
Introduces a new OFEP as per a previous discussion on Slack.
Proposes a new model for features, breaking it down into 3 kinds of values all scoped under a single feature:
Notes
This is being proposed to help open up the possibility to create OpenFeature providers for Featurevisor: https://featurevisor.com/
SDK docs showing examples of the proposed feature model:
Further guides for the concepts introduced:
I understand this proposal can be very different than your overall vision around feature flags, but I feel it's worth raising it to you all because it enables many other tools (both Open Source & SaaS) to create OpenFeature Providers then ❤️