feat: add context value flag #1448

alemrtv · 2024-11-10T18:34:49Z

add the --context-value command line flag to pass arbitrary key value pairs to the evaluation context

Follow-up Tasks

adjust the integration test to include this flag

How to test

issue the flagd start command with the -X flag and provide key value pairs like:
flagd start -X name=jane -X age=31 or flagd start --context-value name=jane --context-value age=31

Signed-off-by: Aleksei Muratov <[email protected]>

…ure#1442) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [google.golang.org/grpc](https://redirect.github.com/grpc/grpc-go) | `v1.67.1` -> `v1.68.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/google.golang.org%2fgrpc/v1.68.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/google.golang.org%2fgrpc/v1.68.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/google.golang.org%2fgrpc/v1.67.1/v1.68.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/google.golang.org%2fgrpc/v1.67.1/v1.68.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>grpc/grpc-go (google.golang.org/grpc)</summary> ### [`v1.68.0`](https://redirect.github.com/grpc/grpc-go/releases/tag/v1.68.0): Release 1.68.0 [Compare Source](https://redirect.github.com/grpc/grpc-go/compare/v1.67.1...v1.68.0) ### Behavior Changes - stats/opentelemetry/csm: Get mesh_id local label from "CSM_MESH_ID" environment variable, rather than parsing from bootstrap file ([#7740](https://redirect.github.com/grpc/grpc-go/issues/7740)) - orca (experimental): if using an ORCA listener, it must now be registered only on a READY SubConn, and the listener will automatically be stopped when the connection is lost. ([#7663](https://redirect.github.com/grpc/grpc-go/issues/7663)) - client: `ClientConn.Close()` now closes transports simultaneously and waits for transports to be closed before returning. ([#7666](https://redirect.github.com/grpc/grpc-go/issues/7666)) - credentials: TLS credentials created via `NewTLS` that use `tls.Config.GetConfigForClient` will now have CipherSuites, supported TLS versions and ALPN configured automatically. These were previously only set for configs not using the `GetConfigForClient` option. ([#7709](https://redirect.github.com/grpc/grpc-go/issues/7709)) ### Bug Fixes - transport: prevent deadlock in client transport shutdown when writing the GOAWAY frame hangs. ([#7662](https://redirect.github.com/grpc/grpc-go/issues/7662)) - mem: reuse buffers more accurately by using slice capacity instead of length ([#7702](https://redirect.github.com/grpc/grpc-go/issues/7702)) - Special Thanks: [@PapaCharlie](https://redirect.github.com/PapaCharlie) - status: Fix regression caused by [#6919](https://redirect.github.com/grpc/grpc-go/issues/6919) in status.Details() causing it to return a wrapped type when getting proto messages generated with protoc-gen-go < v1. - Special Thanks: [@Clement-Jean](https://redirect.github.com/Clement-Jean) ### Dependencies - Bump minimum supported Go version to `go1.22.7`. ([#7624](https://redirect.github.com/grpc/grpc-go/issues/7624)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/open-feature/flagd).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

…#1443) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | golang.org/x/crypto | `v0.28.0` -> `v0.29.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/golang.org%2fx%2fcrypto/v0.29.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/golang.org%2fx%2fcrypto/v0.29.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/golang.org%2fx%2fcrypto/v0.28.0/v0.29.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/golang.org%2fx%2fcrypto/v0.28.0/v0.29.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/open-feature/flagd).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | golang.org/x/mod | `v0.21.0` -> `v0.22.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/golang.org%2fx%2fmod/v0.22.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/golang.org%2fx%2fmod/v0.22.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/golang.org%2fx%2fmod/v0.21.0/v0.22.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/golang.org%2fx%2fmod/v0.21.0/v0.22.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/open-feature/flagd).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | golang.org/x/sync | `v0.8.0` -> `v0.9.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/golang.org%2fx%2fsync/v0.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/golang.org%2fx%2fsync/v0.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/golang.org%2fx%2fsync/v0.8.0/v0.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/golang.org%2fx%2fsync/v0.8.0/v0.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/open-feature/flagd).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | golang.org/x/net | `v0.30.0` -> `v0.31.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/golang.org%2fx%2fnet/v0.31.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/golang.org%2fx%2fnet/v0.31.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/golang.org%2fx%2fnet/v0.30.0/v0.31.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/golang.org%2fx%2fnet/v0.30.0/v0.31.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/open-feature/flagd).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Signed-off-by: Aleksei Muratov <[email protected]>

netlify · 2024-11-10T18:35:09Z

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

Name	Link
🔨 Latest commit	`ea7c818`
🔍 Latest deploy log	https://app.netlify.com/sites/polite-licorice-3db33c/deploys/6748cbe594f8460008d93422

beeme1mr · 2024-11-21T19:42:03Z

Hey @alemrtv, sorry for the delay on the review. I'll take a look as soon as possible.

Signed-off-by: Aleksei Muratov <[email protected]>

aepfli · 2024-11-27T06:49:33Z

flagd/pkg/service/flag-evaluation/flag_evaluator_v2.go

@@ -167,13 +173,22 @@ func (s *FlagEvaluationService) ResolveBoolean(
 ) (*connect.Response[evalV1.ResolveBooleanResponse], error) {
 	sCtx, span := s.flagEvalTracer.Start(ctx, "resolveBoolean", trace.WithSpanKind(trace.SpanKindServer))
 	defer span.End()
+
+	evalCtx := map[string]any{}
+	if e := req.Msg.GetContext(); e != nil {


The context provided during the evaluation (from the client) has a higher priority than the one provided via config flags (aka the consumer wins if there are conflicts). Hence we should add the req.Msg.GetContext() values to the s.contextValues. Wdyt?

aepfli

Thank you. Those changes look great from my perspective. I added a few remarks for ease of maintenance, which would also reduce the size of this PR, and the lines changed.

I don't have reviewer power here ;( but those are really good changes and improvements; thank you.

aepfli · 2024-11-27T06:51:25Z

flagd/pkg/service/flag-evaluation/flag_evaluator.go

+	evalCtx := map[string]any{}
+	if e := req.Msg.GetContext(); e != nil {
+		evalCtx = e.AsMap()
+	}
+	for k, v := range s.contextValues {
+		evalCtx[k] = v
+	}


This code block exists multiple times; changing how we populate the context means we must touch several parts of the code. We should extract this into our method for easier maintenance and effortless changes.

Furthermore, I think it is worth moving this code block into the resolve method and handling it once before resolving the flag.

aepfli · 2024-11-27T06:55:50Z

flagd/pkg/service/flag-evaluation/ofrep/handler.go

 	context := map[string]any{}
 	if res, ok := request.Context.(map[string]any); ok {
 		context = res
 	} else {
 		log.WarnWithID(requestID, "provided context does not comply with flagd, continuing ignoring the context")
 	}

+	for k, v := range contextValues {


same topic regarding prioritization

flagd/cmd/start.go

Signed-off-by: Aleksei Muratov <[email protected]>

aepfli · 2024-11-28T22:44:55Z

flagd/pkg/service/flag-evaluation/flag_evaluator.go

 // resolve is a generic flag resolver
 func resolve[T constraints](ctx context.Context, logger *logger.Logger, resolver resolverSignature[T], flagKey string,
 	evaluationContext *structpb.Struct, resp response[T], metrics telemetry.IMetricsRecorder,
+	configContextValues map[string]any,


i do not think that we need here another value, we are merging the values with the context already on line https://github.com/open-feature/flagd/pull/1448/files#diff-65826bb36f80a7e210128a2d3493c29be6f6c6f9487feae242eccbbc61c74c4fR75 - can we simply provide the result of the other line as evaluationcontext. like

evalCtx := map[string]any{} if e := req.Msg.GetContext(); e != nil { evalCtx = e.AsMap() } // add values to evalCtx here (deleted code from link) and pass only the evalCtx with all the information values, err := s.eval.ResolveAllValues(sCtx, reqID, evalCtx)

Or access s.contextValues within this method and merge in here. But i do not see the need for an additional mergeContext, seems like we are doing this twice now.

Sorry, but I'm confused. To me it looks like there're 2 different use cases: ResolveAll and the generic resolve. And these don't seem to be called at the same time. So there're two places where we need to handle it.

My bad, you are totally right sorry

toddbaert · 2024-11-29T02:51:29Z

Hey @alemrtv ! Sorry for the delayed review... Many of us were away with Kubecon and still playing catch up. I'll review this next week, latest. 🙏

alemrtv and others added 11 commits November 10, 2024 19:12

feat: add context-value flag

7d0889a

Signed-off-by: Aleksei Muratov <[email protected]>

fix: fmt issues

1ff4023

Signed-off-by: Aleksei Muratov <[email protected]>

feat: add a unit test

b5c0acb

Signed-off-by: Aleksei Muratov <[email protected]>

Merge branch 'open-feature:main' into feat/add-context-value-flag

fa5e7ec

feat: add docs

4145161

Signed-off-by: Aleksei Muratov <[email protected]>

alemrtv requested a review from a team as a code owner November 10, 2024 18:34

dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Nov 10, 2024

alemrtv changed the title ~~Feat/add context value flag~~ feat: add context value flag Nov 10, 2024

fix: linter issues

f0df97b

Signed-off-by: Aleksei Muratov <[email protected]>

alemrtv force-pushed the feat/add-context-value-flag branch from 7f23729 to f0df97b Compare November 23, 2024 10:46

Merge branch 'main' into feat/add-context-value-flag

bd166e8

aepfli reviewed Nov 27, 2024

View reviewed changes

aepfli requested review from chrfwow, warber, alexandraoberaigner and toddbaert November 27, 2024 07:13

Merge branch 'main' into feat/add-context-value-flag

326e384

beeme1mr reviewed Nov 27, 2024

View reviewed changes

flagd/cmd/start.go Outdated Show resolved Hide resolved

feat: simplify code

7725996

Signed-off-by: Aleksei Muratov <[email protected]>

alemrtv force-pushed the feat/add-context-value-flag branch from dfc764d to 7725996 Compare November 28, 2024 19:52

feat: simplify code

ea7c818

Signed-off-by: Aleksei Muratov <[email protected]>

alemrtv requested review from aepfli and beeme1mr November 28, 2024 20:09

aepfli reviewed Nov 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add context value flag #1448

feat: add context value flag #1448

alemrtv commented Nov 10, 2024 •

edited

Loading

netlify bot commented Nov 10, 2024 •

edited

Loading

beeme1mr commented Nov 21, 2024

aepfli Nov 27, 2024

aepfli left a comment •

edited

Loading

aepfli Nov 27, 2024

aepfli Nov 27, 2024

aepfli Nov 28, 2024

alemrtv Nov 29, 2024

aepfli Nov 29, 2024

toddbaert commented Nov 29, 2024

feat: add context value flag #1448

Are you sure you want to change the base?

feat: add context value flag #1448

Conversation

alemrtv commented Nov 10, 2024 • edited Loading

Follow-up Tasks

How to test

netlify bot commented Nov 10, 2024 • edited Loading

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

beeme1mr commented Nov 21, 2024

aepfli Nov 27, 2024

Choose a reason for hiding this comment

aepfli left a comment • edited Loading

Choose a reason for hiding this comment

aepfli Nov 27, 2024

Choose a reason for hiding this comment

aepfli Nov 27, 2024

Choose a reason for hiding this comment

aepfli Nov 28, 2024

Choose a reason for hiding this comment

alemrtv Nov 29, 2024

Choose a reason for hiding this comment

aepfli Nov 29, 2024

Choose a reason for hiding this comment

toddbaert commented Nov 29, 2024

alemrtv commented Nov 10, 2024 •

edited

Loading

netlify bot commented Nov 10, 2024 •

edited

Loading

aepfli left a comment •

edited

Loading