Repository: Azure/WALinuxAgent
Pull request #2162 (2021-02-07 00:42:09)

Refactoring the monitor and env threads to use PeriodicOperations

codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162?src=pr&el=h1) Report
> Merging [#2162](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162?src=pr&el=desc) (276c9d6) into [develop](https://codecov.io/gh/Azure/WALinuxAgent/commit/e101f2082bd5621a01a448904eea20131b37f85d?el=desc) (e101f20) will **decrease** coverage by `0.14%`.
> The diff coverage is `69.33%`.

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

```diff
@@             Coverage Diff             @@
##           develop    #2162      +/-   ##
===========================================
- Coverage    70.65%   70.50%   -0.15%     
===========================================
  Files           96       96              
  Lines        13351    13367      +16     
  Branches      1882     1881       -1     
===========================================
- Hits          9433     9425       -8     
- Misses        3504     3533      +29     
+ Partials       414      409       -5     
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/ga/env.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2dhL2Vudi5weQ==) | `63.57% <59.45%> (-3.38%)` | :arrow_down: |
| [azurelinuxagent/ga/periodic\_operation.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2dhL3BlcmlvZGljX29wZXJhdGlvbi5weQ==) | `97.22% <83.33%> (+0.16%)` | :arrow_up: |
| [azurelinuxagent/ga/collect\_logs.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2dhL2NvbGxlY3RfbG9ncy5weQ==) | `87.60% <100.00%> (-0.21%)` | :arrow_down: |
| [azurelinuxagent/ga/collect\_telemetry\_events.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2dhL2NvbGxlY3RfdGVsZW1ldHJ5X2V2ZW50cy5weQ==) | `91.37% <100.00%> (ø)` | |
| [azurelinuxagent/ga/monitor.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2dhL21vbml0b3IucHk=) | `75.29% <100.00%> (-0.43%)` | :arrow_down: |
| [azurelinuxagent/common/conf.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9jb25mLnB5) | `75.98% <0.00%> (-2.46%)` | :arrow_down: |
| [azurelinuxagent/common/osutil/default.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9vc3V0aWwvZGVmYXVsdC5weQ==) | `59.67% <0.00%> (-0.87%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162?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/2162?src=pr&el=footer). Last update [e101f20...dd77659](https://codecov.io/gh/Azure/WALinuxAgent/pull/2162?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/azurelinuxagent/ga/collect_logs.py b/azurelinuxagent/ga/collect_logs.py
index 4cdc7acf..f52aae17 100644
--- a/azurelinuxagent/ga/collect_logs.py
+++ b/azurelinuxagent/ga/collect_logs.py
@@ -34,7 +34,6 @@ from azurelinuxagent.common.protocol.util import get_protocol_util
 from azurelinuxagent.common.utils import shellutil
 from azurelinuxagent.common.utils.shellutil import CommandError
 from azurelinuxagent.common.version import PY_VERSION_MAJOR, PY_VERSION_MINOR, AGENT_NAME, CURRENT_VERSION
-from azurelinuxagent.ga.periodic_operation import PeriodicOperation
 
 
 def get_collect_logs_handler():
@@ -85,10 +84,7 @@ class CollectLogsHandler(ThreadHandlerInterface):
         self.event_thread = None
         self.should_run = True
         self.last_state = None
-
-        self._periodic_operations = [
-            PeriodicOperation("collect_and_send_logs", self.collect_and_send_logs, conf.get_collect_logs_period())
-        ]
+        self.period = conf.get_collect_logs_period()
 
     def run(self):
         self.start()
@@ -127,13 +123,12 @@ class CollectLogsHandler(ThreadHandlerInterface):
 
             while not self.stopped():
                 try:
-                    for op in self._periodic_operations:
-                        op.run()
+                    self.collect_and_send_logs()
                 except Exception as e:
                     logger.error("An error occurred in the log collection thread main loop; "
                                  "will skip the current iteration.\n{0}", ustr(e))
                 finally:
-                    PeriodicOperation.sleep_until_next_operation(self._periodic_operations)
+                    time.sleep(self.period)
         except Exception as e:
             logger.error("An error occurred in the log collection thread; will exit the thread.\n{0}", ustr(e))
 
diff --git a/azurelinuxagent/ga/collect_telemetry_events.py b/azurelinuxagent/ga/collect_telemetry_events.py
index 94d4d68e..b33def2b 100644
--- a/azurelinuxagent/ga/collect_telemetry_events.py
+++ b/azurelinuxagent/ga/collect_telemetry_events.py
@@ -68,7 +68,7 @@ class ExtensionEventSchema(object):
     OperationId = "OperationId"
 
 
-class _ProcessExtensionEventsPeriodicOperation(PeriodicOperation):
+class _ProcessExtensionEvents(PeriodicOperation):
     """
     Periodic operation for collecting extension telemetry events and enqueueing them for the SendTelemetryHandler thread.
     """
@@ -86,14 +86,10 @@ class _ProcessExtensionEventsPeriodicOperation(PeriodicOperation):
                                         not callable(getattr(ExtensionEventSchema, attr)) and not attr.startswith("__")]
 
     def __init__(self, send_telemetry_events_handler):
-        super(_ProcessExtensionEventsPeriodicOperation, self).__init__(
-            name="collect_and_enqueue_extension_events",
-            operation=self._collect_and_enqueue_extension_events,
-            period=_ProcessExtensionEventsPeriodicOperation._EXTENSION_EVENT_COLLECTION_PERIOD)
-
+        super(_ProcessExtensionEvents, self).__init__(_ProcessExtensionEvents._EXTENSION_EVENT_COLLECTION_PERIOD)
         self._send_telemetry_events_handler = send_telemetry_events_handler
 
-    def _collect_and_enqueue_extension_events(self):
+    def _operation(self):
 
         if self._send_telemetry_events_handler.stopped():
             logger.warn("{0} service is not running, skipping current iteration".format(
@@ -381,7 +377,7 @@ class _ProcessExtensionEventsPeriodicOperation(PeriodicOperation):
             event.parameters.append(TelemetryEventParam(param_name, replace_or_add_params[param_name]))
 
 
-class _CollectAndEnqueueEventsPeriodicOperation(PeriodicOperation):
+class _CollectAndEnqueueEvents(PeriodicOperation):
     """
     Periodic operation to collect telemetry events located in the events folder and enqueue them for the
     SendTelemetryHandler thread.
@@ -390,13 +386,10 @@ class _CollectAndEnqueueEventsPeriodicOperation(PeriodicOperation):
     _EVENT_COLLECTION_PERIOD = datetime.timedelta(minutes=1)
 
     def __init__(self, send_telemetry_events_handler):
-        super(_CollectAndEnqueueEventsPeriodicOperation, self).__init__(
-            name="collect_and_enqueue_events",
-            operation=self._collect_and_enqueue_events,
-            period=_CollectAndEnqueueEventsPeriodicOperation._EVENT_COLLECTION_PERIOD)
+        super(_CollectAndEnqueueEvents, self).__init__(_CollectAndEnqueueEvents._EVENT_COLLECTION_PERIOD)
         self._send_telemetry_events_handler = send_telemetry_events_handler
 
-    def _collect_and_enqueue_events(self):
+    def _operation(self):
         """
         Periodically send any events located in the events folder
         """
@@ -445,12 +438,12 @@ class _CollectAndEnqueueEventsPeriodicOperation(PeriodicOperation):
                         event_file_creation_time = datetime.datetime.fromtimestamp(event_file_creation_time_epoch)
 
                         if event.is_extension_event():
-                            _CollectAndEnqueueEventsPeriodicOperation._trim_legacy_extension_event_parameters(event)
+                            _CollectAndEnqueueEvents._trim_legacy_extension_event_parameters(event)
                             CollectTelemetryEventsHandler.add_common_params_to_telemetry_event(event,
                                                                                                event_file_creation_time)
                         else:
-                            _CollectAndEnqueueEventsPeriodicOperation._update_legacy_agent_event(event,
-                                                                                                 event_file_creation_time)
+                            _CollectAndEnqueueEvents._update_legacy_agent_event(event,
+                                                                                event_file_creation_time)
 
                     self._send_telemetry_events_handler.enqueue_event(event)
                 finally:
@@ -558,13 +551,13 @@ class CollectTelemetryEventsHandler(ThreadHandlerInterface):
 
     def daemon(self):
         periodic_operations = [
-            _CollectAndEnqueueEventsPeriodicOperation(self._send_telemetry_events_handler)
+            _CollectAndEnqueueEvents(self._send_telemetry_events_handler)
         ]
 
         logger.info("Extension Telemetry pipeline enabled: {0}".format(
             is_extension_telemetry_pipeline_enabled()))
         if is_extension_telemetry_pipeline_enabled():
-            periodic_operations.append(_ProcessExtensionEventsPeriodicOperation(self._send_telemetry_events_handler))
+            periodic_operations.append(_ProcessExtensionEvents(self._send_telemetry_events_handler))
 
         logger.info("Successfully started the {0} thread".format(self.get_thread_name()))
         while not self.stopped():
diff --git a/azurelinuxagent/ga/env.py b/azurelinuxagent/ga/env.py
index 941323d9..1a6f6226 100644
--- a/azurelinuxagent/ga/env.py
+++ b/azurelinuxagent/ga/env.py
@@ -48,99 +48,88 @@ def get_env_handler():
     return EnvHandler()
 
 
-class EnvHandler(ThreadHandlerInterface):
-    """
-    Monitor changes to dhcp and hostname.
-    If dhcp client process re-start has occurred, reset routes, dhcp with fabric.
+class RemovePersistentNetworkRules(PeriodicOperation):
+    def __init__(self, osutil):
+        super(RemovePersistentNetworkRules, self).__init__(conf.get_remove_persistent_net_rules_period())
+        self.osutil = osutil
 
-    Monitor scsi disk.
-    If new scsi disk found, set timeout
-    """
-
-    _THREAD_NAME = "EnvHandler"
+    def _operation(self):
+        self.osutil.remove_rules_files()
 
-    @staticmethod
-    def get_thread_name():
-        return EnvHandler._THREAD_NAME
 
-    def __init__(self):
-        self.osutil = get_osutil()
+class MonitorDhcpClientRestart(PeriodicOperation):
+    def __init__(self, osutil):
+        super(MonitorDhcpClientRestart, self).__init__(conf.get_monitor_dhcp_client_restart_period())
+        self.osutil = osutil
         self.dhcp_handler = get_dhcp_handler()
-        self.protocol_util = None
-        self._protocol = None
-        self.stopped = True
-        self.hostname = None
-        self.dhcp_id_list = []
-        self.server_thread = None
+        self.dhcp_handler.conf_routes()
         self.dhcp_warning_enabled = True
-        self.archiver = StateArchiver(conf.get_lib_dir())
-        self._reset_firewall_rules = False
-
-        self._periodic_operations = [
-            PeriodicOperation("_remove_persistent_net_rules", self._remove_persistent_net_rules_period, conf.get_remove_persistent_net_rules_period()),
-            PeriodicOperation("_monitor_dhcp_client_restart", self._monitor_dhcp_client_restart, conf.get_monitor_dhcp_client_restart_period()),
-            PeriodicOperation("_cleanup_goal_state_history", self._cleanup_goal_state_history, conf.get_goal_state_history_cleanup_period())
-        ]
-        if conf.enable_firewall():
-            self._periodic_operations.append(PeriodicOperation("_enable_firewall", self._enable_firewall, conf.get_enable_firewall_period()))
-        if conf.get_root_device_scsi_timeout() is not None:
-            self._periodic_operations.append(PeriodicOperation("_set_root_device_scsi_timeout", self._set_root_device_scsi_timeout, conf.get_root_device_scsi_timeout_period()))
-        if conf.get_monitor_hostname():
-            self._periodic_operations.append(PeriodicOperation("_monitor_hostname", self._monitor_hostname_changes, conf.get_monitor_hostname_period()))
+        self.dhcp_id_list = []
 
-    def run(self):
-        if not self.stopped:
-            logger.info("Stop existing env monitor service.")
-            self.stop()
+    def _operation(self):
+        if len(self.dhcp_id_list) == 0:
+            self.dhcp_id_list = self._get_dhcp_client_pid()
+            return
 
-        self.stopped = False
-        logger.info("Start env monitor service.")
-        self.dhcp_handler.conf_routes()
-        self.hostname = self.osutil.get_hostname_record()
-        self.dhcp_id_list = self.get_dhcp_client_pid()
-        self.start()
+        if all(self.osutil.check_pid_alive(pid) for pid in self.dhcp_id_list):
+            return
 
-    def is_alive(self):
-        return self.server_thread.is_alive()
+        new_pid = self._get_dhcp_client_pid()
+        if len(new_pid) != 0 and new_pid != self.dhcp_id_list:
+            logger.info("EnvMonitor: Detected dhcp client restart. Restoring routing table.")
+            self.dhcp_handler.conf_routes()
+            self.dhcp_id_list = new_pid
 
-    def start(self):
-        self.server_thread = threading.Thread(target=self.monitor)
-        self.server_thread.setDaemon(True)
-        self.server_thread.setName(self.get_thread_name())
-        self.server_thread.start()
+    def _get_dhcp_client_pid(self):
+        pid = []
 
-    def monitor(self):
         try:
-            # The initialization of ProtocolUtil for the Environment thread should be done within the thread itself rather
-            # than initializing it in the ExtHandler thread. This is done to avoid any concurrency issues as each
-            # thread would now have its own ProtocolUtil object as per the SingletonPerThread model.
-            self.protocol_util = get_protocol_util()
-            self._protocol = self.protocol_util.get_protocol()
-            while not self.stopped:
-                try:
-                    for op in self._periodic_operations:
-                        op.run()
-                except Exception as e:
-                    logger.error("An error occurred in the environment thread main loop; will skip the current iteration.\n{0}", ustr(e))
-                finally:
-                    PeriodicOperation.sleep_until_next_operation(self._periodic_operations)
-        except Exception as e:
-            logger.error("An error occurred in the environment thread; will exit the thread.\n{0}", ustr(e))
+            # return a sorted list since handle_dhclient_restart needs to compare the previous value with
+            # the new value and the comparison should not be affected by the order of the items in the list
+            pid = sorted(self.osutil.get_dhcp_pid())
 
-    def _remove_persistent_net_rules_period(self):
-        self.osutil.remove_rules_files()
+            if len(pid) == 0 and self.dhcp_warning_enabled:
+                logger.warn("Dhcp client is not running.")
+        except Exception as exception:
+            if self.dhcp_warning_enabled:
+                logger.error("Failed to get the PID of the DHCP client: {0}", ustr(exception))
+
+        self.dhcp_warning_enabled = len(pid) != 0
 
-    def _enable_firewall(self):
+        return pid
+
+
+class CleanupGoalStateHistory(PeriodicOperation):
+    def __init__(self):
+        super(CleanupGoalStateHistory, self).__init__(conf.get_goal_state_history_cleanup_period())
+        self.archiver = StateArchiver(conf.get_lib_dir())
+
+    def _operation(self):
+        """
+        Purge history and create a .zip of the history that has been preserved.
+        """
+        self.archiver.purge()
+        self.archiver.archive()
+
+
+class EnableFirewall(PeriodicOperation):
+    def __init__(self, osutil, protocol):
+        super(EnableFirewall, self).__init__(conf.get_enable_firewall_period())
+        self._osutil = osutil
+        self._protocol = protocol
+        self._reset_firewall_rules = False
+
+    def _operation(self):
         # If the rules ever change we must reset all rules and start over again.
         #
         # There was a rule change at 2.2.26, which started dropping non-root traffic
         # to WireServer.  The previous rules allowed traffic.  Having both rules in
         # place negated the fix in 2.2.26.
         if not self._reset_firewall_rules:
-            self.osutil.remove_firewall(dst_ip=self._protocol.get_endpoint(), uid=os.getuid())
+            self._osutil.remove_firewall(dst_ip=self._protocol.get_endpoint(), uid=os.getuid())
             self._reset_firewall_rules = True
 
-        success = self.osutil.enable_firewall(dst_ip=self._protocol.get_endpoint(), uid=os.getuid())
+        success = self._osutil.enable_firewall(dst_ip=self._protocol.get_endpoint(), uid=os.getuid())
 
         add_periodic(
             logger.EVERY_HOUR,
@@ -150,65 +139,107 @@ class EnvHandler(ThreadHandlerInterface):
             is_success=success,
             log_event=False)
 
-    def _set_root_device_scsi_timeout(self):
-        self.osutil.set_scsi_disks_timeout(conf.get_root_device_scsi_timeout())
 
-    def _monitor_hostname_changes(self):
+class SetRootDeviceScsiTimeout(PeriodicOperation):
+    def __init__(self, osutil):
+        super(SetRootDeviceScsiTimeout, self).__init__(conf.get_root_device_scsi_timeout_period())
+        self._osutil = osutil
+
+    def _operation(self):
+        self._osutil.set_scsi_disks_timeout(conf.get_root_device_scsi_timeout())
+
+
+class MonitorHostNameChanges(PeriodicOperation):
+    def __init__(self, osutil):
+        super(MonitorHostNameChanges, self).__init__(conf.get_monitor_hostname_period())
+        self._osutil = osutil
+        self._hostname = self._osutil.get_hostname_record()
+
+    def _operation(self):
         curr_hostname = socket.gethostname()
-        if curr_hostname != self.hostname:
+        if curr_hostname != self._hostname:
             logger.info("EnvMonitor: Detected hostname change: {0} -> {1}",
-                        self.hostname,
+                        self._hostname,
                         curr_hostname)
-            self.osutil.set_hostname(curr_hostname)
-            self.osutil.publish_hostname(curr_hostname)
-            self.hostname = curr_hostname
+            self._osutil.set_hostname(curr_hostname)
+            self._osutil.publish_hostname(curr_hostname)
+            self._hostname = curr_hostname
 
-    def get_dhcp_client_pid(self):
-        pid = []
 
-        try:
-            # return a sorted list since handle_dhclient_restart needs to compare the previous value with
-            # the new value and the comparison should not be affected by the order of the items in the list
-            pid = sorted(self.osutil.get_dhcp_pid())
+class EnvHandler(ThreadHandlerInterface):
+    """
+    Monitor changes to dhcp and hostname.
+    If dhcp client process re-start has occurred, reset routes, dhcp with fabric.
 
-            if len(pid) == 0 and self.dhcp_warning_enabled:
-                logger.warn("Dhcp client is not running.")
-        except Exception as exception:
-            if self.dhcp_warning_enabled:
-                logger.error("Failed to get the PID of the DHCP client: {0}", ustr(exception))
+    Monitor scsi disk.
+    If new scsi disk found, set timeout
+    """
 
-        self.dhcp_warning_enabled = len(pid) != 0
+    _THREAD_NAME = "EnvHandler"
 
-        return pid
+    @staticmethod
+    def get_thread_name():
+        return EnvHandler._THREAD_NAME
 
-    def _monitor_dhcp_client_restart(self):
-        self.handle_dhclient_restart()
+    def __init__(self):
+        self.stopped = True
+        self.hostname = None
+        self.env_thread = None
 
-    def handle_dhclient_restart(self):
-        if len(self.dhcp_id_list) == 0:
-            self.dhcp_id_list = self.get_dhcp_client_pid()
-            return
+    def run(self):
+        if not self.stopped:
+            logger.info("Stop existing env monitor service.")
+            self.stop()
 
-        if all(self.osutil.check_pid_alive(pid) for pid in self.dhcp_id_list):
-            return
+        self.stopped = False
+        logger.info("Starting env monitor service.")
+        self.start()
 
-        new_pid = self.get_dhcp_client_pid()
-        if len(new_pid) != 0 and new_pid != self.dhcp_id_list:
-            logger.info("EnvMonitor: Detected dhcp client restart. Restoring routing table.")
-            self.dhcp_handler.conf_routes()
-            self.dhcp_id_list = new_pid
+    def is_alive(self):
+        return self.env_thread.is_alive()
 
-    def _cleanup_goal_state_history(self):
-        """
-        Purge history and create a .zip of the history that has been preserved.
-        """
-        self.archiver.purge()
-        self.archiver.archive()
+    def start(self):
+        self.env_thread = threading.Thread(target=self.daemon)
+        self.env_thread.setDaemon(True)
+        self.env_thread.setName(self.get_thread_name())
+        self.env_thread.start()
+
+    def daemon(self):
+        try:
+            # The initialization of the protocol needs to be done within the environment thread itself rather
+            # than initializing it in the ExtHandler thread. This is done to avoid any concurrency issues as each
+            # thread would now have its own ProtocolUtil object as per the SingletonPerThread model.
+            protocol_util = get_protocol_util()
+            protocol = protocol_util.get_protocol()
+            osutil = get_osutil()
+
+            periodic_operations = [
+                RemovePersistentNetworkRules(osutil),
+                MonitorDhcpClientRestart(osutil),
+                CleanupGoalStateHistory()
+            ]
+
+            if conf.enable_firewall():
+                periodic_operations.append(EnableFirewall(osutil, protocol))
+            if conf.get_root_device_scsi_timeout() is not None:
+                periodic_operations.append(SetRootDeviceScsiTimeout(osutil))
+            if conf.get_monitor_hostname():
+                periodic_operations.append(MonitorHostNameChanges(osutil))
+            while not self.stopped:
+                try:
+                    for op in periodic_operations:
+                        op.run()
+                except Exception as e:
+                    logger.error("An error occurred in the environment thread main loop; will skip the current iteration.\n{0}", ustr(e))
+                finally:
+                    PeriodicOperation.sleep_until_next_operation(periodic_operations)
+        except Exception as e:
+            logger.error("An error occurred in the environment thread; will exit the thread.\n{0}", ustr(e))
 
     def stop(self):
         """
         Stop server communication and join the thread to main thread.
         """
         self.stopped = True
-        if self.server_thread is not None:
-            self.server_thread.join()
+        if self.env_thread is not None:
+            self.env_thread.join()
diff --git a/azurelinuxagent/ga/monitor.py b/azurelinuxagent/ga/monitor.py
index 2c90314b..aca4a1ed 100644
--- a/azurelinuxagent/ga/monitor.py
+++ b/azurelinuxagent/ga/monitor.py
@@ -42,51 +42,39 @@ def get_monitor_handler():
     return MonitorHandler()
 
 
-class PollResourceUsageOperation(PeriodicOperation):
+class PollResourceUsage(PeriodicOperation):
     """
     Periodic operation to poll the tracked cgroups for resource usage data.
 
     It also checks whether there are processes in the agent's cgroup that should not be there.
     """
     def __init__(self):
-        super(PollResourceUsageOperation, self).__init__(
-            name="poll resource usage",
-            operation=self._operation_impl,
-            period=datetime.timedelta(minutes=5))
+        super(PollResourceUsage, self).__init__(datetime.timedelta(minutes=5))
 
-    @staticmethod
-    def _operation_impl():
+    def _operation(self):
         CGroupConfigurator.get_instance().check_processes_in_agent_cgroup()
 
         for metric in CGroupsTelemetry.poll_all_tracked():
             report_metric(metric.category, metric.counter, metric.instance, metric.value)
 
 
-class ResetPeriodicLogMessagesOperation(PeriodicOperation):
+class ResetPeriodicLogMessages(PeriodicOperation):
     """
     Periodic operation to clean up the hash-tables maintained by the loggers. For reference, please check
     azurelinuxagent.common.logger.Logger and azurelinuxagent.common.event.EventLogger classes
     """
     def __init__(self):
-        super(ResetPeriodicLogMessagesOperation, self).__init__(
-            name="reset periodic log messages",
-            operation=ResetPeriodicLogMessagesOperation._operation_impl,
-            period=datetime.timedelta(hours=12))
+        super(ResetPeriodicLogMessages, self).__init__(datetime.timedelta(hours=12))
 
-    @staticmethod
-    def _operation_impl():
+    def _operation(self):
         logger.reset_periodic()
 
 
-class ReportNetworkErrorsOperation(PeriodicOperation):
+class ReportNetworkErrors(PeriodicOperation):
     def __init__(self):
-        super(ReportNetworkErrorsOperation, self).__init__(
-            name="report network errors",
-            operation=ReportNetworkErrorsOperation._operation_impl,
-            period=datetime.timedelta(minutes=30))
+        super(ReportNetworkErrors, self).__init__(datetime.timedelta(minutes=30))
 
-    @staticmethod
-    def _operation_impl():
+    def _operation(self):
         io_errors = IOErrorCounter.get_and_reset()
         hostplugin_errors = io_errors.get("hostplugin")
         protocol_errors = io_errors.get("protocol")
@@ -97,15 +85,12 @@ class ReportNetworkErrorsOperation(PeriodicOperation):
             add_event(op=WALAEventOperation.HttpErrors, message=msg)
 
 
-class ReportNetworkConfigurationChangesOperation(PeriodicOperation):
+class ReportNetworkConfigurationChanges(PeriodicOperation):
     """
     Periodic operation to check and log changes in network configuration.
     """
     def __init__(self):
-        super(ReportNetworkConfigurationChangesOperation, self).__init__(
-            name="report network configuration changes",
-            operation=self._operation_impl,
-            period=datetime.timedelta(minutes=1))
+        super(ReportNetworkConfigurationChanges, self).__init__(datetime.timedelta(minutes=1))
         self.osutil = get_osutil()
         self.last_route_table_hash = b''
         self.last_nic_state = {}
@@ -128,7 +113,7 @@ class ReportNetworkConfigurationChangesOperation(PeriodicOperation):
         except Exception as exception:
             logger.warn("Error fetching the network configuration: {0}", ustr(exception))
 
-    def _operation_impl(self):
+    def _operation(self):
         raw_route_list = self.osutil.read_route_table()
         digest = hash_strings(raw_route_list)
         if digest != self.last_route_table_hash:
@@ -143,24 +128,21 @@ class ReportNetworkConfigurationChangesOperation(PeriodicOperation):
             self.last_nic_state = nic_state
 
 
-class SendHostPluginHeartbeatOperation(PeriodicOperation):
+class SendHostPluginHeartbeat(PeriodicOperation):
     """
     Periodic operation for reporting the HostGAPlugin's health. The signal is 'Healthy' when we have been able to communicate with
     plugin at least once in the last _HOST_PLUGIN_HEALTH_PERIOD.
     """
     def __init__(self, protocol, health_service):
-        super(SendHostPluginHeartbeatOperation, self).__init__(
-            name="send_host_plugin_heartbeat",
-            operation=self._operation_impl,
-            period=SendHostPluginHeartbeatOperation._HOST_PLUGIN_HEARTBEAT_PERIOD)
+        super(SendHostPluginHeartbeat, self).__init__(SendHostPluginHeartbeat._HOST_PLUGIN_HEARTBEAT_PERIOD)
         self.protocol = protocol
         self.health_service = health_service
-        self.host_plugin_error_state = ErrorState(min_timedelta=SendHostPluginHeartbeatOperation._HOST_PLUGIN_HEALTH_PERIOD)
+        self.host_plugin_error_state = ErrorState(min_timedelta=SendHostPluginHeartbeat._HOST_PLUGIN_HEALTH_PERIOD)
 
     _HOST_PLUGIN_HEARTBEAT_PERIOD = datetime.timedelta(minutes=1)
     _HOST_PLUGIN_HEALTH_PERIOD = datetime.timedelta(minutes=5)
 
-    def _operation_impl(self):
+    def _operation(self):
         try:
             host_plugin = self.protocol.client.get_host_plugin()
             host_plugin.ensure_initialized()
@@ -198,24 +180,21 @@ class SendHostPluginHeartbeatOperation(PeriodicOperation):
                 log_event=False)
 
 
-class SendImdsHeartbeatOperation(PeriodicOperation):
+class SendImdsHeartbeat(PeriodicOperation):
     """
     Periodic operation to report the IDMS's health. The signal is 'Healthy' when we have successfully called and validated
     a response in the last _IMDS_HEALTH_PERIOD.
     """
     def __init__(self, protocol_util, health_service):
-        super(SendImdsHeartbeatOperation, self).__init__(
-            name="send_imds_heartbeat",
-            operation=self._operation_impl,
-            period=SendImdsHeartbeatOperation._IMDS_HEARTBEAT_PERIOD)
+        super(SendImdsHeartbeat, self).__init__(SendImdsHeartbeat._IMDS_HEARTBEAT_PERIOD)
         self.health_service = health_service
         self.imds_client = get_imds_client(protocol_util.get_wireserver_endpoint())
-        self.imds_error_state = ErrorState(min_timedelta=SendImdsHeartbeatOperation._IMDS_HEALTH_PERIOD)
+        self.imds_error_state = ErrorState(min_timedelta=SendImdsHeartbeat._IMDS_HEALTH_PERIOD)
 
     _IMDS_HEARTBEAT_PERIOD = datetime.timedelta(minutes=1)
     _IMDS_HEALTH_PERIOD = datetime.timedelta(minutes=3)
 
-    def _operation_impl(self):
+    def _operation(self):
         try:
             is_currently_healthy, response = self.imds_client.validate()
 
@@ -282,14 +261,14 @@ class MonitorHandler(ThreadHandlerInterface):
             protocol = protocol_util.get_protocol()
             health_service = HealthService(protocol.get_endpoint())
             periodic_operations = [
-                ResetPeriodicLogMessagesOperation(),
-                ReportNetworkErrorsOperation(),
-                PollResourceUsageOperation(),
-                SendHostPluginHeartbeatOperation(protocol, health_service),
-                SendImdsHeartbeatOperation(protocol_util, health_service)
+                ResetPeriodicLogMessages(),
+                ReportNetworkErrors(),
+                PollResourceUsage(),
+                SendHostPluginHeartbeat(protocol, health_service),
+                SendImdsHeartbeat(protocol_util, health_service)
             ]
 
-            report_network_configuration_changes = ReportNetworkConfigurationChangesOperation()
+            report_network_configuration_changes = ReportNetworkConfigurationChanges()
             if conf.get_monitor_network_configuration_changes():
                 periodic_operations.append(report_network_configuration_changes)
             else:
diff --git a/azurelinuxagent/ga/periodic_operation.py b/azurelinuxagent/ga/periodic_operation.py
index 5c54f1d9..35bc6e68 100644
--- a/azurelinuxagent/ga/periodic_operation.py
+++ b/azurelinuxagent/ga/periodic_operation.py
@@ -33,9 +33,8 @@ class PeriodicOperation(object):
     # To prevent flooding the log with error messages we report failures at most every hour
     _LOG_WARNING_PERIOD = datetime.timedelta(minutes=60)
 
-    def __init__(self, name, operation, period):
-        self._name = name
-        self._operation = operation
+    def __init__(self, period):
+        self._name = self.__class__.__name__
         self._period = period if isinstance(period, datetime.timedelta) else datetime.timedelta(seconds=period)
         self._next_run_time = datetime.datetime.utcnow()
         self._last_warning = None
@@ -45,11 +44,12 @@ class PeriodicOperation(object):
         try:
             if self._next_run_time <= datetime.datetime.utcnow():
                 try:
+                    logger.verbose("Executing {0}...", self._name)
                     self._operation()
                 finally:
                     self._next_run_time = datetime.datetime.utcnow() + self._period
         except Exception as e:
-            warning = "Failed to {0}: {1} --- [NOTE: Will not log the same error for the next hour]".format(self._name, ustr(e))
+            warning = "Error in {0}: {1} --- [NOTE: Will not log the same error for the next hour]".format(self._name, ustr(e))
             if warning != self._last_warning or self._last_warning_time is None or datetime.datetime.utcnow() >= self._last_warning_time + self._LOG_WARNING_PERIOD:
                 logger.warn(warning)
                 self._last_warning_time = datetime.datetime.utcnow()
@@ -58,6 +58,12 @@ class PeriodicOperation(object):
     def next_run_time(self):
         return self._next_run_time
 
+    def _operation(self):
+        """
+        Derived classes must override this with the definition of the operation they need to perform
+        """
+        raise NotImplementedError()
+
     @staticmethod
     def sleep_until_next_operation(operations):
         """


Test patch:
diff --git a/tests/common/test_event.py b/tests/common/test_event.py
index 3693ccb4..6bd18127 100644
--- a/tests/common/test_event.py
+++ b/tests/common/test_event.py
@@ -39,7 +39,7 @@ from azurelinuxagent.common.osutil import get_osutil
 from azurelinuxagent.common.telemetryevent import CommonTelemetryEventSchema, GuestAgentGenericLogsSchema, \
     GuestAgentExtensionEventsSchema, GuestAgentPerfCounterEventsSchema
 from azurelinuxagent.common.version import CURRENT_AGENT, CURRENT_VERSION, AGENT_EXECUTION_MODE
-from azurelinuxagent.ga.collect_telemetry_events import _CollectAndEnqueueEventsPeriodicOperation
+from azurelinuxagent.ga.collect_telemetry_events import _CollectAndEnqueueEvents
 from tests.protocol import mockwiredata
 from tests.protocol.mocks import mock_wire_protocol, HttpRequestPredicates, MockHttpResponse
 from tests.tools import AgentTestCase, data_dir, load_data, patch, skip_if_predicate_true
@@ -99,7 +99,7 @@ class TestEvent(HttpRequestPredicates, AgentTestCase):
         event_list = []
         send_telemetry_events = MagicMock()
         send_telemetry_events.enqueue_event = MagicMock(wraps=event_list.append)
-        event_collector = _CollectAndEnqueueEventsPeriodicOperation(send_telemetry_events)
+        event_collector = _CollectAndEnqueueEvents(send_telemetry_events)
         event_collector.process_events()
         return event_list
 
diff --git a/tests/ga/test_collect_telemetry_events.py b/tests/ga/test_collect_telemetry_events.py
index 9b85c7fb..8ca4dca0 100644
--- a/tests/ga/test_collect_telemetry_events.py
+++ b/tests/ga/test_collect_telemetry_events.py
@@ -35,7 +35,7 @@ from azurelinuxagent.common.protocol.util import ProtocolUtil
 from azurelinuxagent.common.telemetryevent import GuestAgentGenericLogsSchema, \
     CommonTelemetryEventSchema
 from azurelinuxagent.common.utils import fileutil
-from azurelinuxagent.ga.collect_telemetry_events import ExtensionEventSchema, _ProcessExtensionEventsPeriodicOperation
+from azurelinuxagent.ga.collect_telemetry_events import ExtensionEventSchema, _ProcessExtensionEvents
 from tests.protocol.mocks import HttpRequestPredicates
 from tests.tools import AgentTestCase, clear_singleton_instances, data_dir
 
@@ -172,7 +172,7 @@ class TestExtensionTelemetryHandler(AgentTestCase, HttpRequestPredicates):
             telemetry_handler = MagicMock(autospec=True)
             telemetry_handler.stopped = MagicMock(return_value=False)
             telemetry_handler.enqueue_event = MagicMock(wraps=event_list.append)
-        extension_telemetry_processor = _ProcessExtensionEventsPeriodicOperation(telemetry_handler)
+        extension_telemetry_processor = _ProcessExtensionEvents(telemetry_handler)
         extension_telemetry_processor.event_list = event_list
         yield extension_telemetry_processor
 
@@ -354,7 +354,7 @@ class TestExtensionTelemetryHandler(AgentTestCase, HttpRequestPredicates):
     def test_it_should_trim_message_if_more_than_limit(self):
         max_len = 100
         no_of_extensions = 2
-        with patch("azurelinuxagent.ga.collect_telemetry_events._ProcessExtensionEventsPeriodicOperation._EXTENSION_EVENT_MAX_MSG_LEN", max_len):
+        with patch("azurelinuxagent.ga.collect_telemetry_events._ProcessExtensionEvents._EXTENSION_EVENT_MAX_MSG_LEN", max_len):
             handler_name_with_count, event_list = self._setup_and_assert_tests_for_max_sizes()  # pylint: disable=unused-variable
             context1_vals = self._get_param_value_from_event_body_if_exists(event_list,
                                                                             GuestAgentGenericLogsSchema.Context1)
@@ -368,7 +368,7 @@ class TestExtensionTelemetryHandler(AgentTestCase, HttpRequestPredicates):
         max_size = 1000
         no_of_extensions = 3
         with patch("azurelinuxagent.ga.collect_telemetry_events.add_log_event") as mock_event:
-            with patch("azurelinuxagent.ga.collect_telemetry_events._ProcessExtensionEventsPeriodicOperation._EXTENSION_EVENT_MAX_SIZE",
+            with patch("azurelinuxagent.ga.collect_telemetry_events._ProcessExtensionEvents._EXTENSION_EVENT_MAX_SIZE",
                        max_size):
                 handler_name_with_count, _ = self._setup_and_assert_tests_for_max_sizes(no_of_extensions, expected_count=0)
                 self._assert_invalid_extension_error_event_reported(mock_event, handler_name_with_count,
@@ -378,7 +378,7 @@ class TestExtensionTelemetryHandler(AgentTestCase, HttpRequestPredicates):
         max_file_size = 10000
         no_of_extensions = 5
         with patch("azurelinuxagent.ga.collect_telemetry_events.add_log_event") as mock_event:
-            with patch("azurelinuxagent.ga.collect_telemetry_events._ProcessExtensionEventsPeriodicOperation._EXTENSION_EVENT_FILE_MAX_SIZE",
+            with patch("azurelinuxagent.ga.collect_telemetry_events._ProcessExtensionEvents._EXTENSION_EVENT_FILE_MAX_SIZE",
                        max_file_size):
                 handler_name_with_count, _ = self._setup_and_assert_tests_for_max_sizes(no_of_extensions, expected_count=0)
 
diff --git a/tests/ga/test_env.py b/tests/ga/test_env.py
index 26ff3449..553d3e0d 100644
--- a/tests/ga/test_env.py
+++ b/tests/ga/test_env.py
@@ -14,12 +14,17 @@
 #
 # Requires Python 2.6+ and Openssl 1.0+
 #
+import datetime
+
+from azurelinuxagent.common.osutil import get_osutil
 from azurelinuxagent.common.osutil.default import DefaultOSUtil, shellutil
-from azurelinuxagent.ga.env import EnvHandler
+from azurelinuxagent.ga.env import MonitorDhcpClientRestart
 from tests.tools import AgentTestCase, patch
 
-
-class TestEnvHandler(AgentTestCase):
+# Access to a protected member _get_dhcp_client_pid of a client class (protected-access): Disabled; OK
+# to access private/protected members from this unit test
+# pylint: disable=protected-access
+class MonitorDhcpClientRestartTestCase(AgentTestCase):
     def setUp(self):
         AgentTestCase.setUp(self)
 
@@ -40,13 +45,13 @@ class TestEnvHandler(AgentTestCase):
 
     def test_get_dhcp_client_pid_should_return_a_sorted_list_of_pids(self):
         with patch("azurelinuxagent.common.utils.shellutil.run_command", return_value="11 9 5 22 4 6"):
-            pids = EnvHandler().get_dhcp_client_pid()
+            pids = MonitorDhcpClientRestart(get_osutil())._get_dhcp_client_pid()
             self.assertEqual(pids, [4, 5, 6, 9, 11, 22])
 
     def test_get_dhcp_client_pid_should_return_an_empty_list_and_log_a_warning_when_dhcp_client_is_not_running(self):
         with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=lambda _: self.shellutil_run_command(["pidof", "non-existing-process"])):
             with patch('azurelinuxagent.common.logger.Logger.warn') as mock_warn:
-                pids = EnvHandler().get_dhcp_client_pid()
+                pids = MonitorDhcpClientRestart(get_osutil())._get_dhcp_client_pid()
 
         self.assertEqual(pids, [])
 
@@ -58,7 +63,7 @@ class TestEnvHandler(AgentTestCase):
     def test_get_dhcp_client_pid_should_return_and_empty_list_and_log_an_error_when_an_invalid_command_is_used(self):
         with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=lambda _: self.shellutil_run_command(["non-existing-command"])):
             with patch('azurelinuxagent.common.logger.Logger.error') as mock_error:
-                pids = EnvHandler().get_dhcp_client_pid()
+                pids = MonitorDhcpClientRestart(get_osutil())._get_dhcp_client_pid()
 
         self.assertEqual(pids, [])
 
@@ -68,57 +73,54 @@ class TestEnvHandler(AgentTestCase):
         self.assertIn("No such file or directory", args[1])
 
     def test_get_dhcp_client_pid_should_not_log_consecutive_errors(self):
-        env_handler = EnvHandler()
+        monitor_dhcp_client_restart = MonitorDhcpClientRestart(get_osutil())
 
         with patch('azurelinuxagent.common.logger.Logger.warn') as mock_warn:
             def assert_warnings(count):
                 self.assertEqual(mock_warn.call_count, count)
 
                 for call_args in mock_warn.call_args_list:
-                    args, kwargs = call_args  # pylint: disable=unused-variable
+                    args, _ = call_args
                     self.assertEqual("Dhcp client is not running.", args[0])
 
             with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=lambda _: self.shellutil_run_command(["pidof", "non-existing-process"])):
                 # it should log the first error
-                pids = env_handler.get_dhcp_client_pid()
+                pids = monitor_dhcp_client_restart._get_dhcp_client_pid()
                 self.assertEqual(pids, [])
                 assert_warnings(1)
 
                 # it should not log subsequent errors
-                for i in range(0, 3):  # pylint: disable=unused-variable
-                    pids = env_handler.get_dhcp_client_pid()
+                for _ in range(0, 3):
+                    pids = monitor_dhcp_client_restart._get_dhcp_client_pid()
                     self.assertEqual(pids, [])
                     self.assertEqual(mock_warn.call_count, 1)
 
             with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", return_value="123"):
                 # now it should succeed
-                pids = env_handler.get_dhcp_client_pid()
+                pids = monitor_dhcp_client_restart._get_dhcp_client_pid()
                 self.assertEqual(pids, [123])
                 assert_warnings(1)
 
             with patch("azurelinuxagent.common.osutil.default.shellutil.run_command", side_effect=lambda _: self.shellutil_run_command(["pidof", "non-existing-process"])):
                 # it should log the new error
-                pids = env_handler.get_dhcp_client_pid()
+                pids = monitor_dhcp_client_restart._get_dhcp_client_pid()
                 self.assertEqual(pids, [])
                 assert_warnings(2)
 
                 # it should not log subsequent errors
-                for i in range(0, 3):
-                    pids = env_handler.get_dhcp_client_pid()
+                for _ in range(0, 3):
+                    pids = monitor_dhcp_client_restart._get_dhcp_client_pid()
                     self.assertEqual(pids, [])
                     self.assertEqual(mock_warn.call_count, 2)
 
     def test_handle_dhclient_restart_should_reconfigure_network_routes_when_dhcp_client_restarts(self):
         with patch("azurelinuxagent.common.dhcp.DhcpHandler.conf_routes") as mock_conf_routes:
-            env_handler = EnvHandler()
+            monitor_dhcp_client_restart = MonitorDhcpClientRestart(get_osutil())
+            monitor_dhcp_client_restart._period = datetime.timedelta(seconds=0)
 
-            #
-            # before the first call to handle_dhclient_restart, EnvHandler configures the network routes and initializes the DHCP PIDs
-            #
-            with patch.object(env_handler, "get_dhcp_client_pid", return_value=[123]):
-                env_handler.dhcp_handler.conf_routes()
-                env_handler.dhcp_id_list = env_handler.get_dhcp_client_pid()
-                self.assertEqual(mock_conf_routes.call_count, 1)
+            # Run the operation one time to initialize the DHCP PIDs
+            with patch.object(monitor_dhcp_client_restart, "_get_dhcp_client_pid", return_value=[123]):
+                monitor_dhcp_client_restart.run()
 
             #
             # if the dhcp client has not been restarted then it should not reconfigure the network routes
@@ -129,8 +131,8 @@ class TestEnvHandler(AgentTestCase):
                 raise Exception("Unexpected PID: {0}".format(pid))
 
             with patch("azurelinuxagent.common.osutil.default.DefaultOSUtil.check_pid_alive", side_effect=mock_check_pid_alive):
-                with patch.object(env_handler, "get_dhcp_client_pid", side_effect=Exception("get_dhcp_client_pid should not have been invoked")):
-                    env_handler.handle_dhclient_restart()
+                with patch.object(monitor_dhcp_client_restart, "_get_dhcp_client_pid", side_effect=Exception("get_dhcp_client_pid should not have been invoked")):
+                    monitor_dhcp_client_restart.run()
                     self.assertEqual(mock_conf_routes.call_count, 1)  # count did not change
 
             #
@@ -142,8 +144,8 @@ class TestEnvHandler(AgentTestCase):
                 raise Exception("Unexpected PID: {0}".format(pid))
 
             with patch("azurelinuxagent.common.osutil.default.DefaultOSUtil.check_pid_alive", side_effect=mock_check_pid_alive):
-                with patch.object(env_handler, "get_dhcp_client_pid", return_value=[456, 789]):
-                    env_handler.handle_dhclient_restart()
+                with patch.object(monitor_dhcp_client_restart, "_get_dhcp_client_pid", return_value=[456, 789]):
+                    monitor_dhcp_client_restart.run()
                     self.assertEqual(mock_conf_routes.call_count, 2)  # count increased
 
             #
@@ -155,6 +157,6 @@ class TestEnvHandler(AgentTestCase):
                 raise Exception("Unexpected PID: {0}".format(pid))
 
             with patch("azurelinuxagent.common.osutil.default.DefaultOSUtil.check_pid_alive", side_effect=mock_check_pid_alive):
-                with patch.object(env_handler, "get_dhcp_client_pid", side_effect=Exception("get_dhcp_client_pid should not have been invoked")):
-                    env_handler.handle_dhclient_restart()
+                with patch.object(monitor_dhcp_client_restart, "_get_dhcp_client_pid", side_effect=Exception("get_dhcp_client_pid should not have been invoked")):
+                    monitor_dhcp_client_restart.run()
                     self.assertEqual(mock_conf_routes.call_count, 2)  # count did not change
diff --git a/tests/ga/test_monitor.py b/tests/ga/test_monitor.py
index d500a94c..b690bca6 100644
--- a/tests/ga/test_monitor.py
+++ b/tests/ga/test_monitor.py
@@ -27,8 +27,9 @@ from azurelinuxagent.common.event import EVENTS_DIRECTORY
 from azurelinuxagent.common.protocol.healthservice import HealthService
 from azurelinuxagent.common.protocol.util import ProtocolUtil
 from azurelinuxagent.common.protocol.wire import WireProtocol
-from azurelinuxagent.ga.monitor import get_monitor_handler, PeriodicOperation, \
-    ResetPeriodicLogMessagesOperation, SendHostPluginHeartbeatOperation, PollResourceUsageOperation
+from azurelinuxagent.ga.monitor import get_monitor_handler, PeriodicOperation, SendImdsHeartbeat, \
+    ResetPeriodicLogMessages, SendHostPluginHeartbeat, PollResourceUsage, \
+    ReportNetworkErrors, ReportNetworkConfigurationChanges
 from tests.protocol.mocks import mock_wire_protocol, HttpRequestPredicates, MockHttpResponse
 from tests.protocol.mockwiredata import DATA_FILE
 from tests.tools import Mock, MagicMock, patch, AgentTestCase, clear_singleton_instances
@@ -54,8 +55,7 @@ def _mock_wire_protocol():
 class MonitorHandlerTestCase(AgentTestCase):
     def test_it_should_invoke_all_periodic_operations(self):
         def periodic_operation_run(self):
-            invoked_operations.append(self._name)
-        invoked_operations = []
+            invoked_operations.append(self.__class__.__name__)
 
         with _mock_wire_protocol():
             with patch("azurelinuxagent.ga.monitor.MonitorHandler.stopped", side_effect=[False, True, False, True]):
@@ -72,15 +72,15 @@ class MonitorHandlerTestCase(AgentTestCase):
                                 monitor_handler.join()
 
                                 expected_operations = [
-                                    'poll resource usage',
-                                    'report network errors',
-                                    'reset periodic log messages',
-                                    'send_host_plugin_heartbeat',
-                                    'send_imds_heartbeat'
+                                    PollResourceUsage.__name__,
+                                    ReportNetworkErrors.__name__,
+                                    ResetPeriodicLogMessages.__name__,
+                                    SendHostPluginHeartbeat.__name__,
+                                    SendImdsHeartbeat.__name__,
                                 ]
 
                                 if network_changes:
-                                    expected_operations.append('report network configuration changes')
+                                    expected_operations.append(ReportNetworkConfigurationChanges.__name__)
 
                                 invoked_operations.sort()
                                 expected_operations.sort()
@@ -101,7 +101,7 @@ class SendHostPluginHeartbeatOperationTestCase(AgentTestCase, HttpRequestPredica
 
             health_service = HealthService(protocol.get_endpoint())
 
-            SendHostPluginHeartbeatOperation(protocol, health_service).run()
+            SendHostPluginHeartbeat(protocol, health_service).run()
 
             self.assertTrue(http_post_handler.health_service_posted, "The monitor thread did not report host ga plugin health")
 
@@ -118,7 +118,7 @@ class SendHostPluginHeartbeatOperationTestCase(AgentTestCase, HttpRequestPredica
 
                     health_service = HealthService(protocol.get_endpoint())
 
-                    SendHostPluginHeartbeatOperation(protocol, health_service).run()
+                    SendHostPluginHeartbeat(protocol, health_service).run()
 
                     heartbeat_events = [kwargs for _, kwargs in add_event_patcher.call_args_list if kwargs['op'] == 'HostPluginHeartbeatExtended']
                     self.assertTrue(len(heartbeat_events) == 1, "The monitor thread should have reported exactly 1 telemetry event for an unhealthy host ga plugin")
@@ -145,7 +145,7 @@ class SendHostPluginHeartbeatOperationTestCase(AgentTestCase, HttpRequestPredica
 
                 health_service = HealthService(protocol.get_endpoint())
 
-                SendHostPluginHeartbeatOperation(protocol, health_service).run()
+                SendHostPluginHeartbeat(protocol, health_service).run()
 
                 self.assertEqual(0, len(health_service_post_requests), "No health signals should have been posted: {0}".format(health_service_post_requests))
 
@@ -169,7 +169,7 @@ class ResetPeriodicLogMessagesOperationTestCase(AgentTestCase, HttpRequestPredic
         if actual != expected:
             raise Exception('Test setup error: the periodic messages were not added. Got: {0} Expected: {1}'.format(actual, expected))
 
-        ResetPeriodicLogMessagesOperation().run()
+        ResetPeriodicLogMessages().run()
 
         self.assertEqual(0, len(logger.DEFAULT_LOGGER.periodic_messages), "The monitor thread did not reset the periodic log messages")
 
@@ -204,7 +204,7 @@ class TestExtensionMetricsDataTelemetry(AgentTestCase):
                                                MetricValue("Memory", "Total Memory Usage", 1, 1),
                                                MetricValue("Memory", "Max Memory Usage", 1, 1)]
 
-        PollResourceUsageOperation().run()
+        PollResourceUsage().run()
         self.assertEqual(1, patch_poll_all_tracked.call_count)
         self.assertEqual(3, patch_add_metric.call_count)  # Three metrics being sent.
 
@@ -214,7 +214,7 @@ class TestExtensionMetricsDataTelemetry(AgentTestCase):
                                                                patch_add_metric,*args):
         patch_poll_all_tracked.return_value = []
 
-        PollResourceUsageOperation().run()
+        PollResourceUsage().run()
         self.assertEqual(1, patch_poll_all_tracked.call_count)
         self.assertEqual(0, patch_add_metric.call_count)
 
@@ -230,7 +230,7 @@ class TestExtensionMetricsDataTelemetry(AgentTestCase):
 
         CGroupsTelemetry._tracked.append(MemoryCgroup("cgroup_name", "/test/path"))  # pylint: disable=protected-access
 
-        PollResourceUsageOperation().run()
+        PollResourceUsage().run()
         self.assertEqual(0, patch_periodic_warn.call_count)
         self.assertEqual(0, patch_add_metric.call_count)  # No metrics should be sent.
 
@@ -246,7 +246,7 @@ class TestExtensionMetricsDataTelemetry(AgentTestCase):
 
         CGroupsTelemetry._tracked.append(CpuCgroup("cgroup_name", "/test/path"))  # pylint: disable=protected-access
 
-        PollResourceUsageOperation().run()
+        PollResourceUsage().run()
         self.assertEqual(0, patch_periodic_warn.call_count)
         self.assertEqual(0, patch_add_metric.call_count)  # No metrics should be sent.
 
@@ -255,7 +255,7 @@ class TestExtensionMetricsDataTelemetry(AgentTestCase):
     def test_send_extension_metrics_telemetry_for_unsupported_cgroup(self, patch_periodic_warn, patch_add_metric, *args):  # pylint: disable=unused-argument
         CGroupsTelemetry._tracked.append(CGroup("cgroup_name", "/test/path", "io"))  # pylint: disable=protected-access
 
-        PollResourceUsageOperation().run()
+        PollResourceUsage().run()
         self.assertEqual(1, patch_periodic_warn.call_count)
         self.assertEqual(0, patch_add_metric.call_count)  # No metrics should be sent.
 
diff --git a/tests/ga/test_periodic_operation.py b/tests/ga/test_periodic_operation.py
index e6aa5d7f..8fd8d32d 100644
--- a/tests/ga/test_periodic_operation.py
+++ b/tests/ga/test_periodic_operation.py
@@ -21,68 +21,65 @@ from tests.tools import AgentTestCase, patch, PropertyMock
 
 
 class TestPeriodicOperation(AgentTestCase):
-    def test_it_should_take_a_timedelta_as_period(self):
-        def operation():
-            operation.run_time = datetime.datetime.utcnow()
-        operation.run_time = None
+    class SaveRunTimestamp(PeriodicOperation):
+        def __init__(self, period):
+            super(TestPeriodicOperation.SaveRunTimestamp, self).__init__(period)
+            self.run_time = None
+
+        def _operation(self):
+            self.run_time = datetime.datetime.utcnow()
 
-        op = PeriodicOperation("test_operation", operation, period=datetime.timedelta(hours=1))
+    def test_it_should_take_a_timedelta_as_period(self):
+        op = TestPeriodicOperation.SaveRunTimestamp(datetime.timedelta(hours=1))
         op.run()
 
-        expected = operation.run_time + datetime.timedelta(hours=1)
+        expected = op.run_time + datetime.timedelta(hours=1)
         difference = op.next_run_time() - expected
         self.assertTrue(difference < datetime.timedelta(seconds=1),
             "The next run time exceeds the expected value by more than 1 second: {0} vs {1}".format(op.next_run_time(), expected))
 
     def test_it_should_take_a_number_of_seconds_as_period(self):
-        def operation():
-            operation.run_time = datetime.datetime.utcnow()
-        operation.run_time = None
-
-        op = PeriodicOperation("test_operation", operation, period=3600)
+        op = TestPeriodicOperation.SaveRunTimestamp(3600)
         op.run()
 
-        expected = operation.run_time + datetime.timedelta(hours=1)
+        expected = op.run_time + datetime.timedelta(hours=1)
         difference = op.next_run_time() - expected
         self.assertTrue(difference < datetime.timedelta(seconds=1),
             "The next run time exceeds the expected value by more than 1 second: {0} vs {1}".format(op.next_run_time(), expected))
 
-    def test_it_should_be_invoked_when_run_is_called_first_time(self):
-        def operation():
-            operation.invoked = True
-        operation.invoked = False
+    class CountInvocations(PeriodicOperation):
+        def __init__(self, period):
+            super(TestPeriodicOperation.CountInvocations, self).__init__(period)
+            self.invoke_count = 0
 
-        PeriodicOperation("test_operation", operation, period=datetime.timedelta(hours=1)).run()
+        def _operation(self):
+            self.invoke_count += 1
 
-        self.assertTrue(operation.invoked, "The operation was not invoked")
+    def test_it_should_be_invoked_when_run_is_called_first_time(self):
+        op = TestPeriodicOperation.CountInvocations(datetime.timedelta(hours=1))
+        op.run()
 
-    def test_it_should_not_be_invoked_if_the_period_has_not_elapsed(self):
-        def operation():
-            operation.invoked_count += 1
-        operation.invoked_count = 0
+        self.assertTrue(op.invoke_count > 0, "The operation was not invoked")
 
-        pop = PeriodicOperation("test_operation", operation, period=datetime.timedelta(hours=1))
+    def test_it_should_not_be_invoked_if_the_period_has_not_elapsed(self):
+        pop = TestPeriodicOperation.CountInvocations(datetime.timedelta(hours=1))
         for _ in range(5):
             pop.run()
 
         # the first run() invoked the operation, so the count is 1
-        self.assertEqual(operation.invoked_count, 1, "The operation was invoked before the period elapsed")
+        self.assertEqual(pop.invoke_count, 1, "The operation was invoked before the period elapsed")
 
     def test_it_should_be_invoked_if_the_period_has_elapsed(self):
-        def operation():
-            operation.invoked_count += 1
-        operation.invoked_count = 0
-
-        pop = PeriodicOperation("test_operation", operation, period=datetime.timedelta(milliseconds=1))
+        pop = TestPeriodicOperation.CountInvocations(datetime.timedelta(milliseconds=1))
         for _ in range(5):
             pop.run()
             time.sleep(0.001)
 
-        self.assertEqual(operation.invoked_count, 5, "The operation was not invoked after the period elapsed")
+        self.assertEqual(pop.invoke_count, 5, "The operation was not invoked after the period elapsed")
 
-    @staticmethod
-    def _operation_with_failure():
-        raise Exception("A test exception")
+    class RaiseException(PeriodicOperation):
+        def _operation(self):
+            raise Exception("A test exception")
 
     @staticmethod
     def _get_number_of_warnings(warn_patcher, message="A test exception"):
@@ -90,13 +87,13 @@ class TestPeriodicOperation(AgentTestCase):
 
     def test_it_should_log_a_warning_if_the_operation_fails(self):
         with patch("azurelinuxagent.common.logger.warn") as warn_patcher:
-            PeriodicOperation("test_operation", self._operation_with_failure, period=datetime.timedelta(hours=1)).run()
+            TestPeriodicOperation.RaiseException(datetime.timedelta(hours=1)).run()
 
         self.assertEqual(self._get_number_of_warnings(warn_patcher), 1, "The error in the operation was should have been reported exactly once")
 
     def test_it_should_not_log_multiple_warnings_when_the_period_has_not_elapsed(self):
         with patch("azurelinuxagent.common.logger.warn") as warn_patcher:
-            pop = PeriodicOperation("test_operation", self._operation_with_failure, period=datetime.timedelta(hours=1))
+            pop = TestPeriodicOperation.RaiseException(datetime.timedelta(hours=1))
             for _ in range(5):
                 pop.run()
 
@@ -105,33 +102,45 @@ class TestPeriodicOperation(AgentTestCase):
     def test_it_should_not_log_multiple_warnings_when_the_period_has_elapsed(self):
         with patch("azurelinuxagent.common.logger.warn") as warn_patcher:
             with patch("azurelinuxagent.ga.periodic_operation.PeriodicOperation._LOG_WARNING_PERIOD", new_callable=PropertyMock, return_value=datetime.timedelta(milliseconds=1)):
-                pop = PeriodicOperation("test_operation", self._operation_with_failure, period=datetime.timedelta(milliseconds=1))
+                pop = TestPeriodicOperation.RaiseException(datetime.timedelta(milliseconds=1))
                 for _ in range(5):
                     pop.run()
                     time.sleep(0.001)
 
             self.assertEqual(self._get_number_of_warnings(warn_patcher), 5, "The error in the operation was not reported the expected number of times")
 
+    class RaiseTwoExceptions(PeriodicOperation):
+        def __init__(self, period):
+            super(TestPeriodicOperation.RaiseTwoExceptions, self).__init__(period)
+            self._count = 0
+
+        def _operation(self):
+            message = "WARNING {0}".format(self._count)
+            if self._count == 0:
+                self._count += 1
+            raise Exception(message)
+
     def test_it_should_log_warnings_if_they_are_different(self):
         with patch("azurelinuxagent.common.logger.warn") as warn_patcher:
-            for i in range(2):
-                def operation():
-                    raise Exception("WARNING {0}".format(i))  # pylint: disable=cell-var-from-loop
+            pop = TestPeriodicOperation.RaiseTwoExceptions(0)
 
-                pop = PeriodicOperation("test_operation", operation, period=datetime.timedelta(hours=1))
-                for _ in range(5):
-                    pop.run()
+            for _ in range(5):
+                pop.run()
 
             self.assertEqual(self._get_number_of_warnings(warn_patcher, "WARNING 0"), 1, "The first error should have been reported exactly 1 time")
             self.assertEqual(self._get_number_of_warnings(warn_patcher, "WARNING 1"), 1, "The second error should have been reported exactly 1 time")
 
+    class NoOp(PeriodicOperation):
+        def _operation(self):
+            pass
+
     def test_sleep_until_next_operation_should_wait_for_the_closest_operation(self):
         operations = [
-            PeriodicOperation("one", lambda: None, period=datetime.timedelta(seconds=60)),
-            PeriodicOperation("one", lambda: None, period=datetime.timedelta(hours=1)),
-            PeriodicOperation("one", lambda: None, period=datetime.timedelta(seconds=10)),  # closest operation
-            PeriodicOperation("one", lambda: None, period=datetime.timedelta(minutes=11)),
-            PeriodicOperation("one", lambda: None, period=datetime.timedelta(days=1))
+            TestPeriodicOperation.NoOp(datetime.timedelta(seconds=60)),
+            TestPeriodicOperation.NoOp(datetime.timedelta(hours=1)),
+            TestPeriodicOperation.NoOp(datetime.timedelta(seconds=10)),  # closest operation
+            TestPeriodicOperation.NoOp(datetime.timedelta(minutes=11)),
+            TestPeriodicOperation.NoOp(datetime.timedelta(days=1))
         ]
         for op in operations:
             op.run()
diff --git a/tests/ga/test_send_telemetry_events.py b/tests/ga/test_send_telemetry_events.py
index 7f9b2d84..5f14bab0 100644
--- a/tests/ga/test_send_telemetry_events.py
+++ b/tests/ga/test_send_telemetry_events.py
@@ -39,7 +39,7 @@ from azurelinuxagent.common.telemetryevent import TelemetryEvent, TelemetryEvent
 from azurelinuxagent.common.utils import restutil, fileutil
 from azurelinuxagent.common.version import CURRENT_VERSION, DISTRO_NAME, DISTRO_VERSION, AGENT_VERSION, CURRENT_AGENT, \
     DISTRO_CODE_NAME
-from azurelinuxagent.ga.collect_telemetry_events import _CollectAndEnqueueEventsPeriodicOperation
+from azurelinuxagent.ga.collect_telemetry_events import _CollectAndEnqueueEvents
 from azurelinuxagent.ga.send_telemetry_events import get_send_telemetry_events_handler
 from tests.ga.test_monitor import random_generator
 from tests.protocol.mocks import MockHttpResponse, mock_wire_protocol, HttpRequestPredicates
@@ -326,7 +326,7 @@ class TestSendTelemetryEventsHandler(AgentTestCase, HttpRequestPredicates):
         mock_lib_dir.return_value = self.lib_dir
 
         with self._create_send_telemetry_events_handler() as telemetry_handler:
-            monitor_handler = _CollectAndEnqueueEventsPeriodicOperation(telemetry_handler)
+            monitor_handler = _CollectAndEnqueueEvents(telemetry_handler)
             self._create_extension_event(message="Message-Test")
 
             test_mtime = 1000  # epoch time, in ms
@@ -397,7 +397,7 @@ class TestSendTelemetryEventsHandler(AgentTestCase, HttpRequestPredicates):
                 size = 2 ** power
                 self._create_extension_event(size)
 
-            _CollectAndEnqueueEventsPeriodicOperation(telemetry_handler).run()
+            _CollectAndEnqueueEvents(telemetry_handler).run()
 
             # The send_event call would be called each time, as we are filling up the buffer up to the brim for each call.
             TestSendTelemetryEventsHandler._stop_handler(telemetry_handler)
@@ -415,7 +415,7 @@ class TestSendTelemetryEventsHandler(AgentTestCase, HttpRequestPredicates):
                 self._create_extension_event(size)
 
             with patch("azurelinuxagent.common.logger.periodic_warn") as patch_periodic_warn:
-                _CollectAndEnqueueEventsPeriodicOperation(telemetry_handler).run()
+                _CollectAndEnqueueEvents(telemetry_handler).run()
                 TestSendTelemetryEventsHandler._stop_handler(telemetry_handler)
                 self.assertEqual(3, patch_periodic_warn.call_count)