Repository: Azure/WALinuxAgent
Pull request #1557 (2019-06-14 14:54:25)

Preventing empty extensions to be sent into telemetry
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description
Preventing empty extensions to be sent into telemetry, for the cases such as the following (three empty extension metrics) - 

```
{
  "PerfMetrics": {
    "Microsoft.OSTCExtensions.OSPatchingForLinux-2.0.0.5": {},
    "Microsoft.Azure.Extensions.CustomScript-2.0.6": {},
    "Microsoft.Azure.Diagnostics.LinuxDiagnostic-3.0.121": {},
    "Microsoft.OSTCExtensions.LinuxDiagnostic-2.3.9029": {
      "cpu": {
        "cur_cpu": [...]
      },
      "memory": {
        "cur_mem": [...],
        "max_mem": [...]
      }
    },
    "walinuxagent.service": {
      "cpu": {
        "cur_cpu": [...]
      },
      "memory": {
        "cur_mem": [...],
        "max_mem": [...]
      }
    },
    "Microsoft.OSTCExtensions.VMAccessForLinux-1.4.7.1": {
      "cpu": {
        "cur_cpu": [...]
      },
      "memory": {
        "cur_mem": [...],
        "max_mem": [...]
      }
    }
  },
  "SchemaVersion": 1.0
}
```

<!--
Please add an informative description that covers that changes made by the pull request. 
This checklist is used to make sure that common issues in a pull request are addressed.
This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
-->

---

### PR information
- [x] The title of the PR is clear and informative.
- [x] There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).
- [x] Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
- [x] If applicable, the PR references the bug/issue that it fixes in the description.
- [x] New Unit tests were added for the changes made and Travis.CI is passing.

### Quality of Code and Contribution Guidelines
- [x] I have read the [contribution guidelines](https://github.com/Azure/WALinuxAgent/blob/master/.github/CONTRIBUTING.md).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/azure/walinuxagent/1557)
<!-- Reviewable:end -->


codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/1557?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`develop@3816e53`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/Azure/WALinuxAgent/pull/1557/graphs/tree.svg?width=650&token=ae90E2KYCw&height=150&src=pr)](https://codecov.io/gh/Azure/WALinuxAgent/pull/1557?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##             develop    #1557   +/-   ##
==========================================
  Coverage           ?   63.25%           
==========================================
  Files              ?       78           
  Lines              ?    11037           
  Branches           ?     1553           
==========================================
  Hits               ?     6981           
  Misses             ?     3719           
  Partials           ?      337
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/1557?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/common/cgroupstelemetry.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/1557/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9jZ3JvdXBzdGVsZW1ldHJ5LnB5) | `93.79% <100%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/1557?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/1557?src=pr&el=footer). Last update [3816e53...d5b10cc](https://codecov.io/gh/Azure/WALinuxAgent/pull/1557?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/azurelinuxagent/common/cgroupstelemetry.py b/azurelinuxagent/common/cgroupstelemetry.py
index e680a6e2..b323aec6 100644
--- a/azurelinuxagent/common/cgroupstelemetry.py
+++ b/azurelinuxagent/common/cgroupstelemetry.py
@@ -92,7 +92,11 @@ class CGroupsTelemetry(object):
         collected_metrics = {}
 
         for name, cgroup_metrics in CGroupsTelemetry._cgroup_metrics.items():
-            collected_metrics[name] = CGroupsTelemetry._process_cgroup_metric(cgroup_metrics)
+            perf_metric = CGroupsTelemetry._process_cgroup_metric(cgroup_metrics)
+
+            if perf_metric:
+                collected_metrics[name] = perf_metric
+
             cgroup_metrics.clear()
 
         # Doing cleanup after the metrics have already been collected.


Test patch:
diff --git a/tests/common/test_cgroupstelemetry.py b/tests/common/test_cgroupstelemetry.py
index 6a7de45a..9fa9eabe 100644
--- a/tests/common/test_cgroupstelemetry.py
+++ b/tests/common/test_cgroupstelemetry.py
@@ -313,7 +313,10 @@ class TestCGroupsTelemetry(AgentTestCase):
             self.assertListEqual(generate_metric_list(cpu_percent_values),
                                  collected_metrics[name]["cpu"]["cur_cpu"][0:5])
 
-    def test_cgroup_tracking(self):
+    # mocking get_proc_stat to make it run on Mac and other systems
+    # this test does not need to read the values of the /proc/stat file
+    @patch("azurelinuxagent.common.osutil.default.DefaultOSUtil._get_proc_stat")
+    def test_cgroup_tracking(self, *args):
         num_extensions = 5
         num_controllers = 2
         for i in range(num_extensions):
@@ -330,7 +333,10 @@ class TestCGroupsTelemetry(AgentTestCase):
 
         self.assertEqual(num_extensions * num_controllers, len(CGroupsTelemetry._tracked))
 
-    def test_cgroup_pruning(self):
+    # mocking get_proc_stat to make it run on Mac and other systems
+    # this test does not need to read the values of the /proc/stat file
+    @patch("azurelinuxagent.common.osutil.default.DefaultOSUtil._get_proc_stat")
+    def test_cgroup_pruning(self, *args):
         num_extensions = 5
         num_controllers = 2
         for i in range(num_extensions):
@@ -355,7 +361,10 @@ class TestCGroupsTelemetry(AgentTestCase):
 
         self.assertEqual(0, len(CGroupsTelemetry._tracked))
 
-    def test_cgroup_is_tracked(self):
+    # mocking get_proc_stat to make it run on Mac and other systems
+    # this test does not need to read the values of the /proc/stat file
+    @patch("azurelinuxagent.common.osutil.default.DefaultOSUtil._get_proc_stat")
+    def test_cgroup_is_tracked(self, *args):
         num_extensions = 5
         for i in range(num_extensions):
             dummy_cpu_cgroup = CGroup.create("dummy_cpu_path_{0}".format(i), "cpu", "dummy_extension_{0}".format(i))
@@ -372,7 +381,10 @@ class TestCGroupsTelemetry(AgentTestCase):
         self.assertFalse(CGroupsTelemetry.is_tracked("not_present_cpu_dummy_path"))
         self.assertFalse(CGroupsTelemetry.is_tracked("not_present_memory_dummy_path"))
 
-    def test_process_cgroup_metric_with_incorrect_cgroups_mounted(self):
+    # mocking get_proc_stat to make it run on Mac and other systems
+    # this test does not need to read the values of the /proc/stat file
+    @patch("azurelinuxagent.common.osutil.default.DefaultOSUtil._get_proc_stat")
+    def test_process_cgroup_metric_with_incorrect_cgroups_mounted(self, *args):
         num_extensions = 5
         for i in range(num_extensions):
             dummy_cpu_cgroup = CGroup.create("dummy_cpu_path_{0}".format(i), "cpu", "dummy_extension_{0}".format(i))
@@ -496,6 +508,35 @@ class TestCGroupsTelemetry(AgentTestCase):
                             self.assertEqual(len(max_memory_levels._data), 0)
                             self.assertEqual(len(current_cpu_usage._data), 0)
 
+    @patch("azurelinuxagent.common.cgroup.CpuCgroup.collect")
+    @patch("azurelinuxagent.common.cgroup.MemoryCgroup._get_memory_usage")
+    @patch("azurelinuxagent.common.cgroup.MemoryCgroup._get_memory_max_usage")
+    @patch("azurelinuxagent.common.cgroup.CpuCgroup._get_current_cpu_total")
+    @patch("azurelinuxagent.common.osutil.default.DefaultOSUtil.get_total_cpu_ticks_since_boot")
+    def test_extension_temetry_not_sent_for_empty_perf_metrics(self, *args):
+        num_extensions = 5
+        for i in range(num_extensions):
+            dummy_cpu_cgroup = CGroup.create("dummy_cpu_path_{0}".format(i), "cpu", "dummy_extension_{0}".format(i))
+            CGroupsTelemetry.track_cgroup(dummy_cpu_cgroup)
+
+            dummy_memory_cgroup = CGroup.create("dummy_memory_path_{0}".format(i), "memory",
+                                                "dummy_extension_{0}".format(i))
+            CGroupsTelemetry.track_cgroup(dummy_memory_cgroup)
+
+        with patch("azurelinuxagent.common.cgroupstelemetry.CGroupsTelemetry._process_cgroup_metric") as \
+                patch_process_cgroup_metric:
+            with patch("azurelinuxagent.common.cgroup.CGroup.is_active") as patch_is_active:
+
+                patch_is_active.return_value = False
+                patch_process_cgroup_metric.return_value = {}
+                poll_count = 1
+
+                for data_count in range(poll_count, 10):
+                    CGroupsTelemetry.poll_all_tracked()
+
+                collected_metrics = CGroupsTelemetry.report_all_tracked()
+                self.assertEqual(0, len(collected_metrics))
+
     @skip_if_predicate_false(i_am_root, "This test will only run as root")
     @skip_if_predicate_false(CGroupConfigurator.get_instance().enabled, "Does not run when Cgroups are not enabled")
     def test_telemetry_with_tracked_cgroup(self):