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.

Sign up for GitHub

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(flags): remove Unleash get_variant patching code #3914

Merged
merged 4 commits into from
Jan 14, 2025
Merged

feat(flags): remove Unleash get_variant patching code #3914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf2a67e
fix(flags): use feature_enabled field of unleash variants
aliu39 Jan 10, 2025
adb3bb1
Rename nonexistent variant
aliu39 Jan 10, 2025
7c40943
Rm get_variant code
aliu39 Jan 10, 2025
5bfd873
Merge branch 'master' into aliu/unleash-fix
aliu39 Jan 13, 2025
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
16 changes: 0 additions & 16 deletions sentry_sdk/integrations/unleash.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,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 @@ -32,19 +31,4 @@ 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("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
156 changes: 7 additions & 149 deletions tests/integrations/unleash/test_unleash.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def test_is_enabled(sentry_init, capture_events, uninstall_integration):
uninstall_integration(UnleashIntegration.identifier)

with mock_unleash_client():
client = UnleashClient()
client = UnleashClient() # type: ignore[arg-type]
sentry_init(integrations=[UnleashIntegration()])
client.is_enabled("hello")
client.is_enabled("world")
Expand All @@ -34,41 +34,12 @@ 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()
sentry_init(integrations=[UnleashIntegration()]) # type: ignore
client.get_variant("no_payload_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": "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)

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

def task(flag_key):
Expand Down Expand Up @@ -112,63 +83,14 @@ 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()
sentry_init(integrations=[UnleashIntegration()]) # type: ignore
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")
uninstall_integration(UnleashIntegration.identifier)

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

async def task(flag_key):
Expand Down Expand Up @@ -212,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()
sentry_init(integrations=[UnleashIntegration()]) # type: ignore
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()
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 @@ -283,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() # <- Returns a MockUnleashClient
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__
36 changes: 2 additions & 34 deletions tests/integrations/unleash/testutils.py
View file
Open in desktop
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,34 +41,5 @@ def __init__(self, *a, **kw):
"world": False,
}

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

self.disabled_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.disabled_variant)
Loading