flagr icon indicating copy to clipboard operation
flagr copied to clipboard

Add server-side information to evaluation EntityContext

Open sesquipedalian-dev opened this issue 3 years ago • 2 comments

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 the EntityContext, 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 an int variable.

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.

sesquipedalian-dev avatar Nov 13 '20 19:11 sesquipedalian-dev

Codecov Report

Merging #418 (f4f3b45) into master (c3dc3c0) will decrease coverage by 0.20%. The diff coverage is 62.50%.

Impacted file tree graph

@@            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.

codecov-io avatar Nov 16 '20 23:11 codecov-io

@zhouzhuojie will this be merged anyday ?

lightsaway avatar Aug 02 '21 19:08 lightsaway

Stale pull request message

github-actions[bot] avatar Aug 26 '22 21:08 github-actions[bot]