Fix bug in updating aggregation map. (#253)

* Fix bug in updating aggregation map.

* Update unit tests.

* Use tag_value_aggregation_data_map instead of tag_value_aggregation_map.

Author: songy23

examples/stats/helloworld/main.py

@@ -61,9 +61,9 @@ def main():
     # Get aggregated stats and print it to console.
     view_data = view_manager.get_view(VIDEO_SIZE_VIEW_NAME)
     pprint(vars(view_data))
-    for k, v in view_data._tag_value_aggregation_map.items():
+    for k, v in view_data._tag_value_aggregation_data_map.items():
+      pprint(k)
       pprint(vars(v))
-      pprint(vars(v.aggregation_data))
 
 
 if __name__ == '__main__':

opencensus/stats/view_data.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 from datetime import datetime
-import logging
+import copy
 
 
 class ViewData(object):
@@ -36,8 +36,7 @@ def __init__(self,
         self._view = view
         self._start_time = start_time
         self._end_time = end_time
-        self._tag_value_aggregation_map = {}
-        self._tag_map = {}
+        self._tag_value_aggregation_data_map = {}
 
     @property
     def view(self):
@@ -55,13 +54,9 @@ def end_time(self):
         return self._end_time
 
     @property
-    def tag_value_aggregation_map(self):
+    def tag_value_aggregation_data_map(self):
         """the current tag value aggregation map in the view data"""
-        return self._tag_value_aggregation_map
-
-    @property
-    def tag_map(self):
-        return self._tag_map
+        return self._tag_value_aggregation_data_map
 
     def start(self):
         """sets the start time for the view data"""
@@ -71,19 +66,6 @@ def end(self):
         """sets the end time for the view data"""
         self._end_time = datetime.utcnow().isoformat() + 'Z'
 
-    def get_tag_map(self, context):
-        """function to return the tag map based on the context"""
-        if self.tag_map is not None:
-            if context.map.items() <= self.tag_map.items():
-                return self.tag_map
-            else:
-                tags = self.tag_map
-                for tag_key, tag_value in context.map.items():
-                    tags[tag_key] = tag_value
-                return tags
-        else:  # pragma: NO COVER
-            logging.warning("Tag Map cannot be none")
-
     def get_tag_values(self, tags, columns):
         """function to get the tag values from tags and columns"""
         tag_values = []
@@ -99,11 +81,10 @@ def get_tag_values(self, tags, columns):
 
     def record(self, context, value, timestamp):
         """records the view data against context"""
-        tag_values = self.get_tag_values(tags=self.get_tag_map(context),
+        tag_values = self.get_tag_values(tags=context.map,
                                          columns=self.view.columns)
         tuple_vals = tuple(tag_values)
-        for val in tuple_vals:
-            if val not in self.tag_value_aggregation_map:
-                self.tag_value_aggregation_map[val] = self.view.aggregation
-            self.tag_value_aggregation_map.get(
-                val).aggregation_data.add_sample(value)
+        if tuple_vals not in self.tag_value_aggregation_data_map:
+            self.tag_value_aggregation_data_map[tuple_vals] = copy.deepcopy(
+                self.view.aggregation.aggregation_data)
+        self.tag_value_aggregation_data_map.get(tuple_vals).add_sample(value)

tests/unit/stats/test_view_data.py

@@ -15,6 +15,8 @@
 import unittest
 import mock
 from datetime import datetime
+from opencensus.stats import aggregation as aggregation_module
+from opencensus.stats import view as view_module
 from opencensus.stats import view_data as view_data_module
 
 
@@ -29,7 +31,7 @@ def test_constructor(self):
         self.assertEqual(view, view_data.view)
         self.assertEqual(start_time, view_data.start_time)
         self.assertEqual(end_time, view_data.end_time)
-        self.assertEqual({}, view_data.tag_value_aggregation_map)
+        self.assertEqual({}, view_data.tag_value_aggregation_data_map)
 
     def test_start(self):
         view = mock.Mock()
@@ -53,29 +55,6 @@ def test_end(self):
 
         self.assertIsNotNone(view_data.end_time)
 
-    def test_get_tag_map(self):
-        view = mock.Mock()
-        start_time = datetime.utcnow()
-        end_time = datetime.utcnow()
-        view_data = view_data_module.ViewData(view=view,
-                                              start_time=start_time,
-                                              end_time=end_time)
-        test_context_1 = mock.Mock()
-        test_context_1.map = {'key1': 'val1'}
-        context_map_1 = view_data.get_tag_map(context=test_context_1)
-        self.assertEqual(test_context_1.map, view_data.tag_map)
-        self.assertEqual(test_context_1.map, context_map_1)
-
-        test_context_2 = mock.Mock()
-        test_context_2.map = {'key1': 'val2'}
-        context_map_2 = view_data.get_tag_map(context=test_context_2)
-        self.assertEqual(test_context_2.map, context_map_2)
-
-        test_context_3 = mock.Mock()
-        test_context_3.map = {}
-        context_map_3 = view_data.get_tag_map(context=test_context_3)
-        self.assertEqual(test_context_2.map, context_map_3)
-
     def test_get_tag_values(self):
         view = mock.Mock()
         start_time = datetime.utcnow()
@@ -108,26 +87,101 @@ def test_record(self):
         context.map = {'key1': 'val1', 'key2': 'val2'}
         time = datetime.utcnow().isoformat() + 'Z'
         value = 1
-        self.assertEqual({}, view_data.tag_value_aggregation_map)
+        self.assertEqual({}, view_data.tag_value_aggregation_data_map)
 
         view_data.record(context=context, value=value, timestamp=time)
         tag_values = view_data.get_tag_values(
-            tags=view_data.get_tag_map(context=context), columns=view.columns)
+            tags=context.map, columns=view.columns)
         tuple_vals = tuple(tag_values)
         self.assertEqual(['val1'], tag_values)
-        self.assertIsNotNone(view_data.tag_value_aggregation_map)
+        self.assertIsNotNone(view_data.tag_value_aggregation_data_map)
 
-        self.assertTrue('val1' in view_data.tag_value_aggregation_map)
-        self.assertTrue('val1' in tag_values)
-        self.assertIsNotNone(view_data.tag_value_aggregation_map['val1'])
-        self.assertIsNotNone(view_data.tag_value_aggregation_map.get(
-            'val1').add(value))
+        self.assertTrue(tuple_vals in view_data.tag_value_aggregation_data_map)
+        self.assertIsNotNone(view_data.tag_value_aggregation_data_map[tuple_vals])
+        self.assertIsNotNone(view_data.tag_value_aggregation_data_map.get(
+            tuple_vals).add(value))
 
         view_data.record(context=context, value=value, timestamp=time)
         tag_values.append('val2')
-        self.assertFalse('val2' in view_data.tag_value_aggregation_map)
-        view_data.tag_value_aggregation_map['val2'] = view.aggregation
-        self.assertEqual(view_data.tag_value_aggregation_map.get('val2'),
+        tuple_vals_2 = tuple(['val2'])
+        self.assertFalse(tuple_vals_2 in view_data.tag_value_aggregation_data_map)
+        view_data.tag_value_aggregation_data_map[tuple_vals_2] = view.aggregation
+        self.assertEqual(view_data.tag_value_aggregation_data_map.get(tuple_vals_2),
                          view_data.view.aggregation)
-        self.assertIsNotNone(view_data.tag_value_aggregation_map.get(
-            'val2').add(value))
+        self.assertIsNotNone(view_data.tag_value_aggregation_data_map.get(
+            tuple_vals_2).add(value))
+
+
+    def test_record_with_multi_keys(self):
+        measure = mock.Mock()
+        sum_aggregation = aggregation_module.SumAggregation()
+        view = view_module.View(
+            "test_view", "description", ['key1', 'key2'], measure, sum_aggregation)
+        start_time = datetime.utcnow()
+        end_time = datetime.utcnow()
+        view_data = view_data_module.ViewData(view=view,
+                                              start_time=start_time,
+                                              end_time=end_time)
+
+        context = mock.Mock()
+        context.map = {'key1': 'val1', 'key2': 'val2'}
+        time = datetime.utcnow().isoformat() + 'Z'
+        value = 1
+        self.assertEqual({}, view_data.tag_value_aggregation_data_map)
+
+        view_data.record(context=context, value=value, timestamp=time)
+        tag_values = view_data.get_tag_values(
+            tags=context.map, columns=view.columns)
+        tuple_vals = tuple(tag_values)
+        self.assertEqual(['val1', 'val2'], tag_values)
+        self.assertIsNotNone(view_data.tag_value_aggregation_data_map)
+        self.assertTrue(tuple_vals in view_data.tag_value_aggregation_data_map)
+        self.assertIsNotNone(view_data.tag_value_aggregation_data_map[tuple_vals])
+        sum_data = view_data.tag_value_aggregation_data_map.get(tuple_vals)
+        self.assertEqual(1, sum_data.sum_data)
+
+        context_2 = mock.Mock()
+        context_2.map = {'key1': 'val3', 'key2': 'val2'}
+        time_2 = datetime.utcnow().isoformat() + 'Z'
+        value_2 = 2
+        view_data.record(context=context_2, value=value_2, timestamp=time_2)
+        tag_values_2 = view_data.get_tag_values(
+            tags=context_2.map, columns=view.columns)
+        tuple_vals_2 = tuple(tag_values_2)
+        self.assertEqual(['val3', 'val2'], tag_values_2)
+        self.assertTrue(tuple_vals_2 in view_data.tag_value_aggregation_data_map)
+        sum_data_2 = view_data.tag_value_aggregation_data_map.get(tuple_vals_2)
+        self.assertEqual(2, sum_data_2.sum_data)
+
+        time_3 = datetime.utcnow().isoformat() + 'Z'
+        value_3 = 3
+        # Use the same context {'key1': 'val1', 'key2': 'val2'}.
+        # Record to entry [(val1, val2), sum=1].
+        view_data.record(context=context, value=value_3, timestamp=time_3)
+        self.assertEqual(4, sum_data.sum_data)
+        # The other entry should remain unchanged.
+        self.assertEqual(2, sum_data_2.sum_data)
+
+    def test_record_with_missing_key_in_context(self):
+        measure = mock.Mock()
+        sum_aggregation = aggregation_module.SumAggregation()
+        view = view_module.View(
+            "test_view", "description", ['key1', 'key2'], measure, sum_aggregation)
+        start_time = datetime.utcnow()
+        end_time = datetime.utcnow()
+        view_data = view_data_module.ViewData(view=view,
+                                              start_time=start_time,
+                                              end_time=end_time)
+
+        context = mock.Mock()
+        context.map = {'key1': 'val1', 'key3': 'val3'}  # key2 is not in the context.
+        time = datetime.utcnow().isoformat() + 'Z'
+        value = 4
+        view_data.record(context=context, value=value, timestamp=time)
+        tag_values = view_data.get_tag_values(
+            tags=context.map, columns=view.columns)
+        tuple_vals = tuple(tag_values)
+        self.assertEqual(['val1', None], tag_values)
+        self.assertTrue(tuple_vals in view_data.tag_value_aggregation_data_map)
+        sum_data = view_data.tag_value_aggregation_data_map.get(tuple_vals)
+        self.assertEqual(4, sum_data.sum_data)