open-feature / flagd

A feature flag daemon with a Unix philosophy
https://openfeature.dev
Apache License 2.0
564 stars 67 forks source link

[FEATURE] Support addition of arbitrary properties in evaluation context #1435

Open toddbaert opened 3 weeks ago

toddbaert commented 3 weeks ago

Requirements

Currently, flagd injects a couple standard properties into the evaluation context ($flagd.flagKey, $flagd.timestamp).

In addition, we want to support the addition of ANY arbitrary string properties specified on the command line. The command line should look something like this:

flagd start -X name=jane -X age=31 ... or flagd start --context-value name=jane --context-value age=31 ...

The above example will add the key/value pairs "name": "jane" and "age": "31" to the evaluation context for every evaluation.

Note that values are all strings - if desired, JSONLogic can be used in rules to convert them to other types. Also note that there's no prefix associated with these additional fields.

In addition to adding these values to the evaluation context for all evaluations done in the flagd daemon itself, flagd should return these values in the sync-metadata call, so that in-process providers can add these values to their evaluations as well.

alemrtv commented 3 weeks ago

hi @toddbaert, I'd like to try working on this one, could you assign it to me please?

toddbaert commented 3 weeks ago

Yes, absolutely! Assigned.

alemrtv commented 3 weeks ago

@toddbaert

  1. the code is mostly ready, but how would you recommend to test these changes? Currently, the tests already use the context during the flag evaluation in flagd/core/pkg/evaluator/json_test.go. I would need to test it from a much higher level, though, to make sure that the values added by the new flag are passed to Resolve...Value functions. The higher level tests, however, mock the lower level layers: example. Do I need to add some kind of an end-2-end test?

  2. Would you like me to describe the changes in the README.md in the root directory of the project or somewhere else?

toddbaert commented 2 weeks ago
  1. I would add a very basic unit test that asserts the existence of some arbitrary new properties in the context. It might not cover things "end to end" but I suspect your new code has some method with new parameters somewhere now? I would just make sure that if those are set to include some additional props, those props are present; also make sure the sync-metadata RPC is returning them. We have a very robust e2e gherkin test suite: https://github.com/open-feature/flagd-testbed/tree/main/gherkin where we should add this eventually, but that will have to be a separate task.

  2. Good question... after thinking more, I think it'd be best if you added a small section here. Also note the command line args are self-documenting and get injected into the docsite in that section if you do make generate-docs. You can also run the web docs with make run-web-docs. I don't expect you to necessarily understand all the background here, so I'll probably add a bit to your docs; but feel free to take a pass at it! 🙏

alemrtv commented 2 weeks ago

@toddbaert the PR is ready