Merge branch 'main' into dev-mode-timetravel

* main:
  Dev mode (#637)
  Hotfix: resolutions -> resolution
  Adds skip_checks to providers (#566)
  Bump pytest from 7.4.3 to 7.4.4 (#639)
  Disabled second PSU provider for VM tests
  System check providers running (#619)
  Stderr is now by default UTF-8 (#624)
  Refactored kill/killpg mechanism to be unified and actually fail on n… (#625)
  Command fix. Must be list append
  Refactorings
  Moved tinyproxy out of if clause
  Refactoring for error messages and security fix for path echoing (#636)

Author: ArneTR

.github/workflows/tests-vm-main.yml

@@ -34,7 +34,7 @@ jobs:
         name: 'Setup, Run, and Teardown Tests'
         uses: ./.github/actions/gmt-pytest
         with:
-         metrics-to-turn-off: '--categories RAPL Machine Sensors Debug CGroupV2 MacOS'
+         metrics-to-turn-off: '--categories RAPL Machine Sensors Debug CGroupV2 MacOS --providers PsuEnergyAcSdiaMachineProvider'
          github-token: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Eco CI Energy Estimation - Get Measurement

.github/workflows/tests-vm-pr.yml

@@ -25,7 +25,7 @@ jobs:
       - name: 'Setup, Run, and Teardown Tests'
         uses: ./.github/actions/gmt-pytest
         with:
-         metrics-to-turn-off: '--categories RAPL Machine Sensors Debug CGroupV2 MacOS'
+         metrics-to-turn-off: '--categories RAPL Machine Sensors Debug CGroupV2 MacOS --providers PsuEnergyAcSdiaMachineProvider'
          github-token: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Eco CI Energy Estimation - Get Measurement

install_linux.sh

@@ -117,14 +117,12 @@ git submodule update --init
 print_message "Installing needed binaries for building ..."
 if lsb_release -is | grep -q "Fedora"; then
     sudo dnf -y install lm_sensors lm_sensors-devel glib2 glib2-devel tinyproxy lshw
-    sudo systemctl stop tinyproxy
-    sudo systemctl disable tinyproxy
 else
     sudo apt-get update
     sudo apt-get install -y lm-sensors libsensors-dev libglib2.0-0 libglib2.0-dev tinyproxy lshw
-    sudo systemctl stop tinyproxy
-    sudo systemctl disable tinyproxy
 fi
+sudo systemctl stop tinyproxy
+sudo systemctl disable tinyproxy
 
 print_message "Building binaries ..."
 metrics_subdir="metric_providers"

lib/process_helpers.py

@@ -2,36 +2,28 @@
 import os
 import subprocess
 
-def kill_ps(ps_to_kill):
-    print('Killing processes')
-    for ps_info in ps_to_kill:
-        pid = ps_info['ps'].pid
-        print(f"Trying to kill {ps_info['cmd']} with PID: {pid}")
-        try:
-            if ps_info['ps_group'] is True:
-                try:
-                    ps_group_id = os.getpgid(pid)
-                    print(f" with process group {ps_group_id}")
-                    os.killpg(os.getpgid(pid), signal.SIGTERM)
-                    try:
-                        ps_info['ps'].wait(timeout=5)
-                    except subprocess.TimeoutExpired:
-                        # If the process hasn't gracefully exited after 5 seconds we kill it
-                        os.killpg(os.getpgid(pid), signal.SIGKILL)
-                except ProcessLookupError:
-                    # process may be not have been in a process group
-                    print(f"Could not find process-group for {pid}")
-            # always, just in case the calling process (typically the shell) did not die
-            os.kill(pid, signal.SIGTERM)
-            try:
-                ps_info['ps'].wait(timeout=5)
-            except subprocess.TimeoutExpired:
-                # If the process hasn't gracefully exited after 5 seconds we kill it
-                os.kill(pid, signal.SIGKILL)
+def kill_pg(ps, cmd):
+    pgid = os.getpgid(ps.pid)
+    print(f"Trying to kill {cmd} with PGID: {pgid}")
+
+    os.killpg(pgid, signal.SIGTERM)
+    try:
+        ps.wait(timeout=10)
+    except subprocess.TimeoutExpired as exc:
+        # If the process hasn't gracefully exited after 5 seconds we kill it
+        os.killpg(pgid, signal.SIGKILL)
+        raise RuntimeError(f"Killed the process {cmd} with SIGKILL. This could lead to corrupted data!") from exc
 
-        except ProcessLookupError:
-            # process may already have ended or been killed in the process group
-            print(f"Could not find process {pid}")
+def kill_ps(ps, cmd):
+    pid = ps.pid
+    print(f"Trying to kill {cmd} with PID: {pid}")
+
+    os.kill(pid, signal.SIGTERM)
+    try:
+        ps.wait(timeout=10)
+    except subprocess.TimeoutExpired as exc:
+        os.kill(pid, signal.SIGKILL)
+        raise RuntimeError(f"Killed the process {cmd} with SIGKILL. This could lead to corrupted data!") from exc
 
 # currently unused
 def timeout(process, cmd: str, duration: int):

lib/system_checks.py

@@ -12,6 +12,7 @@
 from enum import Enum
 import subprocess
 import psutil
+import locale
 
 from psycopg import OperationalError as psycopg_OperationalError
 
@@ -68,6 +69,8 @@ def check_docker_daemon():
                             check=False, encoding='UTF-8')
     return result.returncode == 0
 
+def check_utf_encoding():
+    return locale.getpreferredencoding().lower() == sys.getdefaultencoding().lower() == 'utf-8'
 
 ######## END CHECK FUNCTIONS ########
 
@@ -78,7 +81,8 @@ def check_docker_daemon():
     (check_free_disk, Status.ERROR, '1GB free hdd space', 'We recommend to free up some disk space'),
     (check_free_memory, Status.ERROR, 'free memory', 'No free memory! Please kill some programs'),
     (check_docker_daemon, Status.ERROR, 'docker daemon', 'The docker daemon could not be reached. Are you running in rootless mode or have added yourself to the docker group? See installation: [See https://docs.green-coding.berlin/docs/installation/]'),
-    (check_containers_running, Status.WARN, 'Running containers', 'You have other containers running on the system. This is usually what you want in local development, but for undisturbed measurements consider going for a measurement cluster [See https://docs.green-coding.berlin/docs/installation/installation-cluster/].'),
+    (check_containers_running, Status.WARN, 'running containers', 'You have other containers running on the system. This is usually what you want in local development, but for undisturbed measurements consider going for a measurement cluster [See https://docs.green-coding.berlin/docs/installation/installation-cluster/].'),
+    (check_utf_encoding, Status.ERROR, 'utf file encoding', 'Your system encoding is not set to utf-8. This is needed as we need to parse console output.'),
 
 ]
 

metric_providers/base.py

@@ -1,12 +1,11 @@
 import os
 from pathlib import Path
 import subprocess
-import signal
-import sys
 from io import StringIO
 import pandas
 
 from lib.system_checks import ConfigurationCheckError
+from lib import process_helpers
 
 class MetricProviderConfigurationError(ConfigurationCheckError):
     pass
@@ -22,7 +21,8 @@ def __init__(
         current_dir,
         metric_provider_executable='metric-provider-binary',
         sudo=False,
-        disable_buffer=True
+        disable_buffer=True,
+        skip_check=False,
     ):
         self._metric_name = metric_name
         self._metrics = metrics
@@ -34,6 +34,7 @@ def __init__(
         self._has_started = False
         self._disable_buffer = disable_buffer
         self._rootless = None
+        self._skip_check = skip_check
 
         self._tmp_folder = '/tmp/green-metrics-tool'
         self._ps = None
@@ -43,16 +44,37 @@ def __init__(
 
         self._filename = f"{self._tmp_folder}/{self._metric_name}.log"
 
-        self.check_system()
+        if not self._skip_check:
+            self.check_system()
 
     # this is the default function that will be overridden in the children
     def check_system(self):
-        pass
+        cmd = ['pgrep', '-f', self._metric_provider_executable]
+        result = subprocess.run(cmd,
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE,
+                            check=False, encoding='UTF-8')
+        if result.returncode == 1:
+            return True
+        if result.returncode == 0:
+            raise MetricProviderConfigurationError(f"Another instance of the {self._metric_name} metrics provider is already running on the system!\nPlease close it before running the Green Metrics Tool.")
+        # implicit else
+        raise subprocess.CalledProcessError(result.stderr, cmd)
 
     # implemented as getter function and not direct access, so it can be overloaded
     # some child classes might not actually have _ps attribute set
+    #
+    # This function has to go through quite some hoops to read the stderr
+    # The preferred way to communicate with processes is through communicate()
+    # However this function ALWAYS waits for the process to terminate and it does not allow reading from processes
+    # in chunks while they are running. Thus we we cannot set encoding='UTF-8' in Popen and must decode here.
     def get_stderr(self):
-        return self._ps.stderr.read()
+        stderr_read = ''
+        if self._ps.stderr is not None:
+            stderr_read = self._ps.stderr.read()
+            if isinstance(stderr_read, bytes):
+                stderr_read = stderr_read.decode('utf-8')
+        return stderr_read
 
     def has_started(self):
         return self._has_started
@@ -132,7 +154,9 @@ def start_profiling(self, containers=None):
             [call_string],
             shell=True,
             preexec_fn=os.setsid,
-            stderr=subprocess.PIPE
+            stderr=subprocess.PIPE,
+            #encoding='UTF-8' # we cannot set this option here as reading later will then flake with "can't concat NoneType to bytes"
+                              # see get_stderr() for additional details
             # since we are launching the command with shell=True we cannot use ps.terminate() / ps.kill().
             # This would just kill the executing shell, but not it's child and make the process an orphan.
             # therefore we use os.setsid here and later call os.getpgid(pid) to get process group that the shell
@@ -146,20 +170,6 @@ def start_profiling(self, containers=None):
     def stop_profiling(self):
         if self._ps is None:
             return
-        try:
-            print(f"Killing process with id: {self._ps.pid}")
-            ps_group_id = os.getpgid(self._ps.pid)
-            print(f" and process group {ps_group_id}")
-            os.killpg(ps_group_id, signal.SIGTERM)
-            try:
-                self._ps.wait(timeout=5)
-            except subprocess.TimeoutExpired:
-                # If the process hasn't gracefully exited after 5 seconds we kill it
-                os.killpg(ps_group_id, signal.SIGKILL)
-                print("Killed the process with SIGKILL. This could lead to corrupted metric log files!")
-
-        except ProcessLookupError:
-            print(f"Could not find process-group for {self._ps.pid}",
-                    file=sys.stderr)  # process/-group may have already closed
 
+        process_helpers.kill_pg(self._ps, self._metric_provider_executable)
         self._ps = None

metric_providers/cpu/energy/RAPL/MSR/component/provider.py

@@ -3,11 +3,12 @@
 from metric_providers.base import BaseMetricProvider
 
 class CpuEnergyRaplMsrComponentProvider(BaseMetricProvider):
-    def __init__(self, resolution):
+    def __init__(self, resolution, skip_check=False):
         super().__init__(
             metric_name='cpu_energy_rapl_msr_component',
             metrics={'time': int, 'value': int, 'package_id': str},
             resolution=resolution,
             unit='mJ',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check=skip_check,
         )

metric_providers/cpu/frequency/sysfs/core/provider.py

@@ -3,17 +3,20 @@
 from metric_providers.base import MetricProviderConfigurationError, BaseMetricProvider
 
 class CpuFrequencySysfsCoreProvider(BaseMetricProvider):
-    def __init__(self, resolution):
+    def __init__(self, resolution, skip_check=False):
         super().__init__(
             metric_name='cpu_frequency_sysfs_core',
             metrics={'time': int, 'value': int, 'core_id': int},
             resolution=0.001*resolution,
             unit='Hz',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
             metric_provider_executable='get-scaling-cur-freq.sh',
+            skip_check=skip_check,
         )
 
     def check_system(self):
+        super().check_system()
+
         file_path = "/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq"
         if os.path.exists(file_path):
             try:

metric_providers/cpu/time/cgroup/container/provider.py

@@ -3,12 +3,13 @@
 from metric_providers.base import BaseMetricProvider
 
 class CpuTimeCgroupContainerProvider(BaseMetricProvider):
-    def __init__(self, resolution, rootless=False):
+    def __init__(self, resolution, rootless=False, skip_check=False):
         super().__init__(
             metric_name='cpu_time_cgroup_container',
             metrics={'time': int, 'value': int, 'container_id': str},
             resolution=resolution,
             unit='us',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check=skip_check,
         )
         self._rootless = rootless

metric_providers/cpu/time/cgroup/system/provider.py

@@ -3,11 +3,12 @@
 from metric_providers.base import BaseMetricProvider
 
 class CpuTimeCgroupSystemProvider(BaseMetricProvider):
-    def __init__(self, resolution):
+    def __init__(self, resolution, skip_check=False):
         super().__init__(
             metric_name='cpu_time_cgroup_system',
             metrics={'time': int, 'value': int},
             resolution=resolution,
             unit='us',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check=skip_check,
         )

metric_providers/cpu/time/procfs/system/provider.py

@@ -3,11 +3,12 @@
 from metric_providers.base import BaseMetricProvider
 
 class CpuTimeProcfsSystemProvider(BaseMetricProvider):
-    def __init__(self, resolution):
+    def __init__(self, resolution, skip_check=False):
         super().__init__(
             metric_name='cpu_time_procfs_system',
             metrics={'time': int, 'value': int},
             resolution=resolution,
             unit='us',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check = skip_check
         )

metric_providers/cpu/utilization/cgroup/container/provider.py

@@ -3,12 +3,13 @@
 from metric_providers.base import BaseMetricProvider
 
 class CpuUtilizationCgroupContainerProvider(BaseMetricProvider):
-    def __init__(self, resolution, rootless=False):
+    def __init__(self, resolution, rootless=False, skip_check=False):
         super().__init__(
             metric_name='cpu_utilization_cgroup_container',
             metrics={'time': int, 'value': int, 'container_id': str},
             resolution=resolution,
             unit='Ratio',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check = skip_check,
         )
         self._rootless = rootless

metric_providers/cpu/utilization/procfs/system/provider.py

@@ -3,11 +3,12 @@
 from metric_providers.base import BaseMetricProvider
 
 class CpuUtilizationProcfsSystemProvider(BaseMetricProvider):
-    def __init__(self, resolution):
+    def __init__(self, resolution, skip_check=False):
         super().__init__(
             metric_name='cpu_utilization_procfs_system',
             metrics={'time': int, 'value': int},
             resolution=resolution,
             unit='Ratio',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check = skip_check,
         )

metric_providers/lm_sensors/abstract_provider.py

@@ -22,7 +22,7 @@ def _create_options(self):
         return ['-c'] + [f"'{i}'" for i in provider_config['chips']] \
             + ['-f'] + [f"'{i}'" for i in provider_config['features']]
 
-    def __init__(self, metric_name, resolution, unit):
+    def __init__(self, metric_name, resolution, unit, skip_check=False):
         if __name__ == '__main__':
             # If you run this on the command line you will need to set this in the config
             # This is separate so it is always clear what config is used.
@@ -35,5 +35,6 @@ def __init__(self, metric_name, resolution, unit):
             resolution=resolution,
             unit=unit,
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check=skip_check,
         )
         self._extra_switches = self._create_options()

metric_providers/memory/energy/RAPL/MSR/component/provider.py

@@ -4,12 +4,13 @@
 
 
 class MemoryEnergyRaplMsrComponentProvider(BaseMetricProvider):
-    def __init__(self, resolution):
+    def __init__(self, resolution, skip_check=False):
         super().__init__(
             metric_name='memory_energy_rapl_msr_component',
             metrics={'time': int, 'value': int, 'package_id': str},
             resolution=resolution,
             unit='mJ',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check=skip_check,
         )
         self._extra_switches = ['-d']

metric_providers/memory/total/cgroup/container/provider.py

@@ -3,12 +3,13 @@
 from metric_providers.base import BaseMetricProvider
 
 class MemoryTotalCgroupContainerProvider(BaseMetricProvider):
-    def __init__(self, resolution, rootless=False):
+    def __init__(self, resolution, rootless=False, skip_check=False):
         super().__init__(
             metric_name='memory_total_cgroup_container',
             metrics={'time': int, 'value': int, 'container_id': str},
             resolution=resolution,
             unit='Bytes',
             current_dir=os.path.dirname(os.path.abspath(__file__)),
+            skip_check=skip_check,
         )
         self._rootless = rootless