Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Flag Evaluation Wire Protocol #67

Closed
wants to merge 4 commits into from
Closed

Conversation

beeme1mr
Copy link
Member

This PR

  • adds a flag evaluation wire protocol OFEP proposal

Related Issues

Fixes #65

Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
Copy link
Member Author

This OFEP potentially addresses @tcarrio pluggable flagd OFEP in an alternative way.

@beeme1mr beeme1mr changed the title add flag evaluation wire protocol ofep Flag Evaluation Wire Protocol Jun 27, 2023
Comment on lines +50 to +54
- OpenTelemetry demo application
- Jenkins
- Quarkus
- https://github.com/quarkusio/quarkus/issues/28032
- Dapr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oleg-nenashev @james-milligan if anyone has relevant discussions in other projects, we can link discussions here

@toddbaert toddbaert self-requested a review June 27, 2023 19:30
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a compelling idea.

Defining such a wire protocol would mean applications could include vendor-neutral feature flags in a manner that "feels" similar to exposing Prometheus metrics, or the like.

@beeme1mr beeme1mr requested review from thisthat and toddbaert June 27, 2023 20:30
Copy link
Member

@thomaspoignant thomaspoignant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nice 👌
My guess is that it will ease the adoption of Open-Feature for new vendors interested by the project.

OFEP-flag-eval-wire-protocol.md Show resolved Hide resolved
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr beeme1mr requested review from a team June 28, 2023 13:17
Copy link
Member

@moredip moredip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really not sold on defining this protocol as part of OpenFeature. I think it distracts from the main focus of OF - the spec and SDK - and I'm skeptical that it will get vendor support.

I'm not sure why we wouldn't just use the existing flagd protocol.

Many feature flag vendors offer SDKs that perform flag evaluation in-process.
This provides fast and consistent flag evaluations with no additional deployment complexities.
Remote evaluation logic can be written in a single language and be used interchangeably with any system complying with the wire protocol.
That makes it ideal for open source projects that need a vendor-agnostic solution for feature flags.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes it ideal for open source projects that need a vendor-agnostic solution for feature flags.

This doesn't follow to me. A big part of the OpenFeature value prop is in providing a vendor-neutral (in-process) SDK, one which an open-source project can plug into without any awareness of what providers are plugged into that SDK.

Copy link
Member

@thomaspoignant thomaspoignant Jun 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem that @oleg-nenashev reported with the current approach, is that it is hard for opensource projects to support all providers out of the box or to provide a plugin system for people to use a specific provider.

In some languages, we can do it easily (such as Java), but for some others such as Go it can be complex and will force these open-source projects to create a plugin system specific to open-feature to be able to support an unknown list of providers.

With a generic provider, you can support directly all systems that offer the server implementation of the generic provider.

As an alternative to creating a new proto schema, we could reuse the existing flagd schema.
This schema already complies with the OpenFeature specification and provides a working end-to-end implementation.

The drawbacks to this alternative are:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't use the existing flagd protocol then IMO we should plan on having flagd implement this new protocol. Delivering a protocol with no reference implementation doesn't seem great.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the goal, flagd will stop using the actual protocol and move to this new one.

On my side, I also plan to use the protocol for thomaspoignant/go-feature-flag, to have a 2nd implementation.

@toddbaert
Copy link
Member

toddbaert commented Jun 28, 2023

I'm really not sold on defining this protocol as part of OpenFeature. I think it distracts from the main focus of OF - the spec and SDK - and I'm skeptical that it will get vendor support.

I'm not sure why we wouldn't just use the existing flagd protocol.

@moredip

I understand this perspective and shared it up until last week, but I think @oleg-nenashev in this thread helped change my view. As @thomaspoignant says here, there are limitations with the SDK+provider model. It works fantastic for first party apps that developers are interested in adding feature flag support to. It works less-well with self-hosted "off-the-shelf" applications (think Jenkins, Grafana, Keycloak Server) that could reasonably define sets of flags to be administered by whoever hosts them. Requiring these apps to maintain tens of plugins (providers) seems like a non-starter, compared to the relatively easy solution of allowing them to conform to this evaluation protocol, which could be made standard.

Practically, there's no actually that much work for us here. The flagd schema largely mirrors the SDK primitives. It could be made more neutral and generic, along the flagd providers, and we could ship both as "OpenFeature RPC" components, which anyone could build compliant backends for (also made easier by gRPC server generation).

@Kavindu-Dodan
Copy link
Contributor

