Address simple telemetry review comments

Tighten singleton locking, simplify CI/cache helpers, clarify per-process telemetry behavior, and update privacy wording for the default host metadata.

Files changed:
- docs/Privacy.md
- olive/telemetry/telemetry.py
- olive/telemetry/library/telemetry_logger.py

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: bmehta001

docs/Privacy.md

@@ -13,4 +13,4 @@ In addition, Olive may collect additional telemetry data such as:
 - Performance data
 - Exception information
 
-Collection of this additional telemetry can be disabled by adding the `--disable_telemetry` flag to any Olive CLI command, or by setting the `OLIVE_DISABLE_TELEMETRY` environment variable to `1` before running. In CI/CD environments (e.g., GitHub Actions, Azure Pipelines, Jenkins), Olive suppresses the general heartbeat/action/error events and only emits the `OliveRecipe` event. The `OliveRecipe` event may include recipe metadata such as pass types, explicitly configured target settings, the host system type and any explicitly configured host accelerator settings, whether a custom package config was provided, a redacted snapshot of custom package-config overrides, and a redacted snapshot of explicitly supplied config overrides. Outside CI/CD environments, if telemetry is enabled but cannot be sent to Microsoft, it will be stored locally and sent when a connection is available. You can override the default cache location by setting the `OLIVE_TELEMETRY_CACHE_DIR` environment variable to a valid directory path.
+Collection of this additional telemetry can be disabled by adding the `--disable_telemetry` flag to any Olive CLI command, or by setting the `OLIVE_DISABLE_TELEMETRY` environment variable to `1` before running. In CI/CD environments (e.g., GitHub Actions, Azure Pipelines, Jenkins), Olive suppresses the general heartbeat/action/error events and only emits the `OliveRecipe` event. The `OliveRecipe` event may include recipe metadata such as pass types, explicitly configured target settings, the host system type (including the default `LocalSystem` host) and any explicitly configured host accelerator settings, whether a custom package config was provided, a redacted snapshot of custom package-config overrides, and a redacted snapshot of explicitly supplied config overrides. Outside CI/CD environments, if telemetry is enabled but cannot be sent to Microsoft, it will be stored locally and sent when a connection is available. You can override the default cache location by setting the `OLIVE_TELEMETRY_CACHE_DIR` environment variable to a valid directory path.

olive/telemetry/library/telemetry_logger.py

@@ -29,7 +29,8 @@ class TelemetryLogger:
 
     _instance: Optional["TelemetryLogger"] = None
     _default_logger: Optional["TelemetryLogger"] = None
-    _singleton_lock = threading.RLock()
+    _instance_lock = threading.RLock()
+    _default_logger_lock = threading.RLock()
     _logger: Optional[logging.Logger] = None
     _logger_exporter: Optional[OneCollectorLogExporter] = None
     _logger_provider: Optional[LoggerProvider] = None
@@ -41,10 +42,11 @@ def __new__(cls, options: Optional[OneCollectorExporterOptions] = None):
             options: Exporter options (only used on first instantiation)
 
         """
-        with cls._singleton_lock:
-            if cls._instance is None:
-                cls._instance = super().__new__(cls)
-                cls._instance._initialize(options)
+        if cls._instance is None:
+            with cls._instance_lock:
+                if cls._instance is None:
+                    cls._instance = super().__new__(cls)
+                    cls._instance._initialize(options)
 
         return cls._instance
 
@@ -160,23 +162,25 @@ def get_default_logger(
             TelemetryLogger instance
 
         """
-        with cls._singleton_lock:
-            if cls._default_logger is None:
-                options = None
-                if connection_string:
-                    options = OneCollectorExporterOptions(
-                        connection_string=connection_string, service_name=service_name
-                    )
-                cls._default_logger = cls(options=options)
+        if cls._default_logger is None:
+            with cls._default_logger_lock:
+                if cls._default_logger is None:
+                    options = None
+                    if connection_string:
+                        options = OneCollectorExporterOptions(
+                            connection_string=connection_string, service_name=service_name
+                        )
+                    cls._default_logger = cls(options=options)
 
         return cls._default_logger
 
     @classmethod
     def shutdown_default_logger(cls) -> None:
         """Shutdown the default telemetry logger."""
