feat!: add evaluation details to finally hook#280
Merged
Conversation
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
aepfli
approved these changes
Nov 21, 2024
askpt
approved these changes
Nov 21, 2024
lukas-reining
approved these changes
Nov 22, 2024
Member
lukas-reining
left a comment
There was a problem hiding this comment.
I really like the change, I am just wondering if we should make this a breaking change.
Maybe we could avoid it as stated in the comment.
nicklasl
approved these changes
Nov 22, 2024
Signed-off-by: Michael Beemer <michael.beemer@dynatrace.com>
Member
Author
|
I'm going to merge on Friday unless there's an objection. Thanks! |
toddbaert
approved these changes
Dec 5, 2024
Member
toddbaert
left a comment
There was a problem hiding this comment.
Approving, and for the record, I think we should generally implement this in SDKs as a breaking change without incrementing the major version, using the leeway this policy gives us. I really don't think many will be impacted, and I think it's an easy and clear fix for those few who are.
This was referenced Dec 11, 2024
github-merge-queue bot
pushed a commit
to open-feature/js-sdk
that referenced
this pull request
Dec 12, 2024
## This PR - adds evaluation details to the `finally` stage in hooks. ### Notes This breaks the signature of the `finally` stages based on [this spec enhancement](open-feature/spec#280). It is **not** considered a breaking change to the SDK because hooks are marked as experimental in the spec, and the change has no impact on known hooks. The noteworthy change to the interface is: ```diff - finally?(hookContext: Readonly<HookContext<T>>, hookHints?: HookHints): HooksReturn; + finally?(hookContext: Readonly<HookContext<T>>, evaluationDetails: EvaluationDetails<T>, hookHints?: HookHints): HooksReturn; ``` ### Follow-up Tasks - Update the JS contribs repo --------- Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
kevinmlong
pushed a commit
to kevinmlong/openfeature-js-sdk
that referenced
this pull request
Dec 21, 2024
## This PR - adds evaluation details to the `finally` stage in hooks. ### Notes This breaks the signature of the `finally` stages based on [this spec enhancement](open-feature/spec#280). It is **not** considered a breaking change to the SDK because hooks are marked as experimental in the spec, and the change has no impact on known hooks. The noteworthy change to the interface is: ```diff - finally?(hookContext: Readonly<HookContext<T>>, hookHints?: HookHints): HooksReturn; + finally?(hookContext: Readonly<HookContext<T>>, evaluationDetails: EvaluationDetails<T>, hookHints?: HookHints): HooksReturn; ``` ### Follow-up Tasks - Update the JS contribs repo --------- Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
wichopy
pushed a commit
to wichopy/openfeature-js-sdk
that referenced
this pull request
Dec 31, 2024
## This PR - adds evaluation details to the `finally` stage in hooks. ### Notes This breaks the signature of the `finally` stages based on [this spec enhancement](open-feature/spec#280). It is **not** considered a breaking change to the SDK because hooks are marked as experimental in the spec, and the change has no impact on known hooks. The noteworthy change to the interface is: ```diff - finally?(hookContext: Readonly<HookContext<T>>, hookHints?: HookHints): HooksReturn; + finally?(hookContext: Readonly<HookContext<T>>, evaluationDetails: EvaluationDetails<T>, hookHints?: HookHints): HooksReturn; ``` ### Follow-up Tasks - Update the JS contribs repo --------- Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com> Signed-off-by: Will Chou <w.chou06@gmail.com>
This was referenced Feb 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR
finallystage in hooks to include evaluation details.Related Issues
#281
Notes
This is a breaking change but will significantly enhance the usefulness of the
finallystage. Currently, thefinallystage is used to perform end-of-transaction or clean-up tasks. If this proposal is accepted, thefinallystage will have access to the same evaluation details that are returned to the application author.This change will make implementing the OpenTelemetry Feature Flag Semantic Convention changes easier because the evaluation details in the
finallystage can contain all the telemetry we'll need. Values likeflag set idandflag set versioncan be supplied via flag metadata, which isn't accessible in theerroror currentfinallystages.Here's a draft PR showing how this could be implemented in JavaScript.
Follow-up Tasks
Implement the changes in all SDKs.