Skip to content

Commit

Permalink
Rm get_variant code
Browse files Browse the repository at this point in the history
  • Loading branch information
aliu39 committed Jan 10, 2025
1 parent adb3bb1 commit 7c40943
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 208 deletions.
16 changes: 0 additions & 16 deletions sentry_sdk/integrations/unleash.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ def setup_once():
# type: () -> None
# Wrap and patch evaluation methods (instance methods)
old_is_enabled = UnleashClient.is_enabled
old_get_variant = UnleashClient.get_variant

@wraps(old_is_enabled)
def sentry_is_enabled(self, feature, *args, **kwargs):
Expand All @@ -33,22 +32,7 @@ def sentry_is_enabled(self, feature, *args, **kwargs):

return enabled

@wraps(old_get_variant)
def sentry_get_variant(self, feature, *args, **kwargs):
# type: (UnleashClient, str, *Any, **Any) -> Any
variant = old_get_variant(self, feature, *args, **kwargs)
enabled = variant.get("feature_enabled", False)

# Payloads are not always used as the feature's value for application logic. They
# may be used for metrics or debugging context instead. Therefore, we treat every
# variant as a boolean toggle, using the `enabled` field.
flags = sentry_sdk.get_current_scope().flags
flags.set(feature, enabled)

return variant

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

# Error processor
scope = sentry_sdk.get_current_scope()
Expand Down
144 changes: 0 additions & 144 deletions tests/integrations/unleash/test_unleash.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,37 +34,6 @@ def test_is_enabled(sentry_init, capture_events, uninstall_integration):
}


def test_get_variant(sentry_init, capture_events, uninstall_integration):
uninstall_integration(UnleashIntegration.identifier)

with mock_unleash_client():
client = UnleashClient() # type: ignore[arg-type]
sentry_init(integrations=[UnleashIntegration()])
client.get_variant("no_payload_feature")
client.get_variant("no_variant_feature")
client.get_variant("string_feature")
client.get_variant("json_feature")
client.get_variant("csv_feature")
client.get_variant("number_feature")
client.get_variant("unknown_feature")

events = capture_events()
sentry_sdk.capture_exception(Exception("something wrong!"))

assert len(events) == 1
assert events[0]["contexts"]["flags"] == {
"values": [
{"flag": "no_payload_feature", "result": True},
{"flag": "no_variant_feature", "result": True},
{"flag": "string_feature", "result": True},
{"flag": "json_feature", "result": True},
{"flag": "csv_feature", "result": True},
{"flag": "number_feature", "result": True},
{"flag": "unknown_feature", "result": False},
]
}


def test_is_enabled_threaded(sentry_init, capture_events, uninstall_integration):
uninstall_integration(UnleashIntegration.identifier)

Expand Down Expand Up @@ -114,55 +83,6 @@ def task(flag_key):
}


def test_get_variant_threaded(sentry_init, capture_events, uninstall_integration):
uninstall_integration(UnleashIntegration.identifier)

with mock_unleash_client():
client = UnleashClient() # type: ignore[arg-type]
sentry_init(integrations=[UnleashIntegration()])
events = capture_events()

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():
client.get_variant(flag_key)
# use a tag to identify to identify events later on
sentry_sdk.set_tag("task_id", flag_key)
sentry_sdk.capture_exception(Exception("something wrong!"))

# Capture an eval before we split isolation scopes.
client.get_variant("hello")

with cf.ThreadPoolExecutor(max_workers=2) as pool:
pool.map(task, ["no_payload_feature", "other"])

# Capture error in original scope
sentry_sdk.set_tag("task_id", "0")
sentry_sdk.capture_exception(Exception("something wrong!"))

assert len(events) == 3
events.sort(key=lambda e: e["tags"]["task_id"])

assert events[0]["contexts"]["flags"] == {
"values": [
{"flag": "hello", "result": False},
]
}
assert events[1]["contexts"]["flags"] == {
"values": [
{"flag": "hello", "result": False},
{"flag": "no_payload_feature", "result": True},
]
}
assert events[2]["contexts"]["flags"] == {
"values": [
{"flag": "hello", "result": False},
{"flag": "other", "result": False},
]
}


@pytest.mark.skipif(sys.version_info < (3, 7), reason="requires python3.7 or higher")
def test_is_enabled_asyncio(sentry_init, capture_events, uninstall_integration):
asyncio = pytest.importorskip("asyncio")
Expand Down Expand Up @@ -214,66 +134,12 @@ async def runner():
}


@pytest.mark.skipif(sys.version_info < (3, 7), reason="requires python3.7 or higher")
def test_get_variant_asyncio(sentry_init, capture_events, uninstall_integration):
asyncio = pytest.importorskip("asyncio")

uninstall_integration(UnleashIntegration.identifier)

with mock_unleash_client():
client = UnleashClient() # type: ignore[arg-type]
sentry_init(integrations=[UnleashIntegration()])
events = capture_events()

