Skip to content

Commit

Permalink
fix: don't count stats for parent flags, even in get_variant (#300)
Browse files Browse the repository at this point in the history
This PR fixes an issue where parent flags would have metrics counted when you call get_variant on a child flag. This was due to a missing skip_stats argument in the get_variant method of a feature.

I have added a test and the specific data setup that was reported to cause the issue.
  • Loading branch information
thomasheartman committed Mar 6, 2024
1 parent 37c7df4 commit 31c33eb
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 1 deletion.
2 changes: 1 addition & 1 deletion UnleashClient/features/Feature.py
Expand Up @@ -101,7 +101,7 @@ def get_variant(self, context: dict = None, skip_stats: bool = False) -> dict:
:param context: Context information
:return:
"""
evaluation_result = self._get_evaluation_result(context)
evaluation_result = self._get_evaluation_result(context, skip_stats)
is_feature_enabled = evaluation_result.enabled
variant = evaluation_result.variant
if variant is None or (is_feature_enabled and variant == DISABLED_VARIATION):
Expand Down
29 changes: 29 additions & 0 deletions tests/unit_tests/test_client.py
Expand Up @@ -555,6 +555,35 @@ def test_uc_registers_variant_metrics_for_nonexistent_features(unleash_client):
assert request["bucket"]["toggles"]["nonexistent-flag"]["variants"]["disabled"] == 1


@responses.activate
def test_uc_doesnt_count_metrics_for_dependency_parents(unleash_client):
# Set up API
responses.add(responses.POST, URL + REGISTER_URL, json={}, status=202)
responses.add(
responses.GET,
URL + FEATURES_URL,
json=MOCK_FEATURE_WITH_DEPENDENCIES_RESPONSE,
status=200,
)
responses.add(responses.POST, URL + METRICS_URL, json={}, status=202)

# Create Unleash client and check initial load
unleash_client.initialize_client()
time.sleep(1)

child = "ChildWithVariant"
# Check a flag that depends on a parent
unleash_client.is_enabled(child)
unleash_client.get_variant(child)

# Verify that the parent doesn't have any metrics registered
time.sleep(12)
request = json.loads(responses.calls[-1].request.body)
assert request["bucket"]["toggles"][child]["yes"] == 2
assert request["bucket"]["toggles"][child]["variants"]["childVariant"] == 1
assert "Parent" not in request["bucket"]["toggles"]


@responses.activate
def test_uc_disabled_registration(unleash_client_toggle_only):
unleash_client = unleash_client_toggle_only
Expand Down
27 changes: 27 additions & 0 deletions tests/utilities/mocks/mock_features.py
Expand Up @@ -196,6 +196,33 @@
}
],
},
{
"name": "ChildWithVariant",
"description": "Feature toggle with variant that depends on a specific variant of the Parent feature toggle",
"enabled": True,
"strategies": [
{
"name": "default",
"parameters": {},
"variants": [
{
"name": "childVariant",
"weight": 1000,
"stickiness": "default",
"weightType": "variable",
}
],
}
],
"createdAt": "2018-10-09T06:04:05.667Z",
"impressionData": False,
"dependencies": [
{
"feature": "Parent",
"variants": ["variant1"],
}
],
},
{
"name": "Disabled",
"description": "Disabled feature toggle",
Expand Down

0 comments on commit 31c33eb

Please sign in to comment.