Refactor adding the common fields to single method (#1707)

* Refactor adding the common fields to a single method
* Resolving the import issues.
* Moving the imports into sys.version check.
* Review Comments and Refactoring of Tests

azurelinuxagent/common/event.py

@@ -22,13 +22,14 @@
 import threading
 import time
 import traceback
+from collections import namedtuple
 from datetime import datetime
 
 import azurelinuxagent.common.conf as conf
 import azurelinuxagent.common.logger as logger
 from azurelinuxagent.common.exception import EventError
-from azurelinuxagent.common.future import ustr
-from azurelinuxagent.common.datacontract import get_properties
+from azurelinuxagent.common.future import ustr, OrderedDict
+from azurelinuxagent.common.datacontract import get_properties, DataContractList
 from azurelinuxagent.common.telemetryevent import TelemetryEventParam, TelemetryEvent
 from azurelinuxagent.common.utils import fileutil, textutil
 from azurelinuxagent.common.version import CURRENT_VERSION, CURRENT_AGENT
@@ -281,7 +282,7 @@ def _add_event(self, duration, evt_type, is_internal, is_success, message, name,
         event.parameters.append(TelemetryEventParam('Duration', int(duration)))
         event.parameters.append(TelemetryEventParam('ExtensionType', str(evt_type)))
 
-        self.add_default_parameters_to_event(event)
+        event.parameters = self.add_default_parameters_to_event(event.parameters)
         data = get_properties(event)
         try:
             self.save_event(json.dumps(data))
@@ -308,7 +309,7 @@ def add_log_event(self, level, message):
         event.parameters.append(TelemetryEventParam('Context2', ''))
         event.parameters.append(TelemetryEventParam('Context3', ''))
 
-        self.add_default_parameters_to_event(event)
+        event.parameters = self.add_default_parameters_to_event(event.parameters)
         data = get_properties(event)
         try:
             self.save_event(json.dumps(data))
@@ -336,34 +337,59 @@ def add_metric(self, category, counter, instance, value, log_event=False):
         event.parameters.append(TelemetryEventParam('Instance', str(instance)))
         event.parameters.append(TelemetryEventParam('Value', float(value)))
 
-        self.add_default_parameters_to_event(event)
+        event.parameters = self.add_default_parameters_to_event(event.parameters)
         data = get_properties(event)
         try:
             self.save_event(json.dumps(data))
         except EventError as e:
             logger.error("{0}", e)
 
     @staticmethod
-    def add_default_parameters_to_event(event, set_default_values=False):
-        # We write the GAVersion here rather than add it in azurelinuxagent.ga.monitor.MonitorHandler.add_sysinfo
-        # as there could be a possibility of events being sent with newer version of the agent, rather than the agent
-        # version generating the event.
+    def add_default_parameters_to_event(event_parameters, set_values_for_agent=True):
+        """
+        Default fields are only populated by Agent and not the extension. Agent will fill up any event if they don't
+        have the default params. Example: GAVersion and ContainerId are populated for agent events on the fly,
+        but not for extension events. Add it if it's missing.
+
+        We write the GAVersion here rather than add it in azurelinuxagent.ga.monitor.MonitorHandler.add_sysinfo
+        as there could be a possibility of events being sent with newer version of the agent, rather than the agent
+        version generating the event.
         # Old behavior example: V1 writes the event on the disk and finds an update immediately, and updates. Now the
-        # new monitor thread would pick up the events from the disk and send it with the CURRENT_AGENT, which would have
-        # newer version of the agent. This causes confusion.
-        #
-        # ContainerId can change due to live migration and we want to preserve the container Id of the container writing
-        # the event, rather than sending the event.
-        # OpcodeName: This is used as the actual time of event generation.
+        new monitor thread would pick up the events from the disk and send it with the CURRENT_AGENT, which would have
+        newer version of the agent. This causes confusion.
 
-        default_parameters = [("GAVersion", CURRENT_AGENT), ('ContainerId', get_container_id_from_env()),
-                              ('OpcodeName', datetime.utcnow().__str__()),
-                              ('EventTid', threading.current_thread().ident),
-                              ('EventPid', os.getpid()), ("TaskName", threading.current_thread().getName()),
-                              ("KeywordName", '')]
+        ContainerId can change due to live migration and we want to preserve the container Id of the container writing
+        the event, rather than sending the event.
+        OpcodeName - This is used as the actual time of event generation.
+
+        :param event_parameters: List of parameters of the event.
+        :param set_values_for_agent: Need default values populated or not. Extensions need only GAVersion and
+                                            ContainerId to be populated and others should be
+        :return: Event with default parameters populated (either values for agent or extension)
+        """
+        DefaultParameter = namedtuple('DefaultParameter', ['name', 'value'])
+        default_parameters = [DefaultParameter("GAVersion", CURRENT_AGENT),
+                              DefaultParameter('ContainerId', get_container_id_from_env()),
+                              DefaultParameter('OpcodeName', datetime.utcnow().__str__() if set_values_for_agent else ""),
+                              DefaultParameter('EventTid', threading.current_thread().ident if set_values_for_agent else 0),
+                              DefaultParameter('EventPid', os.getpid() if set_values_for_agent else 0),
+                              DefaultParameter("TaskName", threading.current_thread().getName() if set_values_for_agent else ""),
+                              DefaultParameter("KeywordName", '')]
+
+        # Converting the event_parameters into a dictionary as it helps to easily look up and get values
+        param_names = OrderedDict([(param.name, param.value) for param in event_parameters])
 
         for param in default_parameters:
-            event.parameters.append(TelemetryEventParam(param[0], param[1]))
+            if param.name not in param_names or set_values_for_agent:
+                # If set_values_for_agent, we disregard any values already set for an existing default property and
+                # replaces it with a latest entry.
+                param_names[param.name] = param.value
+
+        parameters = DataContractList(TelemetryEventParam)
+        for name, value in param_names.items():
+            parameters.append(TelemetryEventParam(name, value))
+
+        return parameters
 
 
 __event_logger__ = EventLogger()

azurelinuxagent/common/future.py

@@ -23,6 +23,8 @@
 
     bytebuffer = memoryview
 
+    from collections import OrderedDict
+
 elif sys.version_info[0] == 2:
     import httplib as httpclient
     from urlparse import urlparse
@@ -32,6 +34,10 @@
 
     bytebuffer = buffer
 
+    if sys.version_info[1] >= 7:
+        from collections import OrderedDict  # For Py 2.7+
+    else:
+        from ordereddict import OrderedDict  # Works only on 2.6
 else:
     raise ImportError("Unknown python version: {0}".format(sys.version_info))
 

azurelinuxagent/common/telemetryevent.py

@@ -28,7 +28,6 @@ def __init__(self, name=None, value=None):
     def __eq__(self, other):
         return isinstance(other, TelemetryEventParam) and other.name == self.name and other.value == self.value
 
-
 class TelemetryEvent(DataContract):
     def __init__(self, eventId=None, providerId=None):
         self.eventId = eventId

azurelinuxagent/ga/monitor.py

@@ -26,11 +26,10 @@
 import azurelinuxagent.common.conf as conf
 import azurelinuxagent.common.logger as logger
 import azurelinuxagent.common.utils.networkutil as networkutil
-
 from azurelinuxagent.common.cgroupstelemetry import CGroupsTelemetry
+from azurelinuxagent.common.datacontract import set_properties
 from azurelinuxagent.common.errorstate import ErrorState
-from azurelinuxagent.common.event import add_event, WALAEventOperation, CONTAINER_ID_ENV_VARIABLE, \
-    get_container_id_from_env
+from azurelinuxagent.common.event import EventLogger
 from azurelinuxagent.common.event import add_event, WALAEventOperation, report_metric
 from azurelinuxagent.common.exception import EventError, ProtocolError, OSUtilError, HttpError
 from azurelinuxagent.common.future import ustr
@@ -39,11 +38,10 @@
 from azurelinuxagent.common.protocol.healthservice import HealthService
 from azurelinuxagent.common.protocol.imds import get_imds_client
 from azurelinuxagent.common.telemetryevent import TelemetryEvent, TelemetryEventParam, TelemetryEventList
-from azurelinuxagent.common.datacontract import set_properties
 from azurelinuxagent.common.utils.restutil import IOErrorCounter
 from azurelinuxagent.common.utils.textutil import parse_doc, findall, find, getattrib, hash_strings
 from azurelinuxagent.common.version import DISTRO_NAME, DISTRO_VERSION, \
-    DISTRO_CODE_NAME, AGENT_NAME, CURRENT_AGENT, CURRENT_VERSION, AGENT_EXECUTION_MODE
+    DISTRO_CODE_NAME, AGENT_NAME, CURRENT_VERSION, AGENT_EXECUTION_MODE
 
 
 def parse_event(data_str):
@@ -321,14 +319,6 @@ def add_sysinfo(self, event):
         sysinfo_names = [v.name for v in self.sysinfo]
         final_parameters = []
 
-        # Refer: azurelinuxagent.common.event.EventLogger.add_default_parameters_to_event for agent specific values.
-        #
-        # Default fields are only populated by Agent and not the extension. Agent will fill up any event if they don't
-        # have the default params. Example: GAVersion and ContainerId are populated for agent events on the fly,
-        # but not for extension events. Add it if it's missing.
-        default_values = [("ContainerId", get_container_id_from_env()), ("GAVersion", CURRENT_AGENT),
-                          ("OpcodeName", ""), ("EventTid", 0), ("EventPid", 0), ("TaskName", ""), ("KeywordName", "")]
-
         for param in event.parameters:
             # Discard any sys_info parameters already in the event, since they will be overwritten
             if param.name in sysinfo_names:
@@ -337,10 +327,7 @@ def add_sysinfo(self, event):
 
         # Add sys_info params populated by the agent
         final_parameters.extend(self.sysinfo)
-
-        for default_value in default_values:
-            if default_value[0] not in event:
-                final_parameters.append(TelemetryEventParam(default_value[0], default_value[1]))
+        final_parameters = EventLogger.add_default_parameters_to_event(final_parameters, set_values_for_agent=False)
 
         event.parameters = final_parameters
 

tests/common/test_event.py

@@ -23,11 +23,11 @@
 from datetime import datetime, timedelta
 
 from azurelinuxagent.common import event, logger
-from azurelinuxagent.common.event import add_event, \
-    WALAEventOperation, elapsed_milliseconds, report_metric
+from azurelinuxagent.common.event import add_event, report_metric, WALAEventOperation, elapsed_milliseconds, EventLogger
 from azurelinuxagent.common.exception import EventError
-from azurelinuxagent.common.future import ustr
+from azurelinuxagent.common.future import ustr, OrderedDict
 from azurelinuxagent.common.protocol.wire import GoalState
+from azurelinuxagent.common.telemetryevent import TelemetryEventParam
 from azurelinuxagent.common.utils import fileutil
 from azurelinuxagent.common.utils.extensionprocessutil import read_output
 from azurelinuxagent.common.version import CURRENT_VERSION, CURRENT_AGENT
@@ -437,6 +437,108 @@ def test_elapsed_milliseconds(self):
         utc_start = datetime.utcnow() + timedelta(days=1)
         self.assertEqual(0, elapsed_milliseconds(utc_start))
 
+    def _assert_default_params_get_correctly_added(self, param_list_actual, parameters_expected):
+        default_parameters_expected_names = set(parameters_expected.keys())
+
+        # Converting list of TelemetryEventParam into a dictionary, for easier look up of values.
+        param_list_dict = OrderedDict([(param.name, param.value) for param in param_list_actual])
+
+        counter = 0
+        for p in default_parameters_expected_names:
+            self.assertIn(p, param_list_dict)
+            self.assertEqual(param_list_dict[p], parameters_expected[p])
+            counter += 1
+
+        self.assertEqual(len(default_parameters_expected_names), counter)
+
+    @patch("azurelinuxagent.common.event.get_container_id_from_env", return_value="TEST_CONTAINER_ID")
+    def test_add_default_parameters_to_extension_event(self, *args):
+        default_parameters_expected = {"GAVersion": CURRENT_AGENT, 'ContainerId': "TEST_CONTAINER_ID", 'OpcodeName': "",
+                                       'EventTid': 0, 'EventPid': 0, "TaskName": "", "KeywordName": ""}
+
+        # When no values are populated in the TelemetryEventParamList.
+        extension_param_list_empty = EventLogger.add_default_parameters_to_event([], set_values_for_agent=False)
+        self._assert_default_params_get_correctly_added(extension_param_list_empty, default_parameters_expected)
+
+        # When some values are already populated in the TelemetryEventParamList.
+        extension_param_list_populated = [TelemetryEventParam('Name', "DummyExtension"),
+                                          TelemetryEventParam('Version', CURRENT_VERSION),
+                                          TelemetryEventParam('Operation', "DummyOperation"),
+                                          TelemetryEventParam('OperationSuccess', True),
+                                          TelemetryEventParam('Message', "TestMessage"),
+                                          TelemetryEventParam('Duration', 10), TelemetryEventParam('ExtensionType', ''),
+                                          TelemetryEventParam('OpcodeName', '')]
+        extension_param_list_with_defaults = EventLogger.add_default_parameters_to_event(extension_param_list_populated,
+                                                                                         set_values_for_agent=False)
+        self._assert_default_params_get_correctly_added(extension_param_list_with_defaults, default_parameters_expected)
+
+        parameters_expected = {"GAVersion": CURRENT_AGENT, 'ContainerId': "TEST_CONTAINER_ID", 'OpcodeName': "",
+                               'EventTid': 100, 'EventPid': 10, "TaskName": "", "KeywordName": ""}
+
+        # When some values are already populated in the TelemetryEventParamList.
+        extension_param_list_populated = [TelemetryEventParam('Name', "DummyExtension"),
+                                          TelemetryEventParam('Version', CURRENT_VERSION),
+                                          TelemetryEventParam('Operation', "DummyOperation"),
+                                          TelemetryEventParam('OperationSuccess', True),
+                                          TelemetryEventParam('Message', "TestMessage"),
+                                          TelemetryEventParam('Duration', 10),
+                                          TelemetryEventParam('ExtensionType', ''),
+                                          TelemetryEventParam('OpcodeName', ''),
+                                          TelemetryEventParam('EventTid', 100),
+                                          TelemetryEventParam('EventPid', 10)]
+        extension_param_list_with_defaults = EventLogger.add_default_parameters_to_event(extension_param_list_populated,
+                                                                                         set_values_for_agent=False)
+        self._assert_default_params_get_correctly_added(extension_param_list_with_defaults,
+                                                        parameters_expected)
+
+    @patch("threading.Thread.getName", return_value="HelloWorldTask")
+    @patch('os.getpid', return_value=42)
+    @patch("azurelinuxagent.common.event.get_container_id_from_env", return_value="TEST_CONTAINER_ID")
+    @patch("azurelinuxagent.common.event.datetime")
+    def test_add_default_parameters_to_agent_event(self, patch_datetime, *args):
+        patch_datetime.utcnow = Mock(return_value=datetime.strptime("2019-01-01 01:30:00",
+                                                                    '%Y-%m-%d %H:%M:%S'))
+        default_parameters_expected = {"GAVersion": CURRENT_AGENT,
+                                       'ContainerId': "TEST_CONTAINER_ID",
+                                       'OpcodeName': "2019-01-01 01:30:00",
+                                       'EventTid': threading.current_thread().ident,
+                                       'EventPid': 42,
+                                       "TaskName": "HelloWorldTask",
+                                       "KeywordName": ""}
+        agent_param_list_empty = EventLogger.add_default_parameters_to_event([], set_values_for_agent=True)
+        self._assert_default_params_get_correctly_added(agent_param_list_empty, default_parameters_expected)
+
+        # When some values are already populated in the TelemetryEventParamList.
+        agent_param_list_populated = [TelemetryEventParam('Name', "DummyExtension"),
+                                      TelemetryEventParam('Version', CURRENT_VERSION),
+                                      TelemetryEventParam('Operation', "DummyOperation"),
+                                      TelemetryEventParam('OperationSuccess', True),
+                                      TelemetryEventParam('Message', "TestMessage"),
+                                      TelemetryEventParam('Duration', 10), TelemetryEventParam('ExtensionType', ''),
+                                      TelemetryEventParam('OpcodeName', '')]
+        agent_param_list_after_defaults_added = EventLogger.add_default_parameters_to_event(agent_param_list_populated,
+                                                                                            set_values_for_agent=True)
+        self._assert_default_params_get_correctly_added(agent_param_list_after_defaults_added,
+                                                        default_parameters_expected)
+
+        # When some values are already populated in the TelemetryEventParamList, along with some
+        # default values already populated and it should be replaced, when set_values_for_agent=True
+        agent_param_list_populated = [TelemetryEventParam('Name', "DummyExtension"),
+                                      TelemetryEventParam('Version', CURRENT_VERSION),
+                                      TelemetryEventParam('Operation', "DummyOperation"),
+                                      TelemetryEventParam('OperationSuccess', True),
+                                      TelemetryEventParam('Message', "TestMessage"),
+                                      TelemetryEventParam('Duration', 10), TelemetryEventParam('ExtensionType', ''),
+                                      TelemetryEventParam('OpcodeName', 'timestamp'),
+                                      TelemetryEventParam('ContainerId', 'SOME-CONTAINER'),
+                                      TelemetryEventParam('EventTid', 10101010), TelemetryEventParam('EventPid', 110),
+                                      TelemetryEventParam('TaskName', 'Test-TaskName')]
+
+        agent_param_list_after_defaults_added = EventLogger.add_default_parameters_to_event(agent_param_list_populated,
+                                                                                            set_values_for_agent=True)
+        self._assert_default_params_get_correctly_added(agent_param_list_after_defaults_added,
+                                                        default_parameters_expected)
+
 
 class TestMetrics(AgentTestCase):
     @patch('azurelinuxagent.common.event.EventLogger.save_event')