Alda/numeric metrics (#76)

* Added a helper method to find numeric metric values in the event tags.

* Added tests for the numeric metric value helper method.

* Renamed REVENUE_METRIC_TYPE enum to EVENT_VALUE_METRIC to be consistent with Java.

* Replaced hard-coded revenue tag with enum. Renamed the local variable event value to revenue value to better describe its reference.

* Added a check for numeric metric value in the event builder.

* Fixed unit tests for numeric metric.

* Changed event value test names to event tags. Updated test comment descriptions.

* Added a couple more unit tests to test track and conversion event generation are valid when only revenue or numeric metric event tags are provided.

* Fixed lint errors.

* Enabled max diff to see unit test error on travis.

* Fixed nit spacing and wording.

* Sort the event tag keys as they were being returned in an undeterministic way, which made some unit tests inconsistent. Event tag keys is a short list, so the sort should not create overhead.

* Refactored numeric metric unit tests to use a subroutine and isolated event object checks to parameters related to event tags so that the unit tests won't be brittle.

* Handled corner cases for the get numeric value better. Strings and numbers are supported; everything else should return None.

* Sorted the event features results in the test so we don't have to sort it in the event builder.

* Fixed an import statement.

* Removed basestring as this is not supported by Python3.

* Updated some code comments.

* Cleaned up some wording and unit test ordering.

* Removed blank line at end of file.

Author: alda-optimizely

optimizely/event_builder.py

@@ -197,13 +197,23 @@ def _add_required_params_for_conversion(self, event_key, event_tags, decisions):
     self.params[self.EventParams.EVENT_METRICS] = []
 
     if event_tags:
-      event_value = event_tag_utils.get_revenue_value(event_tags)
-      if event_value is not None:
-        self.params[self.EventParams.EVENT_METRICS] = [{
-          'name': event_tag_utils.EVENT_VALUE_METRIC,
-          'value': event_value
-        }]
-
+      event_values = []
+
+      revenue_value = event_tag_utils.get_revenue_value(event_tags)
+      if revenue_value is not None:
+         event_values.append({
+          'name': event_tag_utils.REVENUE_METRIC_TYPE,
+          'value': revenue_value
+        })
+
+      numeric_value = event_tag_utils.get_numeric_value(event_tags, self.config.logger)
+      if numeric_value is not None:
+         event_values.append({
+          'name': event_tag_utils.NUMERIC_METRIC_TYPE,
+          'value': numeric_value
+        })
+
+      self.params[self.EventParams.EVENT_METRICS] = event_values
       for event_tag_id in event_tags.keys():
         event_tag_value = event_tags.get(event_tag_id)
         if event_tag_value is None:
@@ -422,9 +432,13 @@ def _add_required_params_for_conversion(self, event_key, event_tags, decisions):
         }
 
         if event_tags:
-          event_value = event_tag_utils.get_revenue_value(event_tags)
-          if event_value is not None:
-            event_dict['revenue'] = event_value
+          revenue_value = event_tag_utils.get_revenue_value(event_tags)
+          if revenue_value is not None:
+            event_dict[event_tag_utils.REVENUE_METRIC_TYPE] = revenue_value
+
+          numeric_value = event_tag_utils.get_numeric_value(event_tags, self.config.logger)
+          if numeric_value is not None:
+            event_dict[event_tag_utils.NUMERIC_METRIC_TYPE] = numeric_value
 
           if len(event_tags) > 0:
             event_dict[self.EventParams.TAGS] = event_tags

optimizely/helpers/event_tag_utils.py

@@ -11,9 +11,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from . import enums
+import math
 import numbers
 
-EVENT_VALUE_METRIC = 'revenue'
+REVENUE_METRIC_TYPE = 'revenue'
+NUMERIC_METRIC_TYPE = 'value'
 
 
 def get_revenue_value(event_tags):
@@ -23,12 +26,94 @@ def get_revenue_value(event_tags):
   if not isinstance(event_tags, dict):
     return None
 
-  if EVENT_VALUE_METRIC not in event_tags:
+  if REVENUE_METRIC_TYPE not in event_tags:
     return None
 
-  raw_value = event_tags[EVENT_VALUE_METRIC]
+  raw_value = event_tags[REVENUE_METRIC_TYPE]
 
   if not isinstance(raw_value, numbers.Integral):
     return None
 
   return raw_value
