This PR:
* stabilizes evaluation API
* stabilizes providers
* hardens hooks, events and context
* hardens all "static context" (client-side) points
This is a pretty substantial step.
`stable` means that we have no more leeway at all in SDKs - **any
breaking changes to the parts of the SDK implementing stable APIs means
a new major version of the SDK is required, without exception**.
I've also opted to "harden" the `static context paradigm` since we have
numerous client implementations in the wild, in particular React and
Angular, but also Android. There could still be breaking changes here
but the bar is higher. I think this is how we're acting already with
regards to these APIs, in all honesty.
I did not promote the [context
propagation](https://openfeature.dev/specification/sections/evaluation-context#33-context-propagation)
sub-section, or the new tracking section - these are _relatively_ new,
though I'm open to opinions on hardening these further.
Potential blockers:
- https://github.com/open-feature/go-sdk/issues/389
- https://github.com/open-feature/spec/issues/270
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
it is hard not to diverge between a cached and a stable response - so i
added an additional tag to separate both cases
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Adding newer evaluation tests, in the usual style. Additionally i am
adding a json file with test data, so we do not need to manually create
it all the time, but could use a json parser
---------
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
## This PR
- clarify evaluation context in hook requirements
### Related Issues
Fixes#328
### Notes
I added the clarification in the non-normative section. Hopefully, this
addresses the ambiguity around what we mean by `evaluation context`.
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
This is a change that I suspect will have little practical impact for
most users, but helps disambiguate some behavior that recently caused
[issues](https://github.com/open-feature/go-sdk/issues/397) and
[confusion](https://github.com/open-feature/go-sdk/pull/400/files#r2191214490).
This is only one means of addressing this concern, so please don't
hesitate to put forward alternative proposals.
"Idempotent" may not be exactly the right word here, since it's only
idempotent within the scope of one execution of the life-cycle, so I'm
open to copy changes here as well.
# EDIT
⚠️ I've substantially changed this PR based on feedback, and
dismissed all approvals:
As @erka and others have pointed out, practically, `shutdown` has been
used for more than just cleaning up providers, it's also used frequently
to "reset" the API for testing purposes. I think this is a valid
use-case and I don't see any reason why it can't be added to what (at
least my understanding of) the original intent of the function was... so
I've changed 1.6.2 to say that the `shutdown` now also resets the state
of the API fully (removes hooks, providers, event handlers, etc) from
the API.
As @chrfwow noted (and I was concerned about as well) guaranteeing that
`shutdown` is not called while a provider is still starting up will be
tricky to implement. Instead I've changed 1.6.1 to say we will
unconditionally run shutdown on all providers, and also added a
recommendation that providers handle this gracefully in the provider
spec.
@sahidvelji I've also added a pre-amble about shutdown in general as you
requested.
@lukas-reining @erka @beeme1mr @sahidvelji @chrfwow please re-review.
---------
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Sahid Velji <sahidvelji@gmail.com>
## This PR
This PR updates the Multi-Provider documentation in the OpenFeature
specification to document the new `track` method support that was added
in [PR #1323](https://github.com/open-feature/js-sdk-contrib/pull/1323).
### 📚 New Documentation Sections
- **Track Method Support**: Added a section explaining how the
Multi-Provider implements tracking functionality
- **Introduction Section**: Updated use case examples to highlight
tracking capabilities
- **BaseEvaluationStrategy**: Added `shouldTrackWithThisProvider` method
to the abstract class
## Code Examples Added
```typescript
// Basic tracking usage
const multiProvider = new MultiProvider([
{ provider: new ProviderA() },
{ provider: new ProviderB() }
])
await OpenFeature.setProviderAndWait(multiProvider)
const client = OpenFeature.getClient()
// Track events across all ready providers
client.track('purchase', { targetingKey: 'user123' }, { value: 99.99, currency: 'USD' })
```
---------
Signed-off-by: Jonathan Norris <jonathan@taplytics.com>
This clarifies some ambiguity in hook execution order, mentioned in
https://github.com/open-feature/spec/issues/164:
> The issue here is that the overall hook execution order would vary if
any of these hooks have more than one entry. For example:
>
> ```
> API = [A, B]
> Client = [C, D]
> Invocation = [E, F]
> Provider = [G, H]
>
> # `before` list
> before_hooks = API + Client + Invocation + Provider
>
> # spreading directly (like Python)
> list1 = Provider + Invocation + Client + API
> # results in [G, H, E, F, C, D, A, B]
>
> # reversing the before list (like JavaScript)
> list2 = before_hooks[:]
> list2.reverse()
> # results in [H, G, F, E, D, C, B, A]
> ```
This change makes clear that `[H, G, F, E, D, C, B, A]` is the correct
answer here, which is the more intuitive choice, in my opinion.
This is also one of the few (only?) normative spec points that isn't a
single sentence, but instead has point-form clauses, so I've also
attempted to fix that.
I've work-shopped this with a few people and they interpreted it
correctly, though admittedly, the term "stack-wise" is doing a lot of
work here. I'm hopeful that our audience is familiar enough with that
sort of term that the intent is clear.
I believe there's a few SDKs where this is incorrectly implemented, I'll
audit those and create issues if this is merged. I don't expect that
such an implementation change will significantly impact anyone.
---------
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->
## This PR
<!-- add the description of the PR here -->
Even though the README says that no explicit status implies experimental
status, I think we should explicitly mark the Hooks section experimental
to align with the other sections.
### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->
### Notes
<!-- any additional notes for this PR -->
### Follow-up Tasks
<!-- anything that is related to this PR but not done here should be
noted under this section -->
<!-- if there is a need for a new issue, please link it here -->
### How to test
<!-- if applicable, add testing instructions under this section -->
Signed-off-by: Sahid Velji <sahidvelji@gmail.com>
## This PR
Loosens the value definition for hook data.
In the example within the spec, and within the implementation in the
dotnet-sdk, the intent was to allow any type of data. For example
creating an open telemetry span in before, storing it hook data, and
ending it in after.
### Notes
Related to: https://github.com/open-feature/dotnet-sdk/pull/387
---------
Signed-off-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->
## This PR
<!-- add the description of the PR here -->
This pull request includes updates to the
`specification/assets/gherkin/contextMerging.feature` file to improve
the clarity of scenario descriptions by specifying the context
(transaction, hook, or both) in which the context entries are added or
overwritten.
Changes to scenario descriptions:
* Updated the scenario description to specify that the context entry is
added for a transaction.
* Updated the scenario description to specify that the context entry is
added for a hook.
* Updated the scenario description to specify that the context entry is
added for both a transaction and a hook.
* Updated the scenario outline description to specify that the context
entry overwrites values for a transaction.
* Updated the scenario outline description to specify that the context
entry overwrites values for a hook.
* Updated the scenario outline description to specify that the context
entry overwrites values for both a transaction and a hook.
### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->
Fixes#303
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
## This PR
Adds gherkin test to verify that flag evaluations provide metadata
### Related Issues
Part of #1467
([https://github.com/open-feature/flagd/issues/1467](https://github.com/open-feature/flagd/issues/1467))
### Follow-up Tasks
Implement steps of the gherkin file in the repositories, and add test
data according to the issue
---------
Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Simon Schrottner <simon.schrottner@dynatrace.com>
- adds small appendix section to spec that defines how particular SDK
fields are mapped to the recently merge OTel semantics
- this allows providers and hooks to export OTel data consistently for
easy interop
- adds a couple supporting glossary liks
---------
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->
## This PR
<!-- add the description of the PR here -->
Add support for the hook-data concept for hooks. Hook-data allows for
per-evaluation data to be propagated between hooks.
This is especially useful for analytics purposes where you may want to
measure things that happen between stages, or you want to do something
like create a span in one stage and close it in another.
This concept is similar to the `series data` concept for LaunchDarkly
hooks.
https://github.com/launchdarkly/open-sdk-specs/tree/main/specs/HOOK-hooks#evaluationseriesdata
Unlike `series data` the data in this approach is mutable. This is
because the `before` stage already has a return value. We could
workaround this by specifying a return structure, but it maybe seems
more complex. The data is only passed to a specific hook instance, so
mutability is not of great concern.
Some functional languages may still need to use an immutable with return
values approach.
I can create an OFEP if we think this merits discussion prior to
proposal.
### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->
Related discussion in a PR comment.
https://github.com/open-feature/java-sdk/pull/1049#discussion_r1718895742
---------
Signed-off-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: Lukas Reining <lukas.reining@codecentric.de>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Minor clarifications to the in-memory provider, especially around `flags
changed` which at the moment is not consistently implemented.
---------
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: Lukas Reining <lukas.reining@codecentric.de>
Just replaces some PNGs with mermaid.
This is nice because they can change color for the website, and they are
more rigorous and maintainable (we already use mermaid in the spec).
old:
new:
```mermaid
flowchart LR
A[Application] --> API(Evaluation API)
API --> P[Provider]
P --> FMS[(Flag Management System)]
```
```mermaid
flowchart LR
B(('Before' stage)) ---> FE[Flag Evaluation]
B -..->|Error| E
FE ---> A(('After' stage))
FE -..->|Error| E(('Error' stage))
A ---> F(('Finally' stage))
E -..-> F
```
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
## This PR
- adds flag set to the glossary
### Notes
The term was voted on by the OpenFeature community.
https://github.com/orgs/open-feature/discussions/394
---------
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
As discussed in recent meetings, we've had complaints about this from
multiple sources. This PR adds a stipulation that no logging is done by
the SDK during flag evaluation. It also defines a very simple `logging
hook` concept as an included utility. These should be very simple to
write and will provide all the needed logging but give authors more
control than built in log statements. It will also be a very nice intro
to the hooks concept for users.
Here's the Java implementation:
https://github.com/open-feature/java-sdk/pull/1084
---------
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
- Adds an appendix section for the Multi-Provider, describing its
purpose and implementation
---------
Signed-off-by: Adam Wootton <adam@taplytics.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
I've been missing this glossary entry for some recent content.
---------
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
## This PR
- adds hard new lines to GitHub notes
### Notes
GitHub does some magic behind the scenes that isn't part of the markdown
standard. This causes some issues on our doc site. Explicitly adding a
hard line break using double space helps us avoid this issue.
### Follow-up Tasks
Update the docs to include this change.
---------
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->
## This PR
In section 4.5.1 of hooks it specifies:
```
> `Flag evaluation options` **MAY** contain `hook hints`, a map of data to be provided to hook invocations.
```
But it does not include this in the types.md file, which makes this a
bit confusing.
### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->
### Notes
<!-- any additional notes for this PR -->
### Follow-up Tasks
<!-- anything that is related to this PR but not done here should be
noted under this section -->
<!-- if there is a need for a new issue, please link it here -->
### How to test
<!-- if applicable, add testing instructions under this section -->
Signed-off-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Adding some additional non-normative text to clarify a few things at the
prompt of @federicobond .
The meaning and justification should be self-evident, and if it's not I
did a bad job. 😅
---------
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Requirement 5.1.5 speaks of a Provider Event Details `error code` field
but this field was missing from the specified type.
This field is required to then populate the Event Details `error code`
field.
Signed-off-by: Federico Bond <federicobond@gmail.com>
No substantial changes here, just editorial improvements and
corrections.
When implementing https://github.com/open-feature/spec/pull/241, I
noticed a few oversights (see comments).
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
* moves provider state into SDK (SDK now maintains provider
state/lifecycle; provider interface is now "stateless")
* refine semantics around client state when context is
pending/reconciled by using new events/states, instead of STALE
* add PROVIDER_FATAL error code
* add PROVIDER_RECONCILING event (client only)
* add RECONCILING provider status status (client only)
Resolves: https://github.com/open-feature/spec/issues/238
---------
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: Lukas Reining <lukas.reining@codecentric.de>
Co-authored-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>
Co-authored-by: Fabrizio Demaria <fabrizio.f.demaria@gmail.com>
Michael Beemer