[Monitor OpenTelemetry Exporter] Add folder permissions for local files (#41384)

* WIP

* Update

* Update changelog

* Update sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py

Co-authored-by: Copilot <[email protected]>

* Update

* Lint

* Update

* Log exception

* Update

* Lint

* Test

* test

* Test

* Remove env clear in tests

* Update

---------

Co-authored-by: Copilot <[email protected]>

Author: hectorhdzg

sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md

@@ -36,6 +36,8 @@
 
 - Extend version range for `psutil` to include 7.x
   ([#40459](https://github.com/Azure/azure-sdk-for-python/pull/40459))
+- Add folder permissions for local files
+  ([#41384](https://github.com/Azure/azure-sdk-for-python/pull/41384))
 
 ## 1.0.0b36 (2025-04-07)
 

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_storage.py

@@ -6,11 +6,15 @@
 import logging
 import os
 import random
+import subprocess
 
 from azure.monitor.opentelemetry.exporter._utils import PeriodicTask
 
 logger = logging.getLogger(__name__)
 
+ICACLS_PATH = os.path.join(
+    os.environ.get("SYSTEMDRIVE", "C:"), r"\Windows\System32\icacls.exe"
+)
 
 def _fmt(timestamp):
     return timestamp.strftime("%Y-%m-%dT%H%M%S.%f")
@@ -92,19 +96,25 @@ def __init__(
         self._max_size = max_size
         self._retention_period = retention_period
         self._write_timeout = write_timeout
-        self._maintenance_routine()
-        self._maintenance_task = PeriodicTask(
-            interval=maintenance_period,
-            function=self._maintenance_routine,
-            name=name,
-        )
-        self._lease_period = lease_period
-        self._maintenance_task.daemon = True
-        self._maintenance_task.start()
+
+        self._enabled = self._check_and_set_folder_permissions()
+        if self._enabled:
+            self._maintenance_routine()
+            self._maintenance_task = PeriodicTask(
+                interval=maintenance_period,
+                function=self._maintenance_routine,
+                name=name,
+            )
+            self._lease_period = lease_period
+            self._maintenance_task.daemon = True
+            self._maintenance_task.start()
+        else:
+            logger.error("Could not set secure permissions on storage folder, local storage is disabled.")
 
     def close(self):
-        self._maintenance_task.cancel()
-        self._maintenance_task.join()
+        if self._enabled:
+            self._maintenance_task.cancel()
+            self._maintenance_task.join()
 
     def __enter__(self):
         return self
@@ -121,43 +131,49 @@ def _maintenance_routine(self):
         except Exception:
             pass  # keep silent
 
+    # pylint: disable=too-many-nested-blocks
     def gets(self):
-        now = _now()
-        lease_deadline = _fmt(now)
-        retention_deadline = _fmt(now - _seconds(self._retention_period))
-        timeout_deadline = _fmt(now - _seconds(self._write_timeout))
-        try:
-            for name in sorted(os.listdir(self._path)):
-                path = os.path.join(self._path, name)
-                if not os.path.isfile(path):
-                    continue  # skip if not a file
-                if path.endswith(".tmp"):
-                    if name < timeout_deadline:
+        if self._enabled:
+            now = _now()
+            lease_deadline = _fmt(now)
+            retention_deadline = _fmt(now - _seconds(self._retention_period))
+            timeout_deadline = _fmt(now - _seconds(self._write_timeout))
+            try:
+                for name in sorted(os.listdir(self._path)):
+                    path = os.path.join(self._path, name)
+                    if not os.path.isfile(path):
+                        continue  # skip if not a file
+                    if path.endswith(".tmp"):
+                        if name < timeout_deadline:
+                            try:
+                                os.remove(path)  # TODO: log data loss
+                            except Exception:
+                                pass  # keep silent
+                    if path.endswith(".lock"):
+                        if path[path.rindex("@") + 1 : -5] > lease_deadline:
+                            continue  # under lease
+                        new_path = path[: path.rindex("@")]
                         try:
-                            os.remove(path)  # TODO: log data loss
+                            os.rename(path, new_path)
                         except Exception:
                             pass  # keep silent
-                if path.endswith(".lock"):
-                    if path[path.rindex("@") + 1 : -5] > lease_deadline:
-                        continue  # under lease
-                    new_path = path[: path.rindex("@")]
-                    try:
-                        os.rename(path, new_path)
-                    except Exception:
-                        pass  # keep silent
-                    path = new_path
-                if path.endswith(".blob"):
-                    if name < retention_deadline:
-                        try:
-                            os.remove(path)  # TODO: log data loss
-                        except Exception:
-                            pass  # keep silent
-                    else:
-                        yield LocalFileBlob(path)
-        except Exception:
-            pass  # keep silent
+                        path = new_path
+                    if path.endswith(".blob"):
+                        if name < retention_deadline:
+                            try:
+                                os.remove(path)  # TODO: log data loss
+                            except Exception:
+                                pass  # keep silent
+                        else:
+                            yield LocalFileBlob(path)
+            except Exception:
+                pass  # keep silent
+        else:
+            pass
 
     def get(self):
+        if not self._enabled:
+            return None
         cursor = self.gets()
         try:
             return next(cursor)
@@ -166,12 +182,8 @@ def get(self):
         return None
 
     def put(self, data, lease_period=None):
-        # Create path if it doesn't exist
-        try:
-            if not os.path.isdir(self._path):
-                os.makedirs(self._path, exist_ok=True)
-        except Exception:
-            pass  # keep silent
+        if not self._enabled:
+            return None
         if not self._check_storage_size():
             return None
         blob = LocalFileBlob(
@@ -187,6 +199,44 @@ def put(self, data, lease_period=None):
             lease_period = self._lease_period
         return blob.put(data, lease_period=lease_period)
 
+    def _check_and_set_folder_permissions(self):
+        """
+        Validate and set folder permissions where the telemetry data will be stored.
+        :return: True if folder was created and permissions set successfully, False otherwise.
+        :rtype: bool
+        """
+        try:
+            # Create path if it doesn't exist
+            os.makedirs(self._path, exist_ok=True)
+            # Windows
+            if os.name == "nt":
+                user = self._get_current_user()
+                if not user:
+                    logger.warning(
+                        "Failed to retrieve current user. Skipping folder permission setup."
+                    )
+                    return False
+                result = subprocess.run(
+                    [
+                        ICACLS_PATH,
+                        self._path,
+                        "/grant",
+                        "*S-1-5-32-544:(OI)(CI)F",  # Full permission for Administrators
+                        f"{user}:(OI)(CI)F",
+                        "/inheritance:r",
+                    ],
+                    check=False,
+                )
+                if result.returncode == 0:
+                    return True
+            # Unix
+            else:
+                os.chmod(self._path, 0o700)
+                return True
+        except Exception:
+            pass  # keep silent
+        return False
+
     def _check_storage_size(self):
         size = 0
         # pylint: disable=unused-variable
@@ -209,7 +259,19 @@ def _check_storage_size(self):
                             "Persistent storage max capacity has been "
                             "reached. Currently at {}KB. Telemetry will be "
                             "lost. Please consider increasing the value of "
-                            "'storage_max_size' in exporter config.".format(str(size / 1024))
+                            "'storage_max_size' in exporter config.".format(
+                                str(size / 1024)
+                            )
                         )
                         return False
         return True
+
+    def _get_current_user(self):
+        user = ""
+        domain = os.environ.get("USERDOMAIN")
+        username = os.environ.get("USERNAME")
+        if domain and username:
+            user = f"{domain}\\{username}"
+        else:
+            user = os.getlogin()
+        return user

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/logs/test_logs.py

@@ -57,7 +57,8 @@ class TestAzureLogExporter(unittest.TestCase):
 
     @classmethod
     def setUpClass(cls):
-        os.environ.clear()
+        os.environ.pop("APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL", None)
+        os.environ.pop("APPINSIGHTS_INSTRUMENTATIONKEY", None)
         os.environ["APPINSIGHTS_INSTRUMENTATIONKEY"] = "1234abcd-5678-4efa-8abc-1234567890ab"
         os.environ["APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL"] = "true"
         cls._exporter = cls._exporter_class()

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/metrics/test_metrics.py

@@ -47,7 +47,8 @@ def func(*_args, **_kwargs):
 class TestAzureMetricExporter(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
-        os.environ.clear()
+        os.environ.pop("APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL", None)
+        os.environ.pop("APPINSIGHTS_INSTRUMENTATIONKEY", None)
         os.environ["APPINSIGHTS_INSTRUMENTATIONKEY"] = "1234abcd-5678-4efa-8abc-1234567890ab"
         os.environ["APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL"] = "true"
         cls._exporter = AzureMonitorMetricExporter()

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/statsbeat/test_exporter.py

@@ -47,7 +47,8 @@ def clean_folder(folder):
 class TestStatsbeatExporter(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
-        os.environ.clear()
+        os.environ.pop("APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL", None)
+        os.environ.pop("APPINSIGHTS_INSTRUMENTATIONKEY", None)
         os.environ["APPINSIGHTS_INSTRUMENTATIONKEY"] = "1234abcd-5678-4efa-8abc-1234567890ab"
         os.environ["APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL"] = "false"
         cls._exporter = _StatsBeatExporter(

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/statsbeat/test_statsbeat.py

@@ -267,7 +267,13 @@ def test_shutdown_statsbeat_metrics(self):
 class TestStatsbeatMetrics(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
-        os.environ.clear()
+        os.environ.pop("WEBSITE_SITE_NAME", None)
+        os.environ.pop("WEBSITE_HOME_STAMPNAME", None)
+        os.environ.pop("FUNCTIONS_WORKER_RUNTIME", None)
+        os.environ.pop("WEBSITE_HOSTNAME", None)
+        os.environ.pop("AKS_ARM_NAMESPACE_ID", None)
+        os.environ.pop("KUBERNETES_SERVICE_HOST", None)
+
         mp = MeterProvider()
         ikey = "1aa11111-bbbb-1ccc-8ddd-eeeeffff3334"
         endpoint = "https://westus-1.in.applicationinsights.azure.com/"

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_base_exporter.py

@@ -75,7 +75,8 @@ class TestBaseExporter(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         # Clear environ so the mocks from past tests do not interfere.
-        os.environ.clear()
+        os.environ.pop("APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL", None)
+        os.environ.pop("APPINSIGHTS_INSTRUMENTATIONKEY", None)
         os.environ["APPINSIGHTS_INSTRUMENTATIONKEY"] = "1234abcd-5678-4efa-8abc-1234567890ab"
         os.environ["APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL"] = "true"
         cls._base = BaseExporter()

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_connection_string_parser.py

@@ -10,7 +10,8 @@
 # pylint: disable=too-many-public-methods
 class TestConnectionStringParser(unittest.TestCase):
     def setUp(self):
-        os.environ.clear()
+        os.environ.pop("APPLICATIONINSIGHTS_CONNECTION_STRING", None)
+        os.environ.pop("APPINSIGHTS_INSTRUMENTATIONKEY", None)
         self._valid_connection_string = "InstrumentationKey=1234abcd-5678-4efa-8abc-1234567890ab"
         self._valid_instrumentation_key = "1234abcd-5678-4efa-8abc-1234567890ab"
 

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_storage.py

@@ -79,9 +79,6 @@ def test_lease_error(self):
 
 # pylint: disable=protected-access
 class TestLocalFileStorage(unittest.TestCase):
-    @classmethod
-    def setup_class(cls):
-        os.makedirs(TEST_FOLDER, exist_ok=True)
 
     @classmethod
     def tearDownClass(cls):

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_utils.py

@@ -33,7 +33,6 @@
 
 class TestUtils(unittest.TestCase):
     def setUp(self):
-        os.environ.clear()
         self._valid_instrumentation_key = "1234abcd-5678-4efa-8abc-1234567890ab"
 
     def test_nanoseconds_to_duration(self):