Moved adding of flags context into Scope (#3917)

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`

Author: antonpirker

sentry_sdk/flag_utils.py renamed to sentry_sdk/feature_flags.py

@@ -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})
 
@@ -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)

sentry_sdk/integrations/feature_flags.py

@@ -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
@@ -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):

sentry_sdk/integrations/launchdarkly.py

@@ -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
@@ -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()])
 

sentry_sdk/integrations/openfeature.py

@@ -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:
@@ -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)

sentry_sdk/integrations/unleash.py

@@ -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
@@ -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)
@@ -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:
@@ -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,

sentry_sdk/scope.py

@@ -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)
@@ -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.
@@ -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.
@@ -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},
+    ]