feat!: flagSetMetadata in OFREP/ResolveAll, core refactors

[toddbaert]

⚠️ This PR brings a breaking change to the flagd-core library: the IStore interface now returns an additional value representing the flag set metadata. There are no breaking changes in flagd's behavior.

Changes in flagd:

• returns flag set metadata as metadata for error flags (best effort)
• returns flag set metadata in OFREP and RPC calls
• moves metadata merging logic to evaluator (all flags inherent flag set metadata, but can override, as as before but now reusable in flagd core)
• removes duplicated flag set metadata keys when the same flag set metadata key exists in multiple sources

To Test

• requires curl, grpcurl, and jq

RPC

grpcurl -import-path  /...../schemas/protobuf/flagd/evaluation/v1  -proto evaluation.proto -plaintext  localhost:8013 flagd.evaluation.v1.Service/ResolveAll | jq

OFREP

curl --location 'http://localhost:8016/ofrep/v1/evaluate/flags'  --header 'Content-Type: application/json' --data '{"context": {"color": "yellow"}}'  | jq

Sync

grpcurl -import-path /...../schemas/protobuf/flagd/sync/v1/ -proto sync.proto -plaintext   localhost:8015 flagd.sync.v1.FlagSyncService/FetchAllFlags    | jq -r .flagConfiguration  | jq

[netlify]

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name	Link
🔨 Latest commit	3a44dfc
🔍 Latest deploy log	https://app.netlify.com/sites/polite-licorice-3db33c/deploys/679b85a54677cc00087ad67c

[toddbaert]

The idea of these tests is now moved above (metadata merging).

[beeme1mr]

There are a couple of TODO that should either be removed or actioned. The change looks great and the functionality aligns with that JS implementation. Thanks!

[toddbaert]

ADD/UPDATE are only relevant to hypothetical gRPC server implementations, and were a premature optimization to allow gRPC servers to tell flagd "just add these flags, or just change this flag" instead of sending a whole payload. I have never seen this implemented, and it's not relevant to any of the other sync types.

I think we should cease to enhance them and deprecate them in the protobuf.

[alexandraoberaigner]

Better late then never...I just finished reviewing & added a few questions :)

[alexandraoberaigner]

[Q] When running the evaluation proto ResolveAll rpc command the result does not include the added metadata to the Response message. Is this how it should be or would it make sense to include the metadata as well?
The response looks like this:

{
  "flags": {
    "fibAlgo": {
      "reason": "DEFAULT",
      "variant": "recursive",
      "stringValue": "recursive"
    },
    "headerColor": {
      "reason": "DEFAULT",
      "variant": "red",
      "stringValue": "#FF0000"
    },
    "isColorYellow": {
      "reason": "TARGETING_MATCH",
      "variant": "off",
      "boolValue": false
    },
    "myBoolFlag": {
      "reason": "STATIC",
      "variant": "on",
      "boolValue": true
    },
    "myFloatFlag": {
      "reason": "STATIC",
      "variant": "one",
      "doubleValue": 1.23
    },
    ...

[toddbaert]

I see it when I try - I suspect that the proto files you are pointing to in the command you are running are out of date, and don't have the latest additions of metadata. cd into the schemas repo and pull it. We only very recently added this field to the proto. Sorry - I should have made sure to mention this in the testing instructions.

The proto files are essentially schemas that tell the unmarshaller how to parse the protobuf binary data; they need to know about every field.

[alexandraoberaigner]

Ah, I see, makes sense! Thanks for explaining :)

[alexandraoberaigner]

[Q] did you miss this TODO or should it rather be a normal comment? reading your comment below the TODO would be to deprecate these cases (ADD/UPDATE/DELETE) for sync impls, or did I misunderstand it?

[toddbaert]

You are understanding correctly. I left it as a TODO because I wanted to open an issue to discuss the deprecation of these, which I still intend to do.

[alexandraoberaigner]

[Q] is this the function that relates to this proto command (from evaluation.proto):
rpc ResolveAll(ResolveAllRequest) returns (ResolveAllResponse) {}

[toddbaert]

Yes, this is the inner "domain layer" function satisfying that rpc. More directly, this function is the actual rpc endpoint.

[alexandraoberaigner]

[Q] What does context.TODO() do?

[toddbaert]

context in Go is sort of like a very explicit (most things in Go are very explicit) ThreadLocal in Java, or a continuation in Javascript; it's passed down the entire call chain (at least in cases of good design) and can be used to propagate signals and store data (for example, request data).

context.TODO() is sort of a "bare" or "root" context often used in testing or at the very start of some call stack.

see: https://pkg.go.dev/context

[alexandraoberaigner]

I should finish my Go course 😁 I thought it was an OpenFeature context attribute

[toddbaert]

That's understandable since there's a name collision here. It's not ideal. I try to refer to the OpenFeature one as "EvaluationContext" to disambiguate them.

[alexandraoberaigner]

[Q] What does source represent?

[toddbaert]

It's a "source" of flags. For example, when you start flagd like this:

flagd start -f file:../config/samples/example_flags.flagd.json -f http://some-domain.com/my-flags.json

you have set 2 sources (one file and one http). These provide flag definitions to flagd, and it will watch them for updates. There are many different styles of sources (syncs) in addition to file and http. See: https://flagd.dev/concepts/syncs/ for the full list.