async def task(flag_key):
with sentry_sdk.isolation_scope():
client.get_variant(flag_key)
# use a tag to identify to identify events later on
sentry_sdk.set_tag("task_id", flag_key)
sentry_sdk.capture_exception(Exception("something wrong!"))

async def runner():
return asyncio.gather(task("no_payload_feature"), task("other"))

# Capture an eval before we split isolation scopes.
client.get_variant("hello")

asyncio.run(runner())

# Capture error in original scope
sentry_sdk.set_tag("task_id", "0")
sentry_sdk.capture_exception(Exception("something wrong!"))

assert len(events) == 3
events.sort(key=lambda e: e["tags"]["task_id"])

assert events[0]["contexts"]["flags"] == {
"values": [
{"flag": "hello", "result": False},
]
}
assert events[1]["contexts"]["flags"] == {
"values": [
{"flag": "hello", "result": False},
{"flag": "no_payload_feature", "result": True},
]
}
assert events[2]["contexts"]["flags"] == {
"values": [
{"flag": "hello", "result": False},
{"flag": "other", "result": False},
]
}


def test_wraps_original(sentry_init, uninstall_integration):
with mock_unleash_client():
client = UnleashClient() # type: ignore[arg-type]

mock_is_enabled = mock.Mock(return_value=random() < 0.5)
mock_get_variant = mock.Mock(return_value={"enabled": random() < 0.5})
client.is_enabled = mock_is_enabled
client.get_variant = mock_get_variant

uninstall_integration(UnleashIntegration.identifier)
sentry_init(integrations=[UnleashIntegration()]) # type: ignore
Expand All @@ -285,26 +151,16 @@ def test_wraps_original(sentry_init, uninstall_integration):
{"kwarg": 1},
)

res = client.get_variant("test-flag", "arg", kwarg=1)
assert res == mock_get_variant.return_value
assert mock_get_variant.call_args == (
("test-flag", "arg"),
{"kwarg": 1},
)


def test_wrapper_attributes(sentry_init, uninstall_integration):
with mock_unleash_client():
client = UnleashClient() # type: ignore[arg-type]

original_is_enabled = client.is_enabled
original_get_variant = client.get_variant

uninstall_integration(UnleashIntegration.identifier)
sentry_init(integrations=[UnleashIntegration()]) # type: ignore

# Mock clients methods have not lost their qualified names after decoration.
assert client.is_enabled.__name__ == "is_enabled"
assert client.is_enabled.__qualname__ == original_is_enabled.__qualname__
assert client.get_variant.__name__ == "get_variant"
assert client.get_variant.__qualname__ == original_get_variant.__qualname__
50 changes: 2 additions & 48 deletions tests/integrations/unleash/testutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ def mock_unleash_client():
Temporarily replaces UnleashClient's methods with mock implementations
for testing.
This context manager swaps out UnleashClient's __init__, is_enabled,
and get_variant methods with mock versions from MockUnleashClient.
This context manager swaps out UnleashClient's __init__ and is_enabled,
methods with mock versions from MockUnleashClient.
Original methods are restored when exiting the context.
After mocking the client class the integration can be initialized.
Expand All @@ -23,17 +23,14 @@ def mock_unleash_client():
"""
old_init = UnleashClient.__init__
old_is_enabled = UnleashClient.is_enabled
old_get_variant = UnleashClient.get_variant

UnleashClient.__init__ = MockUnleashClient.__init__
UnleashClient.is_enabled = MockUnleashClient.is_enabled
UnleashClient.get_variant = MockUnleashClient.get_variant

yield

UnleashClient.__init__ = old_init
UnleashClient.is_enabled = old_is_enabled
UnleashClient.get_variant = old_get_variant


class MockUnleashClient:
Expand All @@ -44,48 +41,5 @@ def __init__(self, *a, **kw):
"world": False,
}

self.feature_to_variant = {
"string_feature": {
"name": "variant1",
"enabled": True,
"feature_enabled": True,
"payload": {"type": "string", "value": "val1"},
},
"json_feature": {
"name": "variant1",
"enabled": True,
"feature_enabled": True,
"payload": {"type": "json", "value": '{"key1": 0.53}'},
},
"number_feature": {
"name": "variant1",
"enabled": True,
"feature_enabled": True,
"payload": {"type": "number", "value": "134.5"},
},
"csv_feature": {
"name": "variant1",
"enabled": True,
"feature_enabled": True,
"payload": {"type": "csv", "value": "abc 123\ncsbq 94"},
},
"no_payload_feature": {
"name": "variant1",
"enabled": True,
"feature_enabled": True,
},
"no_variant_feature": {
"name": "disabled",
"enabled": False,
"feature_enabled": True,
},
}

# Returned when the feature does not exist.
self.fallback_variant = {"name": "disabled", "enabled": False}

def is_enabled(self, feature, *a, **kw):
return self.features.get(feature, False)

def get_variant(self, feature, *a, **kw):
return self.feature_to_variant.get(feature, self.fallback_variant)

0 comments on commit 7c40943

Please sign in to comment.