flagr
flagr copied to clipboard
Add server-side information to evaluation EntityContext
Description
This PR sets up options to add additional properties to the EntityContext
that is used when evaluating which segment on the flag matches the incoming criteria. Each additional property that is added on the server side is prefixed with '@' to differentiate it from the original client context. There are two types added:
- values from the
EvalContext
but aren't available on theEntityContext
, which might be useful or convenient (e.g.@entityID
and@entityType
) - additional properties configured from the server config / environment - the user may provide a JSON map in the env variable
FLAGR_EVAL_SERVER_ENTITY_CONTEXT
to add these additional properties
Motivation and Context
https://github.com/checkr/flagr/issues/322
As mentioned in the issue, one case is where you want to pick different variants based on the entity ID provided; in this case the PR change makes it so you don't have to specify the entityID twice in the request.
I added the additional config properties because of the following use case. We have multiple deployments of flagr (one per 'environment') that use the same flag DB (for configuration management reasons). Our client requests send up an env
property in the EntityContext
, but this means that all our clients have to be configured for that. If the server specifies the env value, we only have to specify it in the server environment rather than in multiple clients.
How Has This Been Tested?
I've only tested it by adding specific go tests so far; I wanted to put up the PR and get feedback.
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)
- [X] Breaking change (fix or feature that would cause existing functionality to change)
- potentially if someone is using
EntityContext
properties with an@
prefix this change could result in different behavior - it also changed the default value of the
FLAGR_RECORDER_KINESIS_AGGREGATE_BATCH_COUNT
config value, because the parser properly validated that the previous default value does not fit into anint
variable.
- potentially if someone is using
Checklist:
- [X] My code follows the code style of this project.
- [X] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [X] I have added tests to cover my changes.
- [X] All new and existing tests passed.
Codecov Report
Merging #418 (f4f3b45) into master (c3dc3c0) will decrease coverage by
0.20%
. The diff coverage is62.50%
.
@@ Coverage Diff @@
## master #418 +/- ##
==========================================
- Coverage 81.75% 81.54% -0.21%
==========================================
Files 27 27
Lines 1611 1626 +15
==========================================
+ Hits 1317 1326 +9
- Misses 214 217 +3
- Partials 80 83 +3
Impacted Files | Coverage Δ | |
---|---|---|
pkg/config/config.go | 75.34% <60.00%> (-2.79%) |
:arrow_down: |
pkg/handler/eval.go | 85.41% <66.66%> (-0.61%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update c3dc3c0...f4f3b45. Read the comment docs.
@zhouzhuojie will this be merged anyday ?
Stale pull request message