feat: add flag metadata field

[james-milligan]

This PR

• implement flag metadata: open-feature/spec@74c373e

Related Issues

#177

Notes

If the flag metadata is not in the provider response, then the returned evaluation details will have a nil value, this can be updated to have an empty map, but IMO its a cleaner implementation to leave this value as nil (nil slices have length 0)

Follow-up Tasks

How to test

[codecov]

Codecov Report

Merging #178 (5752267) into main (e0f37d5) will increase coverage by 1.21%.
The diff coverage is 87.27%.

❗ Current head 5752267 differs from pull request most recent head 492f3e6. Consider uploading reports for the commit 492f3e6 to get more accurate results

@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
+ Coverage   71.25%   72.47%   +1.21%     
==========================================
  Files           8        8              
  Lines         668      723      +55     
==========================================
+ Hits          476      524      +48     
- Misses        174      180       +6     
- Partials       18       19       +1     

Impacted Files	Coverage Δ
pkg/openfeature/client.go	73.24% <86.00%> (+1.38%)	⬆️
pkg/openfeature/provider.go	100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[toddbaert]

This all makes sense to me.

I'm particularly interested in other's opinions on this:

If the flag metadata is not in the provider response, then the returned evaluation details will have a nil value, this can be updated to have an empty map, but IMO its a cleaner implementation to leave this value as nil (nil slices have length 0)

I'll defer to those with more familiarity with go semantics.

[Kavindu-Dodan]

This all makes sense to me.

I'm particularly interested in other's opinions on this:

If the flag metadata is not in the provider response, then the returned evaluation details will have a nil value, this can be updated to have an empty map, but IMO its a cleaner implementation to leave this value as nil (nil slices have length 0)

I'll defer to those with more familiarity with go semantics.

From pure spec point of view [1]

If the flag metadata field in the flag resolution structure returned by the configured provider is set, the evaluation details structure's flag metadata field MUST contain that value. Otherwise, it MUST contain an empty record.

IMO, to be spec compliant, we should have an empty record here 🤔

[1] - https://openfeature.dev/specification/sections/flag-evaluation/#requirement-1413

[beeme1mr]

How do we want to handle releasing a breaking change? If we're going to release this as a minor version, we need to make sure Release Please is configured properly and that the changes are clearly documented.

[toddbaert]

How do we want to handle releasing a breaking change? If we're going to release this as a minor version, we need to make sure Release Please is configured properly and that the changes are clearly documented.

I didn't notice the ! in the title. Is this actually breaking? I assumed from:

If the flag metadata is not in the provider response

That this was non-breaking? @james-milligan - can you clarify?

We can officially, break the provider contract and not consider it a major change because providers are still in hardening, but I'd like to exhaust all possible options before doing this. Alternatively, releasing a v2 would be painful, I think, based on previous experiments.

[james-milligan]

yes this is non breaking, apologies should have caught this sooner!

[toddbaert]

I've updated the title so it reads a bit better in release notes. Approved!