review comments

Author: mrm9084

sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py

@@ -260,11 +260,11 @@ def _update_ff_telemetry_metadata(
             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}"
-            allocation = feature_flag_value[TELEMETRY_KEY].get("allocation")
+            allocation = feature_flag_value.get("allocation")
             if allocation and allocation.get("seed"):
                 self._tracing_context.uses_seed = True
-            if feature_flag_value[TELEMETRY_KEY].get("variant"):
-                self._tracing_context.update_max_variants(len(feature_flag_value[TELEMETRY_KEY].get("variant")))
+            if feature_flag_value.get("variant"):
+                self._tracing_context.update_max_variants(len(feature_flag_value.get("variant", "")))
             if feature_flag_value[TELEMETRY_KEY].get("enabled"):
                 self._tracing_context.uses_telemetry = True
                 feature_flag_value[TELEMETRY_KEY][METADATA_KEY][FEATURE_FLAG_REFERENCE_KEY] = feature_flag_reference

sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_request_tracing_context.py

@@ -110,84 +110,6 @@ def update_max_variants(self, size: int) -> None:
         if self.max_variants is None or size > self.max_variants:
             self.max_variants = size
 
-    def _get_features_string(self) -> str:
-        """
-        Generate the features string for correlation context.
-
-        :return: A string containing features used separated by delimiters.
-        :rtype: str
-        """
-        features_list = []
-
-        if self.uses_load_balancing:
-            features_list.append(LOAD_BALANCING_FEATURE)
-        if self.uses_ai_configuration:
-            features_list.append(AI_CONFIGURATION_FEATURE)
-        if self.uses_aicc_configuration:
-            features_list.append(AI_CHAT_COMPLETION_FEATURE)
-        if self.get_assembly_version(AZURE_AI_PROJECTS_PACKAGE):
-            features_list.append(AI_FOUNDRY_SDK_FEATURE)
-
-        return Delimiter.join(features_list)
-
-    def _create_features_string(self) -> str:
-        """
-        Generate the features string for feature flag usage tracking.
-
-        :return: A string containing feature flag usage tags separated by delimiters.
-        :rtype: str
-        """
-        features_list = []
-
-        if self.uses_seed:
-            features_list.append(FEATURE_FLAG_USES_SEED_TAG)
-
-        if self.uses_telemetry:
-            features_list.append(FEATURE_FLAG_USES_TELEMETRY_TAG)
-
-        return Delimiter.join(features_list)
-
-    @staticmethod
-    def get_host_type() -> str:
-        """
-        Detect the host environment type based on environment variables.
-
-        :return: The detected host type.
-        :rtype: str
-        """
-        if os.environ.get(AzureFunctionEnvironmentVariable):
-            return HostType.AZURE_FUNCTION
-        if os.environ.get(AzureWebAppEnvironmentVariable):
-            return HostType.AZURE_WEB_APP
-        if os.environ.get(ContainerAppEnvironmentVariable):
-            return HostType.CONTAINER_APP
-        if os.environ.get(KubernetesEnvironmentVariable):
-            return HostType.KUBERNETES
-        if os.environ.get(ServiceFabricEnvironmentVariable):
-            return HostType.SERVICE_FABRIC
-
-        return HostType.UNIDENTIFIED
-
-    @staticmethod
-    def get_assembly_version(package_name: str) -> Optional[str]:
-        """
-        Get the version of a Python package.
-
-        :param package_name: The name of the package to get version for.
-        :type package_name: str
-        :return: Package version string or None if not found.
-        :rtype: Optional[str]
-        """
-        if not package_name:
-            return None
-
-        try:
-            return version(package_name)
-        except PackageNotFoundError:
-            pass
-
-        return None
-
     def reset_ai_configuration_tracing(self) -> None:
         """
         Reset AI configuration tracing flags.
@@ -276,12 +198,12 @@ def update_correlation_context_header(
             key_values.append((MAX_VARIANTS_KEY, str(self.max_variants)))
 
         # Add feature flag features if present
-        ff_features_string = self._create_features_string()
+        ff_features_string = self._create_ff_feature_string()
         if ff_features_string:
             key_values.append((FF_FEATURES_KEY, ff_features_string))
 
         # Add general features if present
-        features_string = self._get_features_string()
+        features_string = self._create_features_string()
         if features_string:
             key_values.append((FEATURES_KEY, features_string))
 
@@ -313,6 +235,67 @@ def update_correlation_context_header(
 
         return headers
 
+    def update_feature_filter_telemetry(self, feature_flag) -> None:
+        """
+        Track feature filter usage for App Configuration telemetry.
+
+        :param feature_flag: The feature flag to analyze for filter usage.
+        :type feature_flag: FeatureFlagConfigurationSetting
+        """
+        # Constants are already imported at module level
+
+        if feature_flag.filters:
+            for filter in feature_flag.filters:
+                if filter.get("name") in PERCENTAGE_FILTER_NAMES:
+                    self.feature_filter_usage[PERCENTAGE_FILTER_KEY] = True
+                elif filter.get("name") in TIME_WINDOW_FILTER_NAMES:
+                    self.feature_filter_usage[TIME_WINDOW_FILTER_KEY] = True
+                elif filter.get("name") in TARGETING_FILTER_NAMES:
+                    self.feature_filter_usage[TARGETING_FILTER_KEY] = True
+                else:
+                    self.feature_filter_usage[CUSTOM_FILTER_KEY] = True
+
+    def reset_feature_filter_usage(self) -> None:
+        """Reset the feature filter usage tracking."""
+        self.feature_filter_usage = {}
+
+    def _create_features_string(self) -> str:
+        """
+        Generate the features string for correlation context.
+
+        :return: A string containing features used separated by delimiters.
+        :rtype: str
+        """
+        features_list = []
+
+        if self.uses_load_balancing:
+            features_list.append(LOAD_BALANCING_FEATURE)
+        if self.uses_ai_configuration:
+            features_list.append(AI_CONFIGURATION_FEATURE)
+        if self.uses_aicc_configuration:
+            features_list.append(AI_CHAT_COMPLETION_FEATURE)
+        if self.get_assembly_version(AZURE_AI_PROJECTS_PACKAGE):
+            features_list.append(AI_FOUNDRY_SDK_FEATURE)
+
+        return Delimiter.join(features_list)
+
+    def _create_ff_feature_string(self) -> str:
+        """
+        Generate the features string for feature flag usage tracking.
+
+        :return: A string containing feature flag usage tags separated by delimiters.
+        :rtype: str
+        """
+        features_list = []
+
+        if self.uses_seed:
+            features_list.append(FEATURE_FLAG_USES_SEED_TAG)
+
+        if self.uses_telemetry:
+            features_list.append(FEATURE_FLAG_USES_TELEMETRY_TAG)
+
+        return Delimiter.join(features_list)
+
     def _create_filters_string(self) -> str:
         """
         Create a string representing the feature filters in use.
@@ -333,26 +316,43 @@ def _create_filters_string(self) -> str:
 
         return Delimiter.join(filters)
 
-    def update_feature_filter_telemetry(self, feature_flag) -> None:
+    @staticmethod
+    def get_host_type() -> str:
         """
-        Track feature filter usage for App Configuration telemetry.
+        Detect the host environment type based on environment variables.
 
-        :param feature_flag: The feature flag to analyze for filter usage.
-        :type feature_flag: FeatureFlagConfigurationSetting
+        :return: The detected host type.
+        :rtype: str
         """
-        # Constants are already imported at module level
+        if os.environ.get(AzureFunctionEnvironmentVariable):
+            return HostType.AZURE_FUNCTION
+        if os.environ.get(AzureWebAppEnvironmentVariable):
+            return HostType.AZURE_WEB_APP
+        if os.environ.get(ContainerAppEnvironmentVariable):
+            return HostType.CONTAINER_APP
+        if os.environ.get(KubernetesEnvironmentVariable):
+            return HostType.KUBERNETES
+        if os.environ.get(ServiceFabricEnvironmentVariable):
+            return HostType.SERVICE_FABRIC
 
-        if feature_flag.filters:
-            for filter in feature_flag.filters:
-                if filter.get("name") in PERCENTAGE_FILTER_NAMES:
-                    self.feature_filter_usage[PERCENTAGE_FILTER_KEY] = True
-                elif filter.get("name") in TIME_WINDOW_FILTER_NAMES:
-                    self.feature_filter_usage[TIME_WINDOW_FILTER_KEY] = True
-                elif filter.get("name") in TARGETING_FILTER_NAMES:
-                    self.feature_filter_usage[TARGETING_FILTER_KEY] = True
-                else:
-                    self.feature_filter_usage[CUSTOM_FILTER_KEY] = True
+        return HostType.UNIDENTIFIED
 
-    def reset_feature_filter_usage(self) -> None:
-        """Reset the feature filter usage tracking."""
-        self.feature_filter_usage = {}
+    @staticmethod
+    def get_assembly_version(package_name: str) -> Optional[str]:
+        """
+        Get the version of a Python package.
+
+        :param package_name: The name of the package to get version for.
+        :type package_name: str
+        :return: Package version string or None if not found.
+        :rtype: Optional[str]
+        """
+        if not package_name:
+            return None
+
+        try:
+            return version(package_name)
+        except PackageNotFoundError:
+            pass
+
+        return None

sdk/appconfiguration/azure-appconfiguration-provider/tests/test_request_tracing_context.py

@@ -73,31 +73,31 @@ def testupdate_max_variants(self):
         self.context.update_max_variants(3)
         self.assertEqual(self.context.max_variants, 10)
 
-    def test__get_features_string_empty(self):
-        """Test _get_features_string with no features enabled."""
-        result = self.context._get_features_string()
+    def test__create_features_string_empty(self):
+        """Test _create_features_string with no features enabled."""
+        result = self.context._create_features_string()
         self.assertEqual(result, "")
 
-    def test__get_features_string_with_features(self):
-        """Test _get_features_string with features enabled."""
+    def test__create_features_string_with_features(self):
+        """Test _create_features_string with features enabled."""
         self.context.uses_load_balancing = True
         self.context.uses_ai_configuration = True
         self.context.uses_aicc_configuration = True
 
-        result = self.context._get_features_string()
+        result = self.context._create_features_string()
         self.assertEqual(result, "LB+AI+AICC")
 
-    def test__create_features_string_empty(self):
-        """Test _create_features_string with no FF features enabled."""
-        result = self.context._create_features_string()
+    def test__create_ff_features_string_empty(self):
+        """Test _create_feature_string with no FF features enabled."""
+        result = self.context._create_ff_feature_string()
         self.assertEqual(result, "")
 
-    def test__create_features_string_with_features(self):
-        """Test _create_features_string with FF features enabled."""
+    def test__create_ff_features_string_with_features(self):
+        """Test _create_ff_feature_string with FF features enabled."""
         self.context.uses_seed = True
         self.context.uses_telemetry = True
 
-        result = self.context._create_features_string()
+        result = self.context._create_ff_feature_string()
         expected = f"{FEATURE_FLAG_USES_SEED_TAG}+{FEATURE_FLAG_USES_TELEMETRY_TAG}"
         self.assertEqual(result, expected)