-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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.
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
Allocation Id #37840
Allocation Id #37840
Changes from all commits
6db8216
acc62d9
c333311
2893693
6ade779
3ec51ec
d1c5a17
5595657
246d259
07c771a
26f5042
5d485ab
c09247b
958761e
fac284f
50755b0
ce64aca
c727b1c
33ae94e
c8f3b40
79c9053
d9cd086
0456c6d
411dd2e
7e33ecb
126937a
3261601
963c0f4
58cccc2
775df1a
e5f6ed4
0f688d2
b73b002
923362f
274c372
c4dd229
6421788
59b745c
185389d
e36cc7d
50fe5fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,13 @@ | |
# Licensed under the MIT License. See License.txt in the project root for | ||
# license information. | ||
# ------------------------------------------------------------------------- | ||
import json | ||
import time | ||
import random | ||
import hashlib | ||
import base64 | ||
from dataclasses import dataclass | ||
from typing import Dict, List | ||
from typing import Dict, List, Optional, Mapping, Any | ||
from azure.appconfiguration import ( # type:ignore # pylint:disable=no-name-in-module | ||
FeatureFlagConfigurationSetting, | ||
) | ||
|
@@ -25,24 +26,104 @@ | |
ETAG_KEY, | ||
FEATURE_FLAG_REFERENCE_KEY, | ||
FEATURE_FLAG_ID_KEY, | ||
ALLOCATION_ID_KEY, | ||
) | ||
|
||
FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL = 3600 # 1 hour in seconds | ||
MINIMAL_CLIENT_REFRESH_INTERVAL = 30 # 30 seconds | ||
|
||
JSON = Mapping[str, Any] | ||
|
||
|
||
@dataclass | ||
class _ConfigurationClientWrapperBase: | ||
endpoint: str | ||
|
||
@staticmethod | ||
def _generate_allocation_id(feature_flag_value: Dict[str, JSON]) -> Optional[str]: | ||
""" | ||
Generates an allocation ID for the specified feature. | ||
seed=123abc\ndefault_when_enabled=Control\npercentiles=0,Control,20;20,Test,100\nvariants=Control,standard;Test,special # pylint:disable=line-too-long | ||
|
||
:param Dict[str, JSON] feature_flag_value: The feature to generate an allocation ID for. | ||
:rtype: str | ||
:return: The allocation ID. | ||
""" | ||
|
||
allocation_id = "" | ||
allocated_variants = [] | ||
|
||
allocation: Optional[JSON] = feature_flag_value.get("allocation") | ||
|
||
if allocation: | ||
# Seed | ||
allocation_id = f"seed={allocation.get('seed', '')}" | ||
|
||
# DefaultWhenEnabled | ||
if "default_when_enabled" in allocation: | ||
allocated_variants.append(allocation.get("default_when_enabled")) | ||
|
||
allocation_id += f"\ndefault_when_enabled={allocation.get('default_when_enabled', '')}" | ||
|
||
# Percentile | ||
allocation_id += "\npercentiles=" | ||
|
||
percentile = allocation.get("percentile") | ||
|
||
if percentile: | ||
percentile_allocations = sorted( | ||
(x for x in percentile if x.get("from") != x.get("to")), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we validate that from, to and variant fields are all not None? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From logic in the creation of percentile allocation to must be an int so it will raise an error on creation if it isn't. from is optional[int], so we have to check it. |
||
key=lambda x: x.get("from"), | ||
) | ||
|
||
for percentile_allocation in percentile_allocations: | ||
if "variant" in percentile_allocation: | ||
allocated_variants.append(percentile_allocation.get("variant")) | ||
|
||
allocation_id += ";".join( | ||
f"{pa.get('from')}," f"{base64.b64encode(pa.get('variant').encode()).decode()}," f"{pa.get('to')}" | ||
for pa in percentile_allocations | ||
) | ||
else: | ||
allocation_id = "seed=\ndefault_when_enabled=\npercentiles=" | ||
zhiyuanliang-ms marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if not allocated_variants and (not allocation or not allocation.get("seed")): | ||
return None | ||
|
||
# Variants | ||
allocation_id += "\nvariants=" | ||
|
||
variants_value = feature_flag_value.get("variants") | ||
if variants_value and (isinstance(variants_value, list) or all(isinstance(v, dict) for v in variants_value)): | ||
if allocated_variants: | ||
if isinstance(variants_value, list) and all(isinstance(v, dict) for v in variants_value): | ||
zhiyuanliang-ms marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sorted_variants: List[Dict[str, Any]] = sorted( | ||
(v for v in variants_value if v.get("name") in allocated_variants), | ||
key=lambda v: v.get("name"), | ||
) | ||
|
||
for v in sorted_variants: | ||
allocation_id += f"{base64.b64encode(v.get('name', '').encode()).decode()}," | ||
if "configuration_value" in v: | ||
allocation_id += f"{json.dumps(v.get('configuration_value', ''), separators=(',', ':'))}" | ||
allocation_id += ";" | ||
allocation_id = allocation_id[:-1] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if the sorted_variant list is empty? The last character of allocation_id won't ";" |
||
|
||
# Create a sha256 hash of the allocation_id | ||
hash_object = hashlib.sha256(allocation_id.encode()) | ||
hash_digest = hash_object.digest() | ||
|
||
# Encode the first 15 bytes in base64 url | ||
allocation_id_hash = base64.urlsafe_b64encode(hash_digest[:15]).decode() | ||
return allocation_id_hash | ||
|
||
@staticmethod | ||
def _calculate_feature_id(key, label): | ||
basic_value = f"{key}\n" | ||
if label and not label.isspace(): | ||
basic_value += f"{label}" | ||
feature_flag_id_hash_bytes = hashlib.sha256(basic_value.encode()).digest() | ||
encoded_flag = base64.b64encode(feature_flag_id_hash_bytes) | ||
encoded_flag = encoded_flag.replace(b"+", b"-").replace(b"/", b"_") | ||
encoded_flag = base64.urlsafe_b64encode(feature_flag_id_hash_bytes) | ||
return encoded_flag[: encoded_flag.find(b"=")] | ||
|
||
def _feature_flag_telemetry( | ||
|
@@ -58,10 +139,14 @@ def _feature_flag_telemetry( | |
feature_flag_reference = f"{endpoint}kv/{feature_flag.key}" | ||
if feature_flag.label and not feature_flag.label.isspace(): | ||
feature_flag_reference += f"?label={feature_flag.label}" | ||
feature_flag_value[TELEMETRY_KEY][METADATA_KEY][FEATURE_FLAG_REFERENCE_KEY] = feature_flag_reference | ||
feature_flag_value[TELEMETRY_KEY][METADATA_KEY][FEATURE_FLAG_ID_KEY] = self._calculate_feature_id( | ||
feature_flag.key, feature_flag.label | ||
) | ||
if feature_flag_value[TELEMETRY_KEY].get("enabled"): | ||
feature_flag_value[TELEMETRY_KEY][METADATA_KEY][FEATURE_FLAG_REFERENCE_KEY] = feature_flag_reference | ||
feature_flag_value[TELEMETRY_KEY][METADATA_KEY][FEATURE_FLAG_ID_KEY] = self._calculate_feature_id( | ||
feature_flag.key, feature_flag.label | ||
) | ||
allocation_id = self._generate_allocation_id(feature_flag_value) | ||
if allocation_id: | ||
feature_flag_value[TELEMETRY_KEY][METADATA_KEY][ALLOCATION_ID_KEY] = allocation_id | ||
|
||
def _feature_flag_appconfig_telemetry( | ||
self, feature_flag: FeatureFlagConfigurationSetting, filters_used: Dict[str, bool] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why isn't default_when_enabled base64 encoded here (just like what you did for other variants in percentile allocation)
Is this by design?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a value that would cause a collision here- but for consistency I wouldn't be opposed to adding it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we update this than this will need to be updated too: https://github.com/Azure/AppConfiguration-DotnetProvider/blob/c2e18d7b2c2c458e6951fc88f17e1ec0ebc13884/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs#L355