+
+
+def get_numeric_value(event_tags, logger=None):
+  """
+  A smart getter of the numeric value from the event tags.
+
+  Args:
+      event_tags: A dictionary of event tags.
+      logger: Optional logger.
+
+  Returns:
+      A float numeric metric value is returned when the provided numeric
+      metric value is in the following format:
+          - A string (properly formatted, e.g., no commas)
+          - An integer
+          - A float or double
+      None is returned when the provided numeric metric values is in
+      the following format:
+          - None
+          - A boolean
+          - inf, -inf, nan
+          - A string not properly formatted (e.g., '1,234')
+          - Any values that cannot be cast to a float (e.g., an array or dictionary)
+  """
+
+  logger_message_debug = None
+  numeric_metric_value = None
+
+  if event_tags is None:
+    logger_message_debug = 'Event tags is undefined.'
+  elif not isinstance(event_tags, dict):
+    logger_message_debug = 'Event tags is not a dictionary.'
+  elif NUMERIC_METRIC_TYPE not in event_tags:
+    logger_message_debug = 'The numeric metric key is not in event tags.'
+  else:
+    numeric_metric_value = event_tags[NUMERIC_METRIC_TYPE]
+    try:
+      if isinstance(numeric_metric_value, (numbers.Integral, float, str)):
+        # Attempt to convert the numeric metric value to a float
+        # (if it isn't already a float).
+        cast_numeric_metric_value = float(numeric_metric_value)
+
+        # If not a float after casting, then make everything else a None.
+        # Other potential values are nan, inf, and -inf.
+        if not isinstance(cast_numeric_metric_value, float) \
+              or math.isnan(cast_numeric_metric_value) \
+              or math.isinf(cast_numeric_metric_value):
+          logger_message_debug = 'Provided numeric value {} is in an invalid format.'\
+            .format(numeric_metric_value)
+          numeric_metric_value = None
+        else:
+          # Handle booleans as a special case.
+          # They are treated like an integer in the cast, but we do not want to cast this.
+         if isinstance(numeric_metric_value, bool):
+            logger_message_debug = 'Provided numeric value is a boolean, which is an invalid format.'
+            numeric_metric_value = None
+         else:
+           numeric_metric_value = cast_numeric_metric_value
+      else:
+        logger_message_debug = 'Numeric metric value is not in integer, float, or string form.'
+
+    except ValueError:
+      logger_message_debug = 'Value error while casting numeric metric value to a float.'
+      numeric_metric_value = None
+
+  # Log all potential debug messages while converting the numeric value to a float.
+  if logger and logger_message_debug:
+    logger.log(enums.LogLevels.DEBUG, logger_message_debug)
+
+  # Log the final numeric metric value
+  if numeric_metric_value is not None:
+    if logger:
+      logger.log(enums.LogLevels.INFO,
+                 'The numeric metric value {} will be sent to results.'
+                 .format(numeric_metric_value))
+  else:
+    if logger:
+      logger.log(enums.LogLevels.WARNING,
+                 'The provided numeric metric value {} is in an invalid format and will not be sent to results.'
+                 .format(numeric_metric_value))
+
+  return numeric_metric_value

optimizely/optimizely.py

@@ -21,6 +21,7 @@
 from .error_handler import NoOpErrorHandler as noop_error_handler
 from .event_dispatcher import EventDispatcher as default_event_dispatcher
 from .helpers import enums
+from .helpers import event_tag_utils
 from .helpers import validator
 from .logger import NoOpLogger as noop_logger
 from .logger import SimpleLogger

tests/helpers_tests/test_event_tag_utils.py

@@ -11,7 +11,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import sys
 import unittest
+from optimizely import logger
 
 from optimizely.helpers import event_tag_utils
 
@@ -49,3 +51,78 @@ def test_get_revenue_value__revenue_tag(self):
     self.assertEqual(0, event_tag_utils.get_revenue_value({'revenue': 0}))
     self.assertEqual(65536, event_tag_utils.get_revenue_value({'revenue': 65536}))
     self.assertEqual(9223372036854775807, event_tag_utils.get_revenue_value({'revenue': 9223372036854775807}))
