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

feat: implement transaction context #315

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ print("Value: " + str(flag_value))
| ✅ | [Domains](#domains) | Logically bind clients with providers. |
| ✅ | [Eventing](#eventing) | React to state changes in the provider or flag management system. |
| ✅ | [Shutdown](#shutdown) | Gracefully clean up a provider during application shutdown. |
| ️️⚠️ | [Transaction Context Propagation](#transaction-context-propagation) | Set a specific [evaluation context](/docs/reference/concepts/evaluation-context) for a transaction (e.g. an HTTP request or a thread) |
| ✅ | [Extending](#extending) | Extend OpenFeature with custom providers and hooks. |

<sub>Implemented: ✅ | In-progress: ⚠️ | Not implemented yet: ❌</sub>
Expand Down
25 changes: 25 additions & 0 deletions openfeature/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
from openfeature.provider import FeatureProvider
from openfeature.provider._registry import provider_registry
from openfeature.provider.metadata import Metadata
from openfeature.transaction_context import (
NoopTransactionContextPropagator,
TransactionContextPropagator,
)

__all__ = [
"get_client",
Expand All @@ -26,12 +30,19 @@
"shutdown",
"add_handler",
"remove_handler",
"set_transaction_context_propagator",
"set_transaction_context",
"get_transaction_context",
]

_evaluation_context = EvaluationContext()

_hooks: typing.List[Hook] = []

_transaction_context_propagator: TransactionContextPropagator = (
NoopTransactionContextPropagator()
Copy link
Member Author

Choose a reason for hiding this comment

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

Should the SDK initialize a functional transaction context propagator by default?

Copy link
Member

Choose a reason for hiding this comment

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

We do something similar to this in JS. What would the alternative be?

Copy link
Member Author

@federicobond federicobond May 17, 2024

Choose a reason for hiding this comment

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

Having a default implementation that is not a no-op. But it could introduce a performance penalty if you are not using it, so probably better to default to no-op. We might want to put out a warning if you call set_transaction_context with a NoopTransactionContextPropagator though.

)


def get_client(
domain: typing.Optional[str] = None, version: typing.Optional[str] = None
Expand Down Expand Up @@ -94,3 +105,17 @@

def remove_handler(event: ProviderEvent, handler: EventHandler) -> None:
_event_support.remove_global_handler(event, handler)


def set_transaction_context_propagator(
propagator: TransactionContextPropagator,
) -> None:
_transaction_context_propagator = propagator

Check warning on line 113 in openfeature/api.py

View check run for this annotation

Codecov / codecov/patch

openfeature/api.py#L113

Added line #L113 was not covered by tests


def set_transaction_context(context: EvaluationContext) -> None:
_transaction_context_propagator.set_transaction_context(context)

Check warning on line 117 in openfeature/api.py

View check run for this annotation

Codecov / codecov/patch

openfeature/api.py#L117

Added line #L117 was not covered by tests


def get_transaction_context() -> EvaluationContext:
return _transaction_context_propagator.get_transaction_context()
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this the right API or do we want something based on decorators/context managers?

Copy link
Member

Choose a reason for hiding this comment

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

Could you elaborate on this? The main use case for accessing transaction context would be within the SDK when it merges context prior to flag evaluation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the comment should have been in the set_transaction_context. I was wondering if we should do something like this instead:

with start_transaction_context(context):
    continue_transaction()  # context will be cleaned up after the block completes

Similar to the way it works in JS by taking a callback.

3 changes: 2 additions & 1 deletion openfeature/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,10 @@ def evaluate_flag_details( # noqa: PLR0915
)
invocation_context = invocation_context.merge(ctx2=evaluation_context)

# Requirement 3.2.2 merge: API.context->client.context->invocation.context
# Requirement 3.2.3 merge: API.context->transaction.context->client.context->invocation.context
merged_context = (
api.get_evaluation_context()
.merge(api.get_transaction_context())
.merge(self.context)
.merge(invocation_context)
)
Expand Down
37 changes: 37 additions & 0 deletions openfeature/transaction_context.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import typing
from contextvars import ContextVar

from openfeature.evaluation_context import EvaluationContext

__all__ = [
"TransactionContextPropagator",
"NoopTransactionContextPropagator",
"ContextVarTransactionContextPropagator",
]


class TransactionContextPropagator(typing.Protocol):
def get_transaction_context(self) -> EvaluationContext: ...

def set_transaction_context(self, context: EvaluationContext) -> None: ...


class NoopTransactionContextPropagator(TransactionContextPropagator):
def get_transaction_context(self) -> EvaluationContext:
return EvaluationContext()

def set_transaction_context(self, context: EvaluationContext) -> None:
pass

Check warning on line 24 in openfeature/transaction_context.py

View check run for this annotation

Codecov / codecov/patch

openfeature/transaction_context.py#L24

Added line #L24 was not covered by tests


class ContextVarTransactionContextPropagator(TransactionContextPropagator):
def __init__(self) -> None:
self._contextvar = ContextVar(
"transaction_context", default=EvaluationContext()
)

def get_transaction_context(self) -> EvaluationContext:
return self._contextvar.get()

def set_transaction_context(self, context: EvaluationContext) -> None:
self._contextvar.set(context)
28 changes: 28 additions & 0 deletions tests/test_transaction_context.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from concurrent.futures import ThreadPoolExecutor

from openfeature.evaluation_context import EvaluationContext
from openfeature.transaction_context import ContextVarTransactionContextPropagator


def test_contextvar_transaction_context_propagator():
propagator = ContextVarTransactionContextPropagator()

context = propagator.get_transaction_context()
assert isinstance(context, EvaluationContext)

context = EvaluationContext(targeting_key="foo", attributes={"key": "value"})
propagator.set_transaction_context(context)
transaction_context = propagator.get_transaction_context()

assert transaction_context.targeting_key == "foo"
assert transaction_context.attributes == {"key": "value"}

def thread_fn():
thread_context = propagator.get_transaction_context()
assert thread_context.targeting_key is None
assert thread_context.attributes == {}

with ThreadPoolExecutor() as executor:
future = executor.submit(thread_fn)

future.result()