Repository: census-instrumentation/opencensus-python
Pull request #233 (2018-07-12 21:40:27)

Stats: Fix hello world example and measure_to_view_map.
Fixes https://github.com/census-instrumentation/opencensus-python/issues/225.
Fixes https://github.com/census-instrumentation/opencensus-python/issues/229.
Fixes https://github.com/census-instrumentation/opencensus-python/issues/230.

songy23: Thanks! Finally the CI is green :)

Patch:
diff --git a/examples/stats/helloworld/main.py b/examples/stats/helloworld/main.py
index 816d708..f422bac 100644
--- a/examples/stats/helloworld/main.py
+++ b/examples/stats/helloworld/main.py
@@ -22,11 +22,12 @@ from opencensus.stats import view as view_module
 from opencensus.tags import tag_key as tag_key_module
 from opencensus.tags import tag_map as tag_map_module
 from opencensus.tags import tag_value as tag_value_module
+from pprint import pprint
 
 MiB = 1 << 20
 FRONTEND_KEY = tag_key_module.TagKey("my.org/keys/frontend")
 VIDEO_SIZE_MEASURE = measure_module.MeasureInt(
-    "my.org/measure/video_size", "size of processed videos", "By")
+    "my.org/measures/video_size", "size of processed videos", "By")
 VIDEO_SIZE_VIEW_NAME = "my.org/views/video_size"
 VIDEO_SIZE_DISTRIBUTION = aggregation_module.DistributionAggregation(
     [0.0, 16.0 * MiB, 256.0 * MiB])
@@ -51,14 +52,15 @@ def main():
     # Process video.
     # Record the processed video size.
     tag_value = tag_value_module.TagValue("mobile-ios9.3.5")
-    tag_map = tag_map_module.TagMap().insert(FRONTEND_KEY, tag_value)
+    tag_map = tag_map_module.TagMap()
+    tag_map.insert(FRONTEND_KEY, tag_value)
     measure_map = stats_recorder.new_measurement_map()
     measure_map.measure_int_put(VIDEO_SIZE_MEASURE, 25 * MiB)
     measure_map.record(tag_map)
 
     # Get aggregated stats and print it to console.
     view_data = view_manager.get_view(VIDEO_SIZE_VIEW_NAME)
-    print(view_data)  # TODO: print more meaningful info for view_data
+    pprint(vars(view_data))   # TODO: print more meaningful info for view_data
 
 
 if __name__ == '__main__':
diff --git a/opencensus/stats/measure_to_view_map.py b/opencensus/stats/measure_to_view_map.py
index 6b37d0a..93de781 100644
--- a/opencensus/stats/measure_to_view_map.py
+++ b/opencensus/stats/measure_to_view_map.py
@@ -23,9 +23,10 @@ class MeasureToViewMap(object):
     specific View Datas
 
     """
+
     def __init__(self):
         # stores the one-to-many mapping from Measures to View Datas
-        self._map = defaultdict(list)
+        self._measure_to_view_data_list_map = defaultdict(list)
         # stores a map from the registered View names to the Views
         self._registered_views = {}
         # stores a map from the registered Measure names to the Measures
@@ -44,11 +45,14 @@ class MeasureToViewMap(object):
         if view is None:
             return None
 
-        views = self._map.get(view.measure.name)
-        if views is not None:
-            for view_data in views:
+        view_data_list = self._measure_to_view_data_list_map.get(
+            view.measure.name)
+        if view_data_list is not None:
+            for view_data in view_data_list:
                 if view_data.view.name == view_name:
-                    return copy.deepcopy(view_data)
+                    view_data_copy = copy.deepcopy(view_data)
+                    view_data_copy.end()
+                    return view_data_copy
 
     def filter_exported_views(self, all_views):
         """returns the subset of the given view that should be exported"""
@@ -72,12 +76,11 @@ class MeasureToViewMap(object):
         if registered_measure is not None and registered_measure != measure:
             logging.warning(
                 "A different measure with the same name is already registered")
-        self._registered_views[measure.name] = view
+        self._registered_views[view.name] = view
         if registered_measure is None:
             self._registered_measures[measure.name] = measure
-        self._map[view.measure.name].append(ViewData(view=view,
-                                                     start_time=timestamp,
-                                                     end_time=timestamp))
+        self._measure_to_view_data_list_map[view.measure.name].append(
+            ViewData(view=view, start_time=timestamp, end_time=timestamp))
 
     def record(self, tags, measurement_map, timestamp):
         """records stats with a set of tags"""
@@ -85,11 +88,10 @@ class MeasureToViewMap(object):
             if measure != self._registered_measures.get(measure.name):
                 return
             view_datas = []
-            for key, value in self._map.items():
-                if key == measure.name:
-                    # note that self._map is a multi-map.
-                    view_datas.extend(self._map[key])
+            for measure_name, view_data_list \
+                    in self._measure_to_view_data_list_map.items():
+                if measure_name == measure.name:
+                    view_datas.extend(view_data_list)
             for view_data in view_datas:
-                view_data.record(context=tags,
-                                 value=view_data.view.measure,
-                                 timestamp=timestamp)
+                view_data.record(
+                    context=tags, value=value, timestamp=timestamp)


Test patch:
diff --git a/tests/unit/stats/test_measure_to_view_map.py b/tests/unit/stats/test_measure_to_view_map.py
index 5653e17..f7fde73 100644
--- a/tests/unit/stats/test_measure_to_view_map.py
+++ b/tests/unit/stats/test_measure_to_view_map.py
@@ -20,11 +20,11 @@ from opencensus.stats.view import View
 from opencensus.stats.view_data import ViewData
 from opencensus.stats.measurement import Measurement
 from opencensus.stats.measure import BaseMeasure
+from opencensus.stats.measure import MeasureInt
 from opencensus.stats import measure_to_view_map as measure_to_view_map_module
 
 
 class TestMeasureToViewMap(unittest.TestCase):
-
     @staticmethod
     def _get_target_class():
         return measure_to_view_map_module.MeasureToViewMap
@@ -35,7 +35,8 @@ class TestMeasureToViewMap(unittest.TestCase):
     def test_constructor(self):
         measure_to_view_map = measure_to_view_map_module.MeasureToViewMap()
 
-        self.assertEqual({}, measure_to_view_map._map)
+        self.assertEqual({},
+                         measure_to_view_map._measure_to_view_data_list_map)
         self.assertEqual({}, measure_to_view_map._registered_views)
         self.assertEqual({}, measure_to_view_map._registered_measures)
         self.assertEqual(set(), measure_to_view_map.exported_views)
@@ -46,29 +47,43 @@ class TestMeasureToViewMap(unittest.TestCase):
         columns = mock.Mock()
         measure = mock.Mock()
         aggregation = mock.Mock()
-        view = View(name=name, description=description, columns=columns, measure=measure, aggregation=aggregation)
+        view = View(
+            name=name,
+            description=description,
+            columns=columns,
+            measure=measure,
+            aggregation=aggregation)
         timestamp = mock.Mock()
         measure_to_view_map = measure_to_view_map_module.MeasureToViewMap()
 
         measure_to_view_map._registered_views = {}
-        no_registered_views = measure_to_view_map.get_view(view_name=name, timestamp=timestamp)
+        no_registered_views = measure_to_view_map.get_view(
+            view_name=name, timestamp=timestamp)
         self.assertEqual(None, no_registered_views)
 
         measure_to_view_map._registered_views = {name: view}
-        measure_to_view_map._map = {view.measure.name: [ViewData(view=view, start_time=timestamp, end_time=timestamp)]}
-        print(measure_to_view_map._map)
-        view_data = measure_to_view_map.get_view(view_name=name, timestamp=timestamp)
+        measure_to_view_map._measure_to_view_data_list_map = {
+            view.measure.name:
+            [ViewData(view=view, start_time=timestamp, end_time=timestamp)]
+        }
+        print(measure_to_view_map._measure_to_view_data_list_map)
+        view_data = measure_to_view_map.get_view(
+            view_name=name, timestamp=timestamp)
         self.assertIsNotNone(view_data)
 
-        measure_to_view_map._map = {}
-        view_data = measure_to_view_map.get_view(view_name=name,
-                                                 timestamp=timestamp)
+        measure_to_view_map._measure_to_view_data_list_map = {}
+        view_data = measure_to_view_map.get_view(
+            view_name=name, timestamp=timestamp)
         self.assertIsNone(view_data)
 
-        measure_to_view_map._map = {view.measure.name: [ViewData(
-            view=mock.Mock(), start_time=timestamp, end_time=timestamp)]}
-        view_data = measure_to_view_map.get_view(view_name=name,
-                                                 timestamp=timestamp)
+        measure_to_view_map._measure_to_view_data_list_map = {
+            view.measure.name: [
+                ViewData(
+                    view=mock.Mock(), start_time=timestamp, end_time=timestamp)
+            ]
+        }
+        view_data = measure_to_view_map.get_view(
+            view_name=name, timestamp=timestamp)
         self.assertIsNone(view_data)
 
     def test_filter_exported_views(self):
@@ -77,16 +92,27 @@ class TestMeasureToViewMap(unittest.TestCase):
         columns = mock.Mock()
         measure = mock.Mock()
         aggregation = mock.Mock()
-        test_view_1 = View(name=test_view_1_name, description=description, columns=columns, measure=measure, aggregation=aggregation)
+        test_view_1 = View(
+            name=test_view_1_name,
+            description=description,
+            columns=columns,
+            measure=measure,
+            aggregation=aggregation)
         print("test view 1", test_view_1)
 
         test_view_2_name = "testView2"
-        test_view_2 = View(name=test_view_2_name, description=description, columns=columns, measure=measure, aggregation=aggregation)
+        test_view_2 = View(
+            name=test_view_2_name,
+            description=description,
+            columns=columns,
+            measure=measure,
+            aggregation=aggregation)
         print("test view 2", test_view_2)
         all_the_views = {test_view_1, test_view_2}
         print("all the views", all_the_views)
         measure_to_view_map = measure_to_view_map_module.MeasureToViewMap()
-        views = measure_to_view_map.filter_exported_views(all_views=all_the_views)
+        views = measure_to_view_map.filter_exported_views(
+            all_views=all_the_views)
         print("filtered views", views)
         self.assertEqual(views, all_the_views)
 
@@ -94,9 +120,14 @@ class TestMeasureToViewMap(unittest.TestCase):
         name = "testView"
         description = "testDescription"
         columns = mock.Mock()
-        measure = mock.Mock()
+        measure = MeasureInt("measure", "description", "1")
         aggregation = mock.Mock()
-        view = View(name=name, description=description, columns=columns, measure=measure, aggregation=aggregation)
+        view = View(
+            name=name,
+            description=description,
+            columns=columns,
+            measure=measure,
+            aggregation=aggregation)
         timestamp = mock.Mock()
         measure_to_view_map = measure_to_view_map_module.MeasureToViewMap()
 
@@ -104,41 +135,78 @@ class TestMeasureToViewMap(unittest.TestCase):
         measure_to_view_map._registered_measures = {}
         measure_to_view_map.register_view(view=view, timestamp=timestamp)
         self.assertIsNone(measure_to_view_map.exported_views)
-        self.assertEqual(measure_to_view_map._registered_views[measure.name], view)
-        self.assertEqual(measure_to_view_map._registered_measures[measure.name], measure)
-        self.assertIsNotNone(measure_to_view_map._map[view.measure.name])
+        self.assertEqual(measure_to_view_map._registered_views[view.name],
+                         view)
+        self.assertEqual(
+            measure_to_view_map._registered_measures[measure.name], measure)
+        self.assertIsNotNone(
+            measure_to_view_map._measure_to_view_data_list_map[
+                view.measure.name])
+
+        # Registers a view with an existing measure.
+        view2 = View(
+            name="testView2",
+            description=description,
+            columns=columns,
+            measure=measure,
+            aggregation=aggregation)
+        test_with_registered_measures = measure_to_view_map.register_view(
+            view=view2, timestamp=timestamp)
+        self.assertIsNone(test_with_registered_measures)
+        self.assertEqual(
+            measure_to_view_map._registered_measures[measure.name], measure)      
 
-        test_measure = mock.Mock()
-        measure_to_view_map._registered_measures = {measure.name: test_measure}
-        test_with_registered_measures = measure_to_view_map.register_view(view=view, timestamp=timestamp)
+        # Registers a view with a measure that has the same name as an existing measure,
+        # but with different schema. measure2 and view3 should be ignored.
+        measure2 = MeasureInt("measure", "another measure", "ms")
+        view3 = View(
+            name="testView3",
+            description=description,
+            columns=columns,
+            measure=measure2,
+            aggregation=aggregation)
+        test_with_registered_measures = measure_to_view_map.register_view(
+            view=view3, timestamp=timestamp)
         self.assertIsNone(test_with_registered_measures)
-        self.assertIsNotNone(measure_to_view_map._map[view.measure.name])
+        self.assertEqual(
+            measure_to_view_map._registered_measures[measure2.name], measure) 
 
         measure_to_view_map._registered_measures = {measure.name: None}
-        self.assertIsNone(measure_to_view_map._registered_measures.get(
-            measure.name))
+        self.assertIsNone(
+            measure_to_view_map._registered_measures.get(measure.name))
         measure_to_view_map.register_view(view=view, timestamp=timestamp)
-        self.assertEqual(measure,
-                         measure_to_view_map._registered_measures[measure.name]
-                         )
-        self.assertIsNotNone(measure_to_view_map._map[view.measure.name])
+        # view is already registered, measure will not be registered again.
+        self.assertIsNone(
+            measure_to_view_map._registered_measures.get(measure.name))
+        self.assertIsNotNone(
+            measure_to_view_map._measure_to_view_data_list_map[
+                view.measure.name])
 
         measure_to_view_map._registered_views = {name: view}
-        test_result_1 = measure_to_view_map.register_view(view=view, timestamp=timestamp)
+        test_result_1 = measure_to_view_map.register_view(
+            view=view, timestamp=timestamp)
         self.assertIsNone(test_result_1)
-        self.assertIsNotNone(measure_to_view_map._map[view.measure.name])
+        self.assertIsNotNone(
+            measure_to_view_map._measure_to_view_data_list_map[
+                view.measure.name])
 
     def test_record(self):
         measure_name = "test_measure"
         measure_description = "test_description"
-        measure = BaseMeasure(name=measure_name, description=measure_description)
+        measure = BaseMeasure(
+            name=measure_name, description=measure_description)
 
         view_name = "test_view"
         view_description = "test_description"
         view_columns = ["testTag1", "testColumn2"]
         view_measure = measure
         view_aggregation = mock.Mock()
-        view = View(name=view_name, description=view_description, columns=view_columns, measure=view_measure, aggregation=view_aggregation)
+        view = View(
+            name=view_name,
+            description=view_description,
+            columns=view_columns,
+            measure=view_measure,
+            aggregation=view_aggregation)
 
         measure_value = 5
         tags = {"testTag1": "testTag1Value"}
@@ -147,47 +215,56 @@ class TestMeasureToViewMap(unittest.TestCase):
 
         measure_to_view_map = measure_to_view_map_module.MeasureToViewMap()
         measure_to_view_map._registered_measures = {}
-        record = measure_to_view_map.record(tags=tags, measurement_map=measurement_map, timestamp=timestamp)
-        self.assertNotEqual(measure,
-                            measure_to_view_map._registered_measures.get(
-                                measure.name))
+        record = measure_to_view_map.record(
+            tags=tags, measurement_map=measurement_map, timestamp=timestamp)
+        self.assertNotEqual(
+            measure,
+            measure_to_view_map._registered_measures.get(measure.name))
         self.assertIsNone(record)
 
         measure_to_view_map._registered_measures = {measure.name: measure}
-        measure_to_view_map._map = {}
-        record = measure_to_view_map.record(tags=tags, measurement_map=measurement_map,
-                                   timestamp=timestamp)
-        self.assertEqual(measure,
-                         measure_to_view_map._registered_measures.get(
-                             measure.name))
+        measure_to_view_map._measure_to_view_data_list_map = {}
+        record = measure_to_view_map.record(
+            tags=tags, measurement_map=measurement_map, timestamp=timestamp)
+        self.assertEqual(
+            measure,
+            measure_to_view_map._registered_measures.get(measure.name))
         self.assertIsNone(record)
 
-        measure_to_view_map._map = {measure.name: [mock.Mock()]}
-        measure_to_view_map.record(tags=tags, measurement_map=measurement_map,
-                                   timestamp=timestamp)
-        self.assertEqual(measure,
-                         measure_to_view_map._registered_measures.get(
-                             measure.name))
-        self.assertTrue(measure.name in measure_to_view_map._map)
+        measure_to_view_map._measure_to_view_data_list_map = {
+            measure.name: [mock.Mock()]
+        }
+        measure_to_view_map.record(
+            tags=tags, measurement_map=measurement_map, timestamp=timestamp)
+        self.assertEqual(
+            measure,
+            measure_to_view_map._registered_measures.get(measure.name))
+        self.assertTrue(
+            measure.name in measure_to_view_map._measure_to_view_data_list_map)
 
-        measure_to_view_map._map = {"testing": [mock.Mock()]}
-        measure_to_view_map.record(tags=tags, measurement_map=measurement_map,
-                                   timestamp=timestamp)
-        self.assertTrue(measure.name not in measure_to_view_map._map)
+        measure_to_view_map._measure_to_view_data_list_map = {
+            "testing": [mock.Mock()]
+        }
+        measure_to_view_map.record(
+            tags=tags, measurement_map=measurement_map, timestamp=timestamp)
+        self.assertTrue(measure.name not in
+                        measure_to_view_map._measure_to_view_data_list_map)
 
         measure_to_view_map_mock = mock.Mock()
         measure_to_view_map = measure_to_view_map_mock
         measure_to_view_map._registered_measures = {measure.name: measure}
-        measure_to_view_map._map = mock.Mock()
-        measure_to_view_map.record(tags=mock.Mock(), stats=mock.Mock(), timestamp=mock.Mock())
-        self.assertEqual(measure,
-                         measure_to_view_map._registered_measures.get(
-                             measure.name))
+        measure_to_view_map._measure_to_view_data_list_map = mock.Mock()
+        measure_to_view_map.record(
+            tags=mock.Mock(), stats=mock.Mock(), timestamp=mock.Mock())
+        self.assertEqual(
+            measure,
+            measure_to_view_map._registered_measures.get(measure.name))
         self.assertIsNotNone(measure_to_view_map.view_datas)
         self.assertTrue(measure_to_view_map_mock.record.called)
 
         tags = {"testTag1": "testTag1Value"}
         measurement_map = {}
         measure_to_view_map = measure_to_view_map_module.MeasureToViewMap()
-        record = measure_to_view_map.record(tags=tags, measurement_map=measurement_map, timestamp=timestamp)
+        record = measure_to_view_map.record(
+            tags=tags, measurement_map=measurement_map, timestamp=timestamp)
         self.assertIsNone(record)