I think this is a good initiative and reduced complexity when OF needs to be adopted by other systems. And I tried to visualize how this would fit into our SDKs and given below is my understanding visualization in a diagram,

flowchart TD
    A[SDK] --> CTB(SDK Contrib)
    CTB --> VA(Vendor A) --> Backend(Vendor Server)
    CTB --> VB(Vendor B) --> ....
    A--> W(Wire Protocol)
    W --> IMPLA(Server A)
    W --> PR(Proxy A)
Loading

Left side (in-process)

  • Current SDK relly on SDK Contrib to connect with the actual evaluation of the feature flag
  • SDK Contib layer is sort of a trasformer. It transforms vendor-specific feature flags (most of the time) coming from vendor backend
  • Can the current SDK to SDK Contrib bound solution adopted by a generic solution (Jenkins, K8s) ? Yes, but then that solution needs to expose configurations to setup SDK Contib dependencies

Right side (wire protocol)

  • Wire protocol is standardized, hence SDK will have built-in support
  • Wire protocol directly connects to vendors or their proxy deployments
  • Adoptions consume flag evaluations exposed from language-specific SDK based on the wire protocol

Besides, I think both approaches will live side-by-side through OF SDK as they have their own benefits.


## Action items

- Review the [existing proto schema](https://github.com/open-feature/schemas/blob/main/protobuf/schema/v1/schema.proto) to identify ways to make it provider agnostic.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing the @oleg-nenashev mention is to have also an OpenAPI version of the wire protocol.

Is it something we want to provide also?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My immediate reaction to gRPC only was somewhat negative, since this restricts implementations somewhat. Even though clients can get flexibility with Web gRPC it's not shared in the server-side.

As a wire protocol I'm not entirely opposed.

But I would be interested in seeing OpenAPI definitions as part of this as well, unless there are significant issues with that approach.

@moredip
Copy link
Member

moredip commented Jul 3, 2023

Just to re-iterate, I can (sort of) see the value of this for that open source/platform use case where the product that's having its feature flags managed might want to support multiple providers. However, I'm not a fan of adding this protocol as a core part of OpenFeature.

Firstly, I believe the use case is really a bit of a corner case. Is it really that important that a platform like Jenkins allows its internal feature flags to be managed by a variety of feature flagging frameworks? If you're just a consumer of the platform then the feature flag ruleset is really just going to be another type of configuration - you're not going to need dynamic evaluation or anything like that. It seems much more likely that Jenkins would just pick a specific fixed provider and ask its users to use that framework if they want to manage the flags.

Even if Jenkins could support multiple providers via this new capability, it would still add a bunch of complexity. Jenkins maintainers would need to provide an ongoing list of feature flag rulesets ("as of this release, flag foo_bar should be on by default, and there's also a new flag baz_chirp that should be off for now") AND they'd now be requiring their consumers to run a new standalone process for flagging (very likely flagd).

I don't know why we wouldn't just encourage a platform like Jenkins to just use flagd, and that's the end of it.

Practically, there's no actually that much work for us here. The flagd schema largely mirrors the SDK primitives. It could be made more neutral and generic, along the flagd providers, and we could ship both as "OpenFeature RPC" components, which anyone could build compliant backends for (also made easier by gRPC server generation).

agree it's not much work - really just ratifying some of the flagd work - but there's also an ongoing carrying cost, and more importantly it broadens the entire scope of the OpenFeature to include this wire protocol, which is more confusing for new people coming to OpenFeature.

@Kavindu-Dodan
Copy link
Contributor

If you're just a consumer of the platform then the feature flag ruleset is really just going to be another type of configuration - you're not going to need dynamic evaluation or anything like that

I see this differently. Configurations are a good start and this is the minimal setup of feature flags. But if we think about platform integrations, there is a need for dynamic configurability. This is the missing piece I see wire protocol can fulfill

It seems much more likely that Jenkins would just pick a specific fixed provider and ask its users to use that framework if they want to manage the flags

Wire protocol will avoid vendor locking, which is favorable for commercial/open-source platforms. Plus, this gives flexibility to adoptions so that they can use any feature flag vendor that supports the standard (more or less similar to OF SDKs)

AND they'd now be requiring their consumers to run a new standalone process for flagging (very likely flagd)

I don't think this is mandatory for everyone. Default values will be the standard operating behavior. Flagging service will be only needed for advanced use cases such as platform offerings (ex:- Jenkins as a service, K8s as a service). Since wire protocol is standardized, this can be a custom or vendor-backed implementation.

... broadens the entire scope of the OpenFeature to include this wire protocol, which is more confusing for new people coming to OpenFeature

Agree on the complexity - we will have to maintain the wire protocol and provide a reference implementation(flagd). But I don't think this will confuse newcomers to feature flagging. SDK will continue to support conventional in-process evaluations [1]. Rather than confusing, I think this will enable more use cases.

[1] - #67 (comment)

@toddbaert
Copy link
Member

I don't believe we need 100% consensus in all things, however @moredip has been key in the project since it's inception, and this does represent a sort of new direction for OpenFeature.

@oleg-nenashev Do you have any thoughts in response to @moredip 's comments here? And @Kavindu-Dodan 's responses here? I think these are both full of good points.

@Kavindu-Dodan
Copy link
Contributor

Kavindu-Dodan commented Jul 6, 2023

Some points worth cross-referencing from the community meeting on 6th July 2023 [1] [2]

  • Using flagd itself as the proxy

flagd at the current state is a proxy exposing flags from different data sources. And this will require flagd to be a core component of each SDK and write convertors from other providers to flagd

  • caching & security

Client libraries heavily rely on caching to give flag evaluation performance. Wire protocol may have potential performance issues as it requires a proxy/server request. Further, caching must not be a concern of wire protocol implementation as it could lead to other issues, hence it should be minimal.

Besides, we might also need to consider security for the SDK to proxy/server communication. This could be outsourced to a proxy but may be protocol will need to define a recommendation.

[1] - https://docs.google.com/document/d/1pp6t2giTcdEdVAri_2B1Z6Mv8mHhvtZT1AmkPV9K7xQ/edit#bookmark=id.9ltvj8qmp30v
[2] - https://docs.google.com/document/d/1pp6t2giTcdEdVAri_2B1Z6Mv8mHhvtZT1AmkPV9K7xQ/edit#bookmark=id.le89k8eo7mrq

@justinabrahms
Copy link
Member

Sorry for my delay. I'm getting lost on this proposal w/r/t jenkins and how they need it. It seems like we're talking about replacing jenkins' existing configuration mechanism with open feature. I'm not convinced that's a targeted use-case.

That said, what the proposal is reading to me as is: "Hey, it sucks to have to write N different providers where N is the number of languages that I, $vendor, would like to support." Instead, we're offering them a simpler "implement this interface over http and we'll support you in basically every language that we write SDKs in. That feels like a valuable thing. Thoughts @moredip ?

@beeme1mr
Copy link
Member Author

Hey @justinabrahms, yes, I agree that's the immediate value. At the very least, it would consolidate the dev effort for Go Feature Flagd and flagd providers.

I included the Jenkins parts because @oleg-nenashev, a Jenkins maintainer, expressed interest in this functionality.

@oleg-nenashev
Copy link
Member

I am yet to review it in details, but I definitely express interest. Less for Jenkins, more for the ecosystem of small Golang/Rust projects that need to bundle something vendor neutral


## State: DRAFTING

Create a wire protocol for RPC-based flag evaluation compliant with the OpenFeature specification.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not discussed yet, that I see.. why RPC (vs REST)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could loosen the requirement to not specify rpc. The important part to me is to have a single source of truth like a protobuf or OpenAPI that can be used to generate clients. It's also important to have support for bi-directional communication for events and caching.

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in next 60 days.

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in next 60 days.

@github-actions github-actions bot added the Stale label Oct 14, 2023
@beeme1mr beeme1mr removed the Stale label Nov 20, 2023
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in next 60 days.

@github-actions github-actions bot added the Stale label Dec 21, 2023
@github-actions github-actions bot removed the Stale label Jan 5, 2024
Copy link

github-actions bot commented Feb 4, 2024

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in next 60 days.

@github-actions github-actions bot added the Stale label Feb 4, 2024
@github-actions github-actions bot removed the Stale label Feb 23, 2024
@toddbaert
Copy link
Member

toddbaert commented Feb 26, 2024

@beeme1mr should we close this with the addition of the OFREP to the spec repo and it's own repo? Is the OFEP still needed for record-keeping purposes you think?

I think https://github.com/open-feature/protocol/blob/main/README.md may accomplish the same goals 🤷

@thomaspoignant
Copy link
Member

I think we can close this one, since open-feature/protocol is the place where things are happening.

To follow the evolution of the project please follow to the protocol repository.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] Generic GRPC Provider