+
+  def test_get_numeric_metric__invalid_args(self):
+    """ Test that numeric value is not returned for invalid arguments. """
+    self.assertIsNone(event_tag_utils.get_numeric_value(None))
+    self.assertIsNone(event_tag_utils.get_numeric_value(0.5))
+    self.assertIsNone(event_tag_utils.get_numeric_value(65536))
+    self.assertIsNone(event_tag_utils.get_numeric_value(9223372036854775807))
+    self.assertIsNone(event_tag_utils.get_numeric_value('9223372036854775807'))
+    self.assertIsNone(event_tag_utils.get_numeric_value(True))
+    self.assertIsNone(event_tag_utils.get_numeric_value(False))
+
+  def test_get_numeric_metric__no_value_tag(self):
+    """ Test that numeric value is not returned when there's no numeric event tag. """
+    self.assertIsNone(event_tag_utils.get_numeric_value([]))
+    self.assertIsNone(event_tag_utils.get_numeric_value({}))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': 42}))
+
+  def test_get_numeric_metric__invalid_value_tag(self):
+    """ Test that numeric value is not returned when revenue event tag has invalid data type. """
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': None}))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': 0.5}))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': 12345}))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': '65536'}))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': True}))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': False}))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': [1, 2, 3]}))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'non-value': {'a', 'b', 'c'}}))
+
+  def test_get_numeric_metric__value_tag(self):
+    """ Test that the correct numeric value is returned. """
+
+    # An integer should be cast to a float
+    self.assertEqual(12345.0, event_tag_utils.get_numeric_value({'value': 12345}, logger=logger.SimpleLogger()))
+
+    # A string should be cast to a float
+    self.assertEqual(12345.0, event_tag_utils.get_numeric_value({'value': '12345'}, logger=logger.SimpleLogger()))
+
+    # Valid float values
+    some_float = 1.2345
+    self.assertEqual(some_float, event_tag_utils.get_numeric_value({'value': some_float}, logger=logger.SimpleLogger()))
+
+    max_float = sys.float_info.max
+    self.assertEqual(max_float, event_tag_utils.get_numeric_value({'value': max_float}, logger=logger.SimpleLogger()))
+
+    min_float = sys.float_info.min
+    self.assertEqual(min_float, event_tag_utils.get_numeric_value({'value': min_float}, logger=logger.SimpleLogger()))
+
+    # Invalid values
+    self.assertIsNone(event_tag_utils.get_numeric_value({'value': False}, logger=logger.SimpleLogger()))
+    self.assertIsNone(event_tag_utils.get_numeric_value({'value': None}, logger=logger.SimpleLogger()))
+
+    numeric_value_nan = event_tag_utils.get_numeric_value({'value': float('nan')}, logger=logger.SimpleLogger())
+    self.assertIsNone(numeric_value_nan, 'nan numeric value is {}'.format(numeric_value_nan))
+
+    numeric_value_array = event_tag_utils.get_numeric_value({'value': []}, logger=logger.SimpleLogger())
+    self.assertIsNone(numeric_value_nan, 'Array numeric value is {}'.format(numeric_value_array))
+
+    numeric_value_none = event_tag_utils.get_numeric_value({'value': None}, logger=logger.SimpleLogger())
+    self.assertIsNone(numeric_value_none, 'None numeric value is {}'.format(numeric_value_none))
+
+    numeric_value_invalid_literal = event_tag_utils.get_numeric_value({'value': '1,234'}, logger=logger.SimpleLogger())
+    self.assertIsNone(numeric_value_invalid_literal, 'Invalid string literal value is {}'
+                      .format(numeric_value_invalid_literal))
+
+    numeric_value_overflow = event_tag_utils.get_numeric_value({'value': sys.float_info.max * 10},
+                                                               logger=logger.SimpleLogger())
+    self.assertIsNone(numeric_value_overflow, 'Max numeric value is {}'.format(numeric_value_overflow))
+
+    numeric_value_inf = event_tag_utils.get_numeric_value({'value': float('inf')}, logger=logger.SimpleLogger())
+    self.assertIsNone(numeric_value_inf, 'Infinity numeric value is {}'.format(numeric_value_inf))
+
+    numeric_value_neg_inf = event_tag_utils.get_numeric_value({'value': float('-inf')}, logger=logger.SimpleLogger())
+    self.assertIsNone(numeric_value_neg_inf, 'Negative infinity numeric value is {}'.format(numeric_value_neg_inf))
+
+    self.assertEqual(0.0, event_tag_utils.get_numeric_value({'value': 0.0}, logger=logger.SimpleLogger()))