Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction Context Propagation Proposal #32

Merged
merged 14 commits into from
Mar 3, 2023

Conversation

beeme1mr
Copy link
Member

@beeme1mr beeme1mr commented Aug 30, 2022

Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr beeme1mr changed the title initial evaluation context propagation OFEP Evaluation Context Propagation Proposal Aug 30, 2022
007-OFEP-evaluation-context-propagation.md Outdated Show resolved Hide resolved
007-OFEP-evaluation-context-propagation.md Outdated Show resolved Hide resolved
007-OFEP-evaluation-context-propagation.md Outdated Show resolved Hide resolved
007-OFEP-evaluation-context-propagation.md Outdated Show resolved Hide resolved
beeme1mr and others added 4 commits August 30, 2022 16:30
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: Evan Bradley <github@evanbradley.org>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: Evan Bradley <github@evanbradley.org>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr beeme1mr changed the title Evaluation Context Propagation Proposal Transaction Context Propagation Proposal Aug 30, 2022
Co-authored-by: Evan Bradley <github@evanbradley.org>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@toddbaert toddbaert self-requested a review August 31, 2022 17:37
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes a lot of sense, and could really help in applications without a robust dependency injection framework, or some other means of grabbing transaction-scoped data.

For example, in some multi-layered application, information about the subject (authenticated user) may only be available in the web layer. If a flag must be evaluated in some domain logic or data-access layer, it would require the application author to add the relevant user data down the entire call stack to where it's needed; adding an additional param to every called function on a huge function stack... Storing this data in a side-channel, such as thread-local storage or async continuations, allows this information to be passed to flag evaluations without this burden.

I'd like to see a PR adding this to the spec in a platform independent way.

beeme1mr and others added 4 commits August 31, 2022 15:13
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: Todd Baert <toddbaert@gmail.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@morigs
Copy link

morigs commented Jan 23, 2023

Is distributed context propagation out of context?
As a developer, how can I extract the current context in order to send it further (for instance, using OTel Baggage)?
Perhaps this matter should be mentioned even if it's out of scope

@beeme1mr
Copy link
Member Author

Hi @morigs, that's a good point. I'll explicitly state that distributed context propagation is out of scope of this OFEP. However, this OFEP is likely a prereq for distributed context propagation using OTel.

By the way, the demo environment includes a proof of concept for distributed context propagation. You can see it here.

Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@moredip
Copy link
Member

moredip commented Feb 2, 2023

I think this would be really beneficial for some runtimes (i.e. node), and is a really representative example of how the OpenFeature SDK could provide a very useful but fiddly-to-implement capability so that all the various providers don't have to figure it out themselves.

I would like to see if we could make the API as ergonomic as adding an attribute to an otel span - I think that's a close analog. Likewise it would be great it we could draw inspiration from how otel make the propagator registration as simple as possible for the simple case - maybe you've already done that.

@moredip
Copy link
Member

moredip commented Mar 2, 2023

@beeme1mr do you think you've gotten enough feedback on this that we could consider it accepted, and move on to speccing it out?

Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr beeme1mr merged commit 8604a41 into main Mar 3, 2023
@beeme1mr beeme1mr deleted the evaluation-context-propagation branch March 3, 2023 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants