open-feature / spec

OpenFeature specification
https://openfeature.dev
Apache License 2.0
639 stars 34 forks source link

Add details, "Context" to "Evaluation Context" #34

Closed toddbaert closed 2 years ago

toddbaert commented 2 years ago

As mentioned in the meeting, this avoids some overload and logical terminology clashes, both within OpenFeature (hookContext.context) and without, and I believe maps more specifically to the feature-flags use-case than "context".

Also added a blurb about intent to use implicit state storage mechanisms to propagate attributes.

moredip commented 2 years ago

Personally I prefer the word context, although I do agree that there are some naming conflicts here - context is on of those words that gets used frequently.

I like context because it captures a bit more of the semantics - which path to go down at run time is a decision that can vary at runtime depending on the context. "Attributes" has less semantic meaning for me. Also these values are usually pulled from a request context of some kind (at least for server-side flags), so there is a nice conceptual parallel there.

toddbaert commented 2 years ago

Personally I prefer the word context, although I do agree that there are some naming conflicts here - context is on of those words that gets used frequently.

I like context because it captures a bit more of the semantics - which path to go down at run time is a decision that can vary at runtime depending on the context. "Attributes" has less semantic meaning for me. Also these values are usually pulled from a request context of some kind (at least for server-side flags), so there is a nice conceptual parallel there.

@moredip I see all of your points. I think I could personally stick with "context", but if we do, we should rename the HookContext data structure. I think having 2 structures/concepts with "context" in their name is probably going to cause confusion. We'll also have to specifically look out for golang which features the Context package as part of its standard library. Making things a bit worse, we'd probably need to use this package to propagate state (the OpenFeature context) in the golang SDK as an equivalent to async-local-storage in node or thread-local-storage in Java... so we'd be putting our Context in the Context :sweat_smile:

I don't think this is a deal-breaker, just something to note.

moredip commented 2 years ago

Perhaps we could disambiguate a bit with a more specific name: FlagDecisionContext or FlagEvaluationContext or similar

toddbaert commented 2 years ago

Perhaps we could disambiguate a bit with a more specific name: FlagDecisionContext or FlagEvaluationContext or similar

@moredip I force-pushed an update. I've changed Context to Evaluation Context. We use EvaluationDetails as well in a few other places in the SDK research typings - so I think in general, it seems "Evaluation" is a good way to denote objects related to, and impacting the evaluation of flags.

I also think HookContext is pretty distinct from EvaluationContext, so it largely clears that up.

Let me know your thoughts... of course we could go with FlagEvaluationContext but I think if we always include "Flag" it's going to feel redundant.