[REFACTOR] Deprecate `sync.GetMetadata` and add equivalent field to `sync.SyncFlags` payload.

[toddbaert]

Requirements

Currently, to support static context enrichment via attributes sent from flagd, in-process providers call sync.GetMetadata along with starting the stream. This is not ideal. The stream isn't valid without the metadata response (since it could result in rules not functioning as expected due to missing attributes) and the metadata response is useless without the stream.

We should simply move the metadata into the stream response as an optional property. This should be done in a non-breaking way.

Requirements:

• [x] update proto
  • deprecate GetMetadata and and a new field to Sync Response (consider sync context as a name, since this field is here to carry additional context attrs (https://github.com/open-feature/flagd-schemas/pull/183)
• update flagd
  • serve the current contents of the GetMetadata payload in this new property in the sync stream
  • add a warning log when the old GetMetadata handler is called saying it's deprecated
  • [x] both done with https://github.com/open-feature/flagd/pull/1642
• update providers to use the new metadata field, but also use the old RPC for now (new field should "win")

cc @cupofcat @aepfli @chrfwow @beeme1mr @alexandraoberaigner

[toddbaert]

@aepfli You also mentioned something about having another field in the SyncFlags RPC... I think it was to make some of the stream logic more isomorphic between the EventStream (RPC mode) and SyncStream (in-process mode), a type field I think... Can you describe that here? We might want to tackle both at the same time. Also, let's try to imagine how it can be done in a non-breaking way so that old providers are compatible with new implementations and vice-versa.

[aepfli]

of course i can elaborate. Currently within the RPC mode we do have the EventStream, which will send on the inital connection a message provider_ready, and for upcoming messages configuration_change. This allows us within the RPC mode to be stateless. As flagd anyways knows when a configuration is established, or when it only informs about updates.

see https://buf.build/open-feature/flagd/docs/main:flagd.evaluation.v1#flagd.evaluation.v1.EventStreamResponse

this would allow us to unify the logic, and keep both modes stateless. Currently we need to know, if we have been connected or not as we need to know, if we need to fire a configuration change or provider ready event.

I hope i could shed some light here, and i hope my motivation regarding this unification is clear

[toddbaert]

of course i can elaborate. Currently within the RPC mode we do have the EventStream, which will send on the inital connection a message provider_ready, and for upcoming messages configuration_change. This allows us within the RPC mode to be stateless. As flagd anyways knows when a configuration is established, or when it only informs about updates.

see https://buf.build/open-feature/flagd/docs/main:flagd.evaluation.v1#flagd.evaluation.v1.EventStreamResponse

this would allow us to unify the logic, and keep both modes stateless. Currently we need to know, if we have been connected or not as we need to know, if we need to fire a configuration change or provider ready event.

I hope i could shed some light here, and i hope my motivation regarding this unification is clear

~Yep, clear to me. I will add this to the description.~

I'm actually going to make this a separate issue.

[aepfli]

I was thinking about this. Testing this is currently a little bit hard. Because the information will most likely be sent on both channels, via the dedicated endpoint and via the sync payload, do we have a feature flag (pun intended) to switch between the two of them, e.g., a temporary config flag? This would allow us to test this behaviour with our test harness.

Maybe it is worth adding such a config flag, wdyt?

[aepfli]

i think we should also implement this in go-sdk-contrib, or am i missing here something