go-kit / kit

A standard library for microservices.
https://gokit.io
MIT License
26.51k stars 2.43k forks source link

Proposal: `flags` package for abstract usage of Feature Flags #684

Closed nelz9999 closed 3 years ago

nelz9999 commented 6 years ago

Much like the metrics package, I believe Feature Flags (/Toggles/etc...) should be handled similarly:

~Metrics~ Feature Flags are dependencies, and should be passed to the components that need them in the same way you'd construct and pass a database handle, or reference to another component.

Would the project be open to receiving a contribution of a flags package, contained in it a couple of helper and concrete implementation packages: default (always returns the same value); random (randomly delivers results from a discrete list of options); launchdarkly (because this the service I use in my own projects).

The set of interfaces would probably be a bit wider than the 3 main interfaces in metrics, maybe something akin to this:

This would standardize on using context.Context to hold any targeting information. (E.g. LaunchDarkly uses their own User struct for deciding on which option to return.)

peterbourgon commented 6 years ago

I'm not totally convinced that this is a common-enough pattern to warrant inclusion in Go kit. But I'm subject to my own biases. Would other users care to chime in?

yurishkuro commented 6 years ago

Also, passing dependencies via context.Context is explicitly an anti-pattern. Context is meant for transaction-scoped attributes.

nelz9999 commented 6 years ago

@yurishkuro Hrm... If I'm interpreting what you mean by "transaction-scoped", I believe that's what this is using it for...

In the context of a web request (ACK that this might be used in non-HTTP usages), I believe the process would be pull whatever you need off the incoming request to identify the requesting user, build the request-scoped datum (in the LD example, it'd be one of their User structs), and put that in the Context... The non-request-scoped stuff is injected much earlier at construction time (feature flag key, error handling, client, etc.) of the feature flag (e.g. Booler), and you hand the Booler around as a dependency to the structs that need to consume them and operate conditionally.

yurishkuro commented 6 years ago

perhaps I am not understanding what you're proposing. If the application's handler extracts app-specific information from the request (like that User struct), it can certainly store it in the context, but that does not require any additional framework. Neither the representation of that data nor its extraction from the request can be generalized since it's specific to the application. So what would these OptString(context.Context) string methods return and who would call them?

nelz9999 commented 6 years ago

The intention is for the interfaces to express "If you need vendor-specific information to calculate the flag results, the established mechanism to send that information is via the Context."

In the OptString (or Stringer in the PR), the vendor-specific code takes the request object, talks to the vendor-specific client with the request object and construction-time parameters, and then calculates which "variation" is returned for this request.

Maybe a concrete(-ish) example will help?