-        if cls._default_logger:
-            cls._default_logger.shutdown()
-            cls._default_logger = None
+        with cls._default_logger_lock:
+            if cls._default_logger:
+                cls._default_logger.shutdown()
+                cls._default_logger = None
 
 
 def get_telemetry_logger(

olive/telemetry/telemetry.py

@@ -219,9 +219,11 @@ def on_payload_transmitted(self, args: "PayloadTransmittedCallbackArgs") -> None
         finally:
             with self._condition:
                 self._callbacks_item_count += args.item_count
+                # Wake threads waiting for flush/shutdown callback accounting.
                 self._condition.notify_all()
 
     def wait_for_callbacks(self, timeout_sec: float, during_flush: bool = False) -> bool:
+        """Wait until callbacks have caught up with logged telemetry items."""
         deadline = time.time() + timeout_sec
         with self._condition:
             while True:
@@ -278,11 +280,9 @@ def cache_path(self) -> Optional[Path]:
 
         """
         telemetry_cache_dir = None
-        telemetry_cache_dir_override = os.environ.get("OLIVE_TELEMETRY_CACHE_DIR")
+        telemetry_cache_dir_override = (os.environ.get("OLIVE_TELEMETRY_CACHE_DIR") or "").strip()
         if telemetry_cache_dir_override:
-            telemetry_cache_dir_override = telemetry_cache_dir_override.strip()
-            if telemetry_cache_dir_override:
-                telemetry_cache_dir = Path(telemetry_cache_dir_override).expanduser()
+            telemetry_cache_dir = Path(telemetry_cache_dir_override).expanduser()
         if not telemetry_cache_dir:
             telemetry_cache_dir = get_telemetry_base_dir() / "cache"
         return telemetry_cache_dir / self._cache_file_name
@@ -442,7 +442,9 @@ def is_flushing(self) -> bool:
 class Telemetry:
     """Wrapper that wires environment configuration into the library logger.
 
-    This is a singleton class - all instances share the same state.
+    This is a per-process singleton class - all instances in a process share the same state.
+    Separate processes get separate in-memory singleton instances and coordinate only through
+    the shared telemetry cache file lock.
     Use Telemetry() to get the singleton instance.
     """
 
@@ -451,11 +453,12 @@ class Telemetry:
 
     def __new__(cls):
         """Create or return the singleton instance."""
-        with cls._lock:
-            if cls._instance is None:
-                instance = super().__new__(cls)
-                instance._initialized = False
-                cls._instance = instance
+        if cls._instance is None:
+            with cls._lock:
+                if cls._instance is None:
+                    instance = super().__new__(cls)
+                    instance._initialized = False
+                    cls._instance = instance
         return cls._instance
 
     def __init__(self):
@@ -472,7 +475,7 @@ def __init__(self):
             self._logger = self._create_logger()
             event_source.disable()
 
-            is_ci = self._is_ci_environment()
+            is_ci = is_ci_environment()
             self._recipe_only_ci_telemetry = is_ci
             if not is_ci:
                 self._cache_handler = TelemetryCacheHandler(self)
@@ -485,11 +488,6 @@ def __init__(self):
             # Fail silently — telemetry must never crash the host application
             self._initialized = True
 
-    @staticmethod
-    def _is_ci_environment() -> bool:
-        """Detect CI/CD environments by checking well-known environment variables."""
-        return is_ci_environment()
-
     def _create_logger(self) -> Optional[TelemetryLogger]:
         try:
             return get_telemetry_logger(base64.b64decode(CONNECTION_STRING).decode(), service_name=APP_NAME)