Skip to content

Commit

Permalink
Moved adding of flags context into Scope (#3917)
Browse files Browse the repository at this point in the history
Using an error_processor to read data from the scope to add to the event is an anti-pattern.  Moving this into `Scope.apply_to_event()`. 

This PR:
- moves code that adds flags to an event from an error processor into the `Scope` class 
- moves `add_feature_flag()` function from `sentry_sdk.integrations.feature_flags` into `sentry_sdk.feature_flags`
  • Loading branch information
antonpirker authored Jan 13, 2025
1 parent 9f9ff34 commit 288f69a
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 119 deletions.
20 changes: 11 additions & 9 deletions sentry_sdk/flag_utils.py → sentry_sdk/feature_flags.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
from typing import TYPE_CHECKING

import sentry_sdk
from sentry_sdk._lru_cache import LRUCache

from typing import TYPE_CHECKING

if TYPE_CHECKING:
from typing import TypedDict, Optional
from sentry_sdk._types import Event, ExcInfo
from typing import TypedDict

FlagData = TypedDict("FlagData", {"flag": str, "result": bool})

Expand Down Expand Up @@ -33,8 +32,11 @@ def set(self, flag, result):
self.buffer.set(flag, result)


def flag_error_processor(event, exc_info):
# type: (Event, ExcInfo) -> Optional[Event]
scope = sentry_sdk.get_current_scope()
event["contexts"]["flags"] = {"values": scope.flags.get()}
return event
def add_feature_flag(flag, result):
# type: (str, bool) -> None
"""
Records a flag and its value to be sent on subsequent error events.
We recommend you do this on flag evaluations. Flags are buffered per Sentry scope.
"""
flags = sentry_sdk.get_current_scope().flags
flags.set(flag, result)
44 changes: 0 additions & 44 deletions sentry_sdk/integrations/feature_flags.py

This file was deleted.

4 changes: 1 addition & 3 deletions sentry_sdk/integrations/launchdarkly.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import sentry_sdk

from sentry_sdk.integrations import DidNotEnable, Integration
from sentry_sdk.flag_utils import flag_error_processor

try:
import ldclient
Expand Down Expand Up @@ -41,8 +40,7 @@ def __init__(self, ld_client=None):
@staticmethod
def setup_once():
# type: () -> None
scope = sentry_sdk.get_current_scope()
scope.add_error_processor(flag_error_processor)
pass


class LaunchDarklyHook(Hook):
Expand Down
4 changes: 0 additions & 4 deletions sentry_sdk/integrations/openfeature.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import sentry_sdk

from sentry_sdk.integrations import DidNotEnable, Integration
from sentry_sdk.flag_utils import flag_error_processor

try:
from openfeature import api
Expand All @@ -21,9 +20,6 @@ class OpenFeatureIntegration(Integration):
@staticmethod
def setup_once():
# type: () -> None
scope = sentry_sdk.get_current_scope()
scope.add_error_processor(flag_error_processor)

# Register the hook within the global openfeature hooks list.
api.add_hooks(hooks=[OpenFeatureHook()])

Expand Down
5 changes: 0 additions & 5 deletions sentry_sdk/integrations/unleash.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from typing import Any

import sentry_sdk
from sentry_sdk.flag_utils import flag_error_processor
from sentry_sdk.integrations import Integration, DidNotEnable

try:
Expand Down Expand Up @@ -49,7 +48,3 @@ def sentry_get_variant(self, feature, *args, **kwargs):

UnleashClient.is_enabled = sentry_is_enabled # type: ignore
UnleashClient.get_variant = sentry_get_variant # type: ignore

# Error processor
scope = sentry_sdk.get_current_scope()
scope.add_error_processor(flag_error_processor)
17 changes: 16 additions & 1 deletion sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from sentry_sdk.attachments import Attachment
from sentry_sdk.consts import DEFAULT_MAX_BREADCRUMBS, FALSE_VALUES, INSTRUMENTER
from sentry_sdk.flag_utils import FlagBuffer, DEFAULT_FLAG_CAPACITY
from sentry_sdk.feature_flags import FlagBuffer, DEFAULT_FLAG_CAPACITY
from sentry_sdk.profiler.continuous_profiler import try_autostart_continuous_profiler
from sentry_sdk.profiler.transaction_profiler import Profile
from sentry_sdk.session import Session
Expand Down Expand Up @@ -1378,6 +1378,14 @@ def _apply_contexts_to_event(self, event, hint, options):
else:
contexts["trace"] = self.get_trace_context()

def _apply_flags_to_event(self, event, hint, options):
# type: (Event, Hint, Optional[Dict[str, Any]]) -> None
flags = self.flags.get()
if len(flags) > 0:
event.setdefault("contexts", {}).setdefault("flags", {}).update(
{"values": flags}
)

def _drop(self, cause, ty):
# type: (Any, str) -> Optional[Any]
logger.info("%s (%s) dropped event", ty, cause)
Expand Down Expand Up @@ -1476,6 +1484,7 @@ def apply_to_event(

if not is_transaction and not is_check_in:
self._apply_breadcrumbs_to_event(event, hint, options)
self._apply_flags_to_event(event, hint, options)

event = self.run_error_processors(event, hint)
if event is None:
Expand Down Expand Up @@ -1518,6 +1527,12 @@ def update_from_scope(self, scope):
self._propagation_context = scope._propagation_context
if scope._session:
self._session = scope._session
if scope._flags:
if not self._flags:
self._flags = deepcopy(scope._flags)
else:
for flag in scope._flags.get():
self._flags.set(flag["flag"], flag["result"])

def update_from_kwargs(
self,
Expand Down
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@
import pytest

import sentry_sdk
from sentry_sdk.integrations.feature_flags import (
FeatureFlagsIntegration,
add_feature_flag,
)
from sentry_sdk.feature_flags import add_feature_flag, FlagBuffer


def test_featureflags_integration(sentry_init, capture_events, uninstall_integration):
uninstall_integration(FeatureFlagsIntegration.identifier)
sentry_init(integrations=[FeatureFlagsIntegration()])
sentry_init()

add_feature_flag("hello", False)
add_feature_flag("world", True)
Expand All @@ -34,8 +30,7 @@ def test_featureflags_integration(sentry_init, capture_events, uninstall_integra
def test_featureflags_integration_threaded(
sentry_init, capture_events, uninstall_integration
):
uninstall_integration(FeatureFlagsIntegration.identifier)
sentry_init(integrations=[FeatureFlagsIntegration()])
sentry_init()
events = capture_events()

# Capture an eval before we split isolation scopes.
Expand Down Expand Up @@ -86,8 +81,7 @@ def test_featureflags_integration_asyncio(
):
asyncio = pytest.importorskip("asyncio")

uninstall_integration(FeatureFlagsIntegration.identifier)
sentry_init(integrations=[FeatureFlagsIntegration()])
sentry_init()
events = capture_events()

# Capture an eval before we split isolation scopes.
Expand Down Expand Up @@ -131,3 +125,45 @@ async def runner():
{"flag": "world", "result": False},
]
}


def test_flag_tracking():
"""Assert the ring buffer works."""
buffer = FlagBuffer(capacity=3)
buffer.set("a", True)
flags = buffer.get()
assert len(flags) == 1
assert flags == [{"flag": "a", "result": True}]

buffer.set("b", True)
flags = buffer.get()
assert len(flags) == 2
assert flags == [{"flag": "a", "result": True}, {"flag": "b", "result": True}]

buffer.set("c", True)
flags = buffer.get()
assert len(flags) == 3
assert flags == [
{"flag": "a", "result": True},
{"flag": "b", "result": True},
{"flag": "c", "result": True},
]

buffer.set("d", False)
flags = buffer.get()
assert len(flags) == 3
assert flags == [
{"flag": "b", "result": True},
{"flag": "c", "result": True},
{"flag": "d", "result": False},
]

buffer.set("e", False)
buffer.set("f", False)
flags = buffer.get()
assert len(flags) == 3
assert flags == [
{"flag": "d", "result": False},
{"flag": "e", "result": False},
{"flag": "f", "result": False},
]
43 changes: 0 additions & 43 deletions tests/test_flag_utils.py

This file was deleted.

0 comments on commit 288f69a

Please sign in to comment.