FFM-5955 - Metrics fixes (#48)

* FFM-5995 Start storing unique targets separately in the analyitcs class

* FFM-5995 Start storing unique targets separately in the analyitcs class

* FFM-5995 Start refactoring to build unique target data

* FFM-5995 Start refactoring to build unique target data

* FFM-5995 Use new variation value

* FFM-5995 tidy up

* FFM-5995 tidy up

* FFM-5995 lint

* FFM-5995 lint

* FFM-5995 lint

* FFM-5995 Stop early client init variations getting posted to metrics

* FFM-5995 Use != to compare constant literals per linter

* FFM-5995 Linter

Author: erdirowlands

featureflags/analytics.py

@@ -1,6 +1,6 @@
 import time
 from threading import Lock, Thread
-from typing import Dict, List
+from typing import Dict, List, Union
 
 import attr
 
@@ -11,6 +11,7 @@
 from .api.default.post_metrics import sync_detailed as post_metrics
 from .config import Config
 from .evaluations.target import Target
+from .evaluations.target_attributes import TargetAttributes
 from .evaluations.variation import Variation
 from .models.key_value import KeyValue
 from .models.metrics import Metrics
@@ -23,7 +24,7 @@
 FEATURE_IDENTIFIER_ATTRIBUTE = 'featureIdentifier'
 FEATURE_NAME_ATTRIBUTE = 'featureName'
 VARIATION_IDENTIFIER_ATTRIBUTE = 'variationIdentifier'
-VARIATION_VALUE_ATTRIBUTE = 'featureValue'
+VARIATION_VALUE_ATTRIBUTE = 'variationValue'
 TARGET_ATTRIBUTE = 'target'
 SDK_VERSION_ATTRIBUTE = 'SDK_VERSION'
 SDK_VERSION = '1.0.0'
@@ -42,6 +43,13 @@ class AnalyticsEvent(object):
     count: int = 0
 
 
+@attr.s(auto_attribs=True)
+class MetricTargetData(object):
+    identifier: str
+    name: str
+    attributes: Union[Unset, TargetAttributes] = Unset
+
+
 class AnalyticsService(object):
 
     def __init__(self, config: Config, client: AuthenticatedClient,
@@ -51,6 +59,7 @@ def __init__(self, config: Config, client: AuthenticatedClient,
         self._client = client
         self._environment = environment
         self._data: Dict[str, AnalyticsEvent] = {}
+        self._target_data: Dict[str, MetricTargetData] = {}
 
         self._running = False
         self._runner = Thread(target=self._sync)
@@ -67,15 +76,33 @@ def enqueue(self, target: Target, identifier: str,
 
         self._lock.acquire()
         try:
-            key = self.get_key(event)
-            if key in self._data:
-                self._data[key].count += 1
+            # Store unique evaluation events. We map a unique evaluation
+            # event to its count.
+            unique_evaluation_key = self.get_key(event)
+            if unique_evaluation_key in self._data:
+                self._data[unique_evaluation_key].count += 1
             else:
                 event.count = 1
-                self._data[key] = event
+                self._data[unique_evaluation_key] = event
+
+            # Store unique targets. If the target already exists
+            # just ignore it.
+            if event.target is not None and not event.target.anonymous:
+                unique_target_key = self.get_target_key(event)
+                if unique_target_key not in self._target_data:
+                    target_name = event.target.name
+                    # If the target has no name use the identifier
+                    if not target_name:
+                        target_name = event.target.identifier
+                    self._target_data[unique_target_key] = MetricTargetData(
+                        identifier=event.target.identifier,
+                        name=target_name,
+                        attributes=event.target.attributes
+                    )
         finally:
             self._lock.release()
 
+    # Returns a key for unique evaluations events.
     def get_key(self, event: AnalyticsEvent) -> str:
         return '{feature}-{variation}-{value}-{target}'.format(
             feature=event.flag_identifier,
@@ -84,6 +111,11 @@ def get_key(self, event: AnalyticsEvent) -> str:
             target=GLOBAL_TARGET,
         )
 
+    # Returns a key for unique targets. Targets are considered unique
+    # if they have different identifiers.
+    def get_target_key(self, event: AnalyticsEvent) -> str:
+        return event.target.identifier
+
     def _sync(self) -> None:
         if not self._running:
             log.info("Starting AnalyticsService with request interval: %d",
@@ -103,22 +135,6 @@ def _send_data(self) -> None:
         metrics_data: List[MetricsData] = []
         try:
             for _, event in self._data.items():
-                if event.target is not None and not event.target.anonymous:
-                    target_attributes: List[KeyValue] = []
-                    if not isinstance(event.target.attributes, Unset):
-                        for key, value in event.target.attributes.items():
-                            target_attributes.append(KeyValue(key, value))
-                    target_name = event.target.identifier
-                    if event.target.name:
-                        target_name = event.target.name
-
-                    td = TargetData(
-                        identifier=event.target.identifier,
-                        name=target_name,
-                        attributes=target_attributes
-                    )
-                    target_data.append(td)
-
                 metric_attributes: List[KeyValue] = [
                     KeyValue(FEATURE_IDENTIFIER_ATTRIBUTE,
                              event.flag_identifier),
@@ -140,8 +156,20 @@ def _send_data(self) -> None:
                     attributes=metric_attributes
                 )
                 metrics_data.append(md)
+            for _, unique_target in self._target_data.items():
+                target_attributes: List[KeyValue] = []
+                if not isinstance(unique_target.attributes, Unset):
+                    for key, value in unique_target.attributes.items():
+                        target_attributes.append(KeyValue(key, value))
+                td = TargetData(
+                    identifier=unique_target.identifier,
+                    name=unique_target.name,
+                    attributes=target_attributes
+                )
+                target_data.append(td)
         finally:
             self._data = {}
+            self._target_data = {}
             self._lock.release()
         body: Metrics = Metrics(target_data=target_data,
                                 metrics_data=metrics_data)

featureflags/client.py

@@ -118,37 +118,57 @@ def authenticate(self):
     def bool_variation(self, identifier: str, target: Target,
                        default: bool) -> bool:
         variation = self._evaluator.evaluate(identifier, target)
-        if self._config.enable_analytics:
+        # Only register metrics if analytics is enabled,
+        # and sometimes when the SDK starts up we can
+        # evaluate before the flag is cached which results in
+        # an empty identifier.
+        if self._config.enable_analytics and variation.identifier != "":
             self._analytics.enqueue(target, identifier, variation)
         return variation.bool(default)
 
     def int_variation(self, identifier: str, target: Target,
                       default: int) -> int:
         variation = self._evaluator.evaluate(identifier, target)
-        if self._config.enable_analytics:
+        # Only register metrics if analytics is enabled,
+        # and sometimes when the SDK starts up we can
+        # evaluate before the flag is cached which results in
+        # an empty identifier.
+        if self._config.enable_analytics and variation.identifier != "":
             self._analytics.enqueue(target, identifier, variation)
         return variation.int(default)
 
     def number_variation(self, identifier: str, target: Target,
                          default: float) -> float:
         variation = self._evaluator.evaluate(
             identifier, target)
-        if self._config.enable_analytics:
+        # Only register metrics if analytics is enabled,
+        # and sometimes when the SDK starts up we can
+        # evaluate before the flag is cached which results in
+        # an empty identifier.
+        if self._config.enable_analytics and variation.identifier != "":
             self._analytics.enqueue(target, identifier, variation)
         return variation.number(default)
 
     def string_variation(self, identifier: str, target: Target,
                          default: str) -> str:
         variation = self._evaluator.evaluate(
             identifier, target)
-        if self._config.enable_analytics:
+        # Only register metrics if analytics is enabled,
+        # and sometimes when the SDK starts up we can
+        # evaluate before the flag is cached which results in
+        # an empty identifier.
+        if self._config.enable_analytics and variation.identifier != "":
             self._analytics.enqueue(target, identifier, variation)
         return variation.string(default)
 
     def json_variation(self, identifier: str, target: Target,
                        default: Dict[str, Any]) -> Dict[str, Any]:
         variation = self._evaluator.evaluate(identifier, target)
-        if self._config.enable_analytics:
+        # Only register metrics if analytics is enabled,
+        # and sometimes when the SDK starts up we can
+        # evaluate before the flag is cached which results in
+        # an empty identifier.
+        if self._config.enable_analytics and variation.identifier != "":
             self._analytics.enqueue(target, identifier, variation)
         return variation.json(default)
 

featureflags/evaluations/evaluator.py

@@ -113,7 +113,7 @@ def _check_target_in_segment(self, segments: List[str],
 
                 # Should Target be included via segment rules
                 if segment.rules and self._evaluate_clauses(segment.rules,
-                                                                  target):
+                                                            target):
                     log.debug('Target %s included in segment %s via rules\n',
                               target.name, segment.name)
                     return True

tests/integration/test_evaluator.py

@@ -48,7 +48,8 @@ def from_dict(cls, source: Dict[str, Any]) -> 'Feature':
                         for segment in source["segments"]]
         targets = []
         if "targets" in source:
-            targets = [Target.from_dict(target) for target in source["targets"]]
+            targets = [Target.from_dict(target) for
+                       target in source["targets"]]
 
         tests = [Test.from_dict(test) for test in source["tests"]]