From beb951210279b4a5f05a7961ab83b9f08e1a6895 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 5 Dec 2024 11:56:57 -0800 Subject: [PATCH] update ffIntegration test to be e2e, and fix LRU copy bug --- sentry_sdk/_lru_cache.py | 7 +- .../featureflags/test_featureflags.py | 118 ++++++++++++------ tests/test_lru_cache.py | 19 +++ 3 files changed, 105 insertions(+), 39 deletions(-) diff --git a/sentry_sdk/_lru_cache.py b/sentry_sdk/_lru_cache.py index ec557b1093..fff2fb85e5 100644 --- a/sentry_sdk/_lru_cache.py +++ b/sentry_sdk/_lru_cache.py @@ -62,7 +62,7 @@ """ -from copy import copy +from copy import copy, deepcopy SENTINEL = object() @@ -92,10 +92,13 @@ def __init__(self, max_size): self.hits = self.misses = 0 def __copy__(self): + """ + Cache keys and values are shallow copied. + """ cache = LRUCache(self.max_size) cache.full = self.full cache.cache = copy(self.cache) - cache.root = copy(self.root) + cache.root = deepcopy(self.root) return cache def set(self, key, value): diff --git a/tests/integrations/featureflags/test_featureflags.py b/tests/integrations/featureflags/test_featureflags.py index f89d64e9b3..4d2eb1731a 100644 --- a/tests/integrations/featureflags/test_featureflags.py +++ b/tests/integrations/featureflags/test_featureflags.py @@ -2,10 +2,14 @@ import concurrent.futures as cf import sentry_sdk +from sentry_sdk.integrations import _processed_integrations from sentry_sdk.integrations.featureflags import FeatureFlagsIntegration -def test_featureflags_integration(sentry_init): +def test_featureflags_integration(sentry_init, capture_events): + _processed_integrations.discard( + FeatureFlagsIntegration.identifier + ) # force reinstall sentry_init(integrations=[FeatureFlagsIntegration()]) flags_integration = sentry_sdk.get_client().get_integration(FeatureFlagsIntegration) @@ -13,61 +17,101 @@ def test_featureflags_integration(sentry_init): flags_integration.set_flag("world", True) flags_integration.set_flag("other", False) - assert sentry_sdk.get_current_scope().flags.get() == [ - {"flag": "hello", "result": False}, - {"flag": "world", "result": True}, - {"flag": "other", "result": False}, - ] + events = capture_events() + sentry_sdk.capture_exception(Exception("something wrong!")) + [event] = events + assert event["contexts"]["flags"] == { + "values": [ + {"flag": "hello", "result": False}, + {"flag": "world", "result": True}, + {"flag": "other", "result": False}, + ] + } -def test_featureflags_integration_threaded(sentry_init): + +def test_featureflags_integration_threaded(sentry_init, capture_events): + _processed_integrations.discard( + FeatureFlagsIntegration.identifier + ) # force reinstall sentry_init(integrations=[FeatureFlagsIntegration()]) + events = capture_events() + + # Capture an eval before we split isolation scopes. flags_integration = sentry_sdk.get_client().get_integration(FeatureFlagsIntegration) + flags_integration.set_flag("hello", False) def task(flag_key): # Creates a new isolation scope for the thread. # This means the evaluations in each task are captured separately. with sentry_sdk.isolation_scope(): + flags_integration = sentry_sdk.get_client().get_integration( + FeatureFlagsIntegration + ) flags_integration.set_flag(flag_key, False) - return sentry_sdk.get_current_scope().flags.get() - - # Capture an eval before we split isolation scopes. - flags_integration.set_flag("hello", False) + # use a tag to identify to identify events later on + sentry_sdk.set_tag("flag_key", flag_key) + sentry_sdk.capture_exception(Exception("something wrong!")) + # Run tasks in separate threads with cf.ThreadPoolExecutor(max_workers=2) as pool: - results = list(pool.map(task, ["world", "other"])) - - assert results[0] == [ - {"flag": "hello", "result": False}, - {"flag": "world", "result": False}, - ] - assert results[1] == [ - {"flag": "hello", "result": False}, - {"flag": "other", "result": False}, - ] - - -def test_featureflags_integration_asyncio(sentry_init): - """Assert concurrently evaluated flags do not pollute one another.""" + pool.map(task, ["world", "other"]) + + assert len(events) == 2 + events.sort(key=lambda e: e["tags"]["flag_key"]) + assert events[0]["contexts"]["flags"] == { + "values": [ + {"flag": "hello", "result": False}, + {"flag": "other", "result": False}, + ] + } + assert events[1]["contexts"]["flags"] == { + "values": [ + {"flag": "hello", "result": False}, + {"flag": "world", "result": False}, + ] + } + + +def test_featureflags_integration_asyncio(sentry_init, capture_events): + _processed_integrations.discard( + FeatureFlagsIntegration.identifier + ) # force reinstall sentry_init(integrations=[FeatureFlagsIntegration()]) + events = capture_events() + + # Capture an eval before we split isolation scopes. flags_integration = sentry_sdk.get_client().get_integration(FeatureFlagsIntegration) + flags_integration.set_flag("hello", False) async def task(flag_key): + # Creates a new isolation scope for the thread. + # This means the evaluations in each task are captured separately. with sentry_sdk.isolation_scope(): + flags_integration = sentry_sdk.get_client().get_integration( + FeatureFlagsIntegration + ) flags_integration.set_flag(flag_key, False) - return sentry_sdk.get_current_scope().flags.get() + # use a tag to identify to identify events later on + sentry_sdk.set_tag("flag_key", flag_key) + sentry_sdk.capture_exception(Exception("something wrong!")) async def runner(): return asyncio.gather(task("world"), task("other")) - flags_integration.set_flag("hello", False) - - results = asyncio.run(runner()).result() - assert results[0] == [ - {"flag": "hello", "result": False}, - {"flag": "world", "result": False}, - ] - assert results[1] == [ - {"flag": "hello", "result": False}, - {"flag": "other", "result": False}, - ] + asyncio.run(runner()) + + assert len(events) == 2 + events.sort(key=lambda e: e["tags"]["flag_key"]) + assert events[0]["contexts"]["flags"] == { + "values": [ + {"flag": "hello", "result": False}, + {"flag": "other", "result": False}, + ] + } + assert events[1]["contexts"]["flags"] == { + "values": [ + {"flag": "hello", "result": False}, + {"flag": "world", "result": False}, + ] + } diff --git a/tests/test_lru_cache.py b/tests/test_lru_cache.py index 3e9c0ac964..92c8257eaf 100644 --- a/tests/test_lru_cache.py +++ b/tests/test_lru_cache.py @@ -1,3 +1,5 @@ +from copy import copy + import pytest from sentry_sdk._lru_cache import LRUCache @@ -58,3 +60,20 @@ def test_cache_get_all(): assert cache.get_all() == [(1, 1), (2, 2), (3, 3)] cache.get(1) assert cache.get_all() == [(2, 2), (3, 3), (1, 1)] + + +def test_cache_copy(): + cache = LRUCache(3) + cache.set(0, 0) + cache.set(1, 1) + + copied = copy(cache) + cache.set(2, 2) + cache.set(3, 3) + assert copied.get_all() == [(0, 0), (1, 1)] + assert cache.get_all() == [(1, 1), (2, 2), (3, 3)] + + copied = copy(cache) + cache.get(1) + assert copied.get_all() == [(1, 1), (2, 2), (3, 3)] + assert cache.get_all() == [(2, 2), (3, 3), (1, 1)]