Make EvaluationContext optional when evaluating flags

[gabegorelick]

Requirements

EvaluationContexts can be defined globally, on the client, on the Go context, or at the point of flag evaluation. Currently, the flag evaluation functions in the Go SDK require an EvaluationContext, even if there is already one defined at a higher level.

// Attach an EvaluationContext globally
openfeature.SetEvaluationContext(openfeature.NewEvaluationContext(
    "",
    map[string]interface{}{
        "myGlobalKey":  "myGlobalValue",
    },
))

// Attach an EvaluationContext to the client
client := openfeature.NewClient("my-app")
client.SetEvaluationContext(openfeature.NewEvaluationContext(
    "", 
    map[string]interface{}{
        "myGlobalKey":  "myGlobalValue",
    },
))

// Attach an EvaluationContext to the Go context
ctx := openfeature.WithTransactionContext(context.TODO(), openfeature.EvaluationContext{})

// You still need to pass an evaluation context here, even though we already attached
// evaluation contexts globally, on the client, and on the Go context.
client.Boolean(ctx, "boolFlag", false, openfeature.EvaluationContext{})

[sahidvelji]

Since this is a breaking change, we could consider making the EvaluationContext an Option in v2. Something like WithEvaluationContext.
https://pkg.go.dev/github.com/open-feature/go-sdk/openfeature#Option

See #401

I like this suggestion a lot though. It would make for a cleaner API surface and it would also align with how the Python, Java, and JS SDKs work.

[erka]

Grafana probably would like it. They actually do it like this

if openfeature.NewDefaultClient().Boolean(ctx, featuremgmt.FlagPluginsAutoUpdate, false, openfeature.TransactionContext(ctx)) {
  //..
}

or
https://github.com/grafana/grafana/blob/31f062cb49098b5e43d44e337416a51088a4ede4/pkg/registry/apis/iam/register.go#L236-L241

@dangra Could you describe your usage pattern please?

[dangra]

hi @erka, thanks for the ping but I am not associated with Grafana nor have contributed to it.

That said, not requiring an evaluation context at evaluation call would be rad.
Internally at job, I have seen people passing of.TransactionContext(ctx) but to me it doesn't make sense, that already happens when passing ctx as first argument.

I tend to use an empty context ala of.EvaluationContext{}. I do this so often that have been thinking on using an empty context instance with a short name, ala of.NoCtx (or of.Empty, of.E,...). Ideally make it part of the openfeature namespace but I never come up with a smart and clear short name for it.

[erka]

I am not associated with Grafana nor have contributed to it.

I understand that, and I was just wondering how the OpenFeature Go SDK is used at your company. Thanks for sharing. @dangra

[tangenti]

I don't see the problem of passing an empty evaluation context here.

It doesn't make sense to me to change the interface for a particular use case and potentially break all the existing users.

[erka]

@tangenti I read your message as “you could do it in v2, no problem at all.”

[sahidvelji]

I don't see the problem of passing an empty evaluation context here.

It doesn't make sense to me to change the interface for a particular use case and potentially break all the existing users.

The other SDKs don't require the evaluation context for flag evaluations though, so I would prefer to align with those in v2. Of course we wouldn't change anything in v1.

[tangenti]

@tangenti I read your message as “you could do it in v2, no problem at all.”

I don't see a solid reason to have it in v2 either. It makes the migration hard without bringing much benefits.

[erka]

@leoromanovsky Curious to hear your thoughts on this.

[dangra]

This is a constant itch I keep coming back to. As a user, I would love to see the evaluation context argument made optional in v2. My use cases usually end up with all the needed attributes already in the client and transaction context, so having to pass an empty evaluation context on every call is frustrating.

To make it worse, Copilot flags the empty context as wrong and suggests replacing it with openfeature.TransactionContext(ctx). It is mostly harmless, but it pollutes the codebase.

On the other side, LLM models made porting code across breaking changes a trivial task.