Only ping azuremetadataservice if vm retry (#946)

Author: lzchen

contrib/opencensus-ext-azure/CHANGELOG.md

@@ -8,6 +8,8 @@
   ([#935](https://github.com/census-instrumentation/opencensus-python/pull/935))
 - Add links in properties for trace exporter envelopes
   ([#936](https://github.com/census-instrumentation/opencensus-python/pull/936))
+- Fix attach rate metrics for VM to only ping data service on retry
+  ([#946](https://github.com/census-instrumentation/opencensus-python/pull/946))
 
 ## 1.0.4
 Released 2020-06-29

contrib/opencensus-ext-azure/opencensus/ext/azure/metrics_exporter/heartbeat_metrics/heartbeat.py

@@ -36,9 +36,9 @@ class HeartbeatMetric:
 
     def __init__(self):
         self.vm_data = {}
-        self.is_vm = False
+        self.vm_retry = False
         self.properties = OrderedDict()
-        self.update_properties()
+        self._init_properties()
         self.heartbeat = LongGauge(
             HeartbeatMetric.NAME,
             'Heartbeat metric with custom dimensions',
@@ -50,22 +50,23 @@ def __init__(self):
         )
 
     def get_metrics(self):
-        if self.is_vm:
-            # Only need to update if in vm (properties could change)
-            self.properties.clear()
-            self.update_properties()
-            self.heartbeat = LongGauge(
-                HeartbeatMetric.NAME,
-                'Heartbeat metric with custom dimensions',
-                'count',
-                list(self.properties.keys()),
-            )
-            self.heartbeat.get_or_create_time_series(
-                list(self.properties.values())
-            )
+        if self.vm_retry:
+            # Only need to possibly update if vm retry
+            if self._get_azure_compute_metadata() and not self.vm_retry:
+                self._populate_vm_data()
+                # Recreate the metric to initialize key/values
+                self.heartbeat = LongGauge(
+                    HeartbeatMetric.NAME,
+                    'Heartbeat metric with custom dimensions',
+                    'count',
+                    list(self.properties.keys()),
+                )
+                self.heartbeat.get_or_create_time_series(
+                    list(self.properties.values())
+                )
         return [self.heartbeat.get_metric(datetime.datetime.utcnow())]
 
-    def update_properties(self):
+    def _init_properties(self):
         self.properties[LabelKey("sdk", '')] = LabelValue(
             'py{}:oc{}:ext{}'.format(
                 platform.python_version(),
@@ -86,35 +87,40 @@ def update_properties(self):
             # Function apps
             self.properties[LabelKey("azfunction_appId", '')] = \
                 LabelValue(os.environ.get("WEBSITE_HOSTNAME"))
-        elif self.get_azure_compute_metadata():
+        elif self._get_azure_compute_metadata() and not self.vm_retry:
             # VM
-            if self.vm_data:
-                self.properties[LabelKey("azInst_vmId", '')] = \
-                    LabelValue(self.vm_data.get("vmId", ''))
-                self.properties[LabelKey("azInst_subscriptionId", '')] = \
-                    LabelValue(self.vm_data.get("subscriptionId", ''))
-                self.properties[LabelKey("azInst_osType", '')] = \
-                    LabelValue(self.vm_data.get("osType", ''))
+            self._populate_vm_data()
 
-    def get_azure_compute_metadata(self):
+    def _get_azure_compute_metadata(self):
         try:
             request_url = "{0}?{1}&{2}".format(
                 _AIMS_URI, _AIMS_API_VERSION, _AIMS_FORMAT)
             response = requests.get(
                 request_url, headers={"MetaData": "True"}, timeout=5.0)
         except (requests.exceptions.ConnectionError, requests.Timeout):
             # Not in VM
-            self.is_vm = False
+            self.vm_retry = False
             return False
         except requests.exceptions.RequestException:
-            pass  # retry
+            self.vm_retry = True  # retry
+            return False
 
-        self.is_vm = True
         try:
             text = response.text
             self.vm_data = json.loads(text)
         except Exception:  # pylint: disable=broad-except
             # Error in reading response body, retry
-            pass
+            self.vm_retry = True
+            return False
 
+        self.vm_retry = False
         return True
+
+    def _populate_vm_data(self):
+        if self.vm_data:
+            self.properties[LabelKey("azInst_vmId", '')] = \
+                LabelValue(self.vm_data.get("vmId", ''))
+            self.properties[LabelKey("azInst_subscriptionId", '')] = \
+                LabelValue(self.vm_data.get("subscriptionId", ''))
+            self.properties[LabelKey("azInst_osType", '')] = \
+                LabelValue(self.vm_data.get("osType", ''))

contrib/opencensus-ext-azure/tests/test_azure_heartbeat_metrics.py

@@ -186,7 +186,7 @@ def test_heartbeat_metric_init_vm(self):
                 )
             )
             metric = heartbeat_metrics.HeartbeatMetric()
-            self.assertTrue(metric.is_vm)
+            self.assertFalse(metric.vm_retry)
             self.assertEqual(metric.NAME, 'Heartbeat')
             keys = list(metric.properties.keys())
             values = list(metric.properties.values())
@@ -213,7 +213,7 @@ def test_heartbeat_metric_not_vm(self):
             throw(requests.exceptions.ConnectionError)
         ):
             metric = heartbeat_metrics.HeartbeatMetric()
-            self.assertFalse(metric.is_vm)
+            self.assertFalse(metric.vm_retry)
             self.assertEqual(metric.NAME, 'Heartbeat')
             keys = list(metric.properties.keys())
             self.assertEqual(len(keys), 2)
@@ -224,34 +224,34 @@ def test_heartbeat_metric_not_vm_timeout(self):
             throw(requests.Timeout)
         ):
             metric = heartbeat_metrics.HeartbeatMetric()
-            self.assertFalse(metric.is_vm)
+            self.assertFalse(metric.vm_retry)
             self.assertEqual(metric.NAME, 'Heartbeat')
             keys = list(metric.properties.keys())
             self.assertEqual(len(keys), 2)
 
-    def test_heartbeat_metric_vm_error_response(self):
-        with mock.patch('requests.get') as get:
-            get.return_value = MockResponse(
-                200,
-                json.dumps(
-                    {
-                        'vmId': 5,
-                        'subscriptionId': 3,
-                        'osType': 'Linux'
-                    }
-                )
-            )
+    def test_heartbeat_metric_vm_retry(self):
+        with mock.patch(
+            'requests.get',
+            throw(requests.exceptions.RequestException)
+        ):
             metric = heartbeat_metrics.HeartbeatMetric()
-            self.assertTrue(metric.is_vm)
+            self.assertTrue(metric.vm_retry)
             keys = list(metric.properties.keys())
-            self.assertEqual(len(keys), 5)
-            with mock.patch(
-                'requests.get',
-                throw(Exception)
-            ):
-                metric.vm_data.clear()
-                self.assertTrue(metric.is_vm)
-                self.assertEqual(len(metric.vm_data), 0)
-                self.assertTrue(metric.is_vm)
+            self.assertEqual(len(keys), 2)
+            self.assertEqual(len(metric.vm_data), 0)
+            with mock.patch('requests.get') as get:
+                get.return_value = MockResponse(
+                    200,
+                    json.dumps(
+                        {
+                            'vmId': 5,
+                            'subscriptionId': 3,
+                            'osType': 'Linux'
+                        }
+                    )
+                )
+                metric.get_metrics()
+                self.assertFalse(metric.vm_retry)
+                self.assertEqual(len(metric.vm_data), 3)
                 keys = list(metric.properties.keys())
                 self.assertEqual(len(keys), 5)