Address telemetry review comments

Avoid duplicate OliveRecipe telemetry from Docker workflows by suppressing inner workflow recipe events and forwarding CI detection into workflow containers. Keep CI telemetry ephemeral by skipping cache setup, and make recipe metadata stable by avoiding filesystem-sensitive model/resource classification.

Files changed:

- docs/Privacy.md

- olive/systems/docker/docker_system.py

- olive/telemetry/constants.py

- olive/telemetry/telemetry.py

- olive/workflows/run/run.py

- test/systems/docker/test_docker_system.py

- test/test_telemetry.py

- test/workflows/test_workflow_run.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, explicit target and host 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. 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 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/systems/docker/docker_system.py

@@ -5,6 +5,7 @@
 import copy
 import json
 import logging
+import os
 import sys
 import tempfile
 from pathlib import Path
@@ -19,6 +20,8 @@
 from olive.systems.common import AcceleratorConfig, SystemType
 from olive.systems.olive_system import OliveSystem
 from olive.systems.system_config import LocalTargetUserConfig, SystemConfig
+from olive.telemetry.constants import SUPPRESS_WORKFLOW_TELEMETRY_ENV
+from olive.telemetry.telemetry import is_ci_environment
 from olive.workflows.run.config import RunConfig
 
 if TYPE_CHECKING:
@@ -241,6 +244,9 @@ def _prepare_environment(self, base_env) -> dict:
         # Add default environment variables
         environment.setdefault("PYTHONPYCACHEPREFIX", "/tmp")
         environment["OLIVE_LOG_LEVEL"] = logging.getLevelName(logger.getEffectiveLevel())
+        environment[SUPPRESS_WORKFLOW_TELEMETRY_ENV] = "1"
+        if is_ci_environment():
+            environment["CI"] = "1"
 
         # Add HuggingFace token if needed
         if self.hf_token:
@@ -303,8 +309,6 @@ def _create_runner_script_mount(self) -> tuple[str, str]:
     @staticmethod
     def _get_huggingface_token() -> Optional[str]:
         """Get HuggingFace token from environment or file."""
-        import os
-
         # Check environment variable
         token = os.getenv("HF_TOKEN")
         if token:

olive/telemetry/constants.py

@@ -3,6 +3,7 @@
 # Licensed under the MIT License.
 # --------------------------------------------------------------------------
 
-"""OneCollector connection string."""
+"""Telemetry constants."""
 
 CONNECTION_STRING = "SW5zdHJ1bWVudGF0aW9uS2V5PTYyMTUwOTExZGMwMDRmYzliYjY3YmE5NjA2NDI3ZTU2LWVjNjFmOWFmLTVkN2EtNGQxOS1hZjMxLWI5Y2Q2OWU5ODdmMS02OTE1"
+SUPPRESS_WORKFLOW_TELEMETRY_ENV = "OLIVE_SUPPRESS_WORKFLOW_TELEMETRY"

olive/telemetry/telemetry.py

@@ -473,11 +473,11 @@ def __init__(self):
             self._logger = self._create_logger()
             event_source.disable()
 
-            self._cache_handler = TelemetryCacheHandler(self)
-            self._setup_payload_callbacks()
             is_ci = self._is_ci_environment()
             self._recipe_only_ci_telemetry = is_ci
             if not is_ci:
+                self._cache_handler = TelemetryCacheHandler(self)
+                self._setup_payload_callbacks()
                 self._log_heartbeat()
             if os.environ.get("OLIVE_DISABLE_TELEMETRY") == "1":
                 self.disable_telemetry()
@@ -500,7 +500,7 @@ def _create_logger(self) -> Optional[TelemetryLogger]:
     def _setup_payload_callbacks(self) -> None:
         # Register callback for payload transmission events
         # No need to store unregister function - logger shutdown will clean up callbacks
-        if self._logger is None:
+        if self._logger is None or self._cache_handler is None:
             return
         self._logger.register_payload_transmitted_callback(
             self._cache_handler.on_payload_transmitted,

olive/workflows/run/run.py

@@ -6,19 +6,20 @@
 import importlib
 import json
 import logging
+import os
 from copy import deepcopy
 from os import PathLike
-from pathlib import Path
+from pathlib import Path, PurePosixPath, PureWindowsPath
 from typing import TYPE_CHECKING, Any, Optional, Union
 
 from olive.common.config_utils import load_config_file
 from olive.common.utils import hash_dict, set_tempdir
 from olive.hardware.constants import ExecutionProvider
 from olive.logging import set_default_logger_severity, set_ort_logger_severity, set_verbosity_info
 from olive.package_config import OlivePackageConfig
-from olive.resource_path import create_resource_path, find_all_resources
 from olive.systems.accelerator_creator import create_accelerator
 from olive.systems.common import SystemType
+from olive.telemetry.constants import SUPPRESS_WORKFLOW_TELEMETRY_ENV
 from olive.telemetry.telemetry import is_ci_environment
 from olive.telemetry.telemetry_extensions import log_recipe_result
 from olive.workflows.run.config import RunConfig
@@ -227,7 +228,7 @@ def run(
 
         if parsed_run_config.engine.host and parsed_run_config.engine.host.type == SystemType.Docker:
             docker_system = parsed_run_config.engine.host.create_system()
-            workflow_output = docker_system.run_workflow(parsed_run_config)
+            workflow_output = docker_system.run_workflow(deepcopy(parsed_run_config))
             success = True
             return workflow_output
 
@@ -240,17 +241,18 @@ def run(
         exception_type = type(exc).__name__
         raise
     finally:
-        metadata = _build_recipe_result_metadata(
-            run_config,
-            run_config_telemetry_input,
-            parsed_run_config,
-            recipe_telemetry_metadata,
-            list_required_packages=list_required_packages,
-            package_config_input=package_config_telemetry_input,
-            package_config_provided=package_config_provided,
-        )
-        recipe_name = metadata.pop("recipe_name")
-        log_recipe_result(recipe_name, success=success, metadata=metadata, exception_type=exception_type)
+        if os.environ.get(SUPPRESS_WORKFLOW_TELEMETRY_ENV) != "1":
+            metadata = _build_recipe_result_metadata(
+                run_config,
+                run_config_telemetry_input,
+                parsed_run_config,
+                recipe_telemetry_metadata,
+                list_required_packages=list_required_packages,
+                package_config_input=package_config_telemetry_input,
+                package_config_provided=package_config_provided,
+            )
+            recipe_name = metadata.pop("recipe_name")
+            log_recipe_result(recipe_name, success=success, metadata=metadata, exception_type=exception_type)
 
 
 def generate_files_from_packages(packages, file_name):
@@ -510,12 +512,20 @@ def _classify_input_model_source(model_identifier: Any) -> str:
     if identifier.startswith(("http://", "https://")):
         return "url"
 
-    resource_path = create_resource_path(identifier)
-    if resource_path.is_local_resource():
-        return "local_file" if resource_path.type.value == "file" else "local_folder"
+    if _is_explicit_local_model_path(identifier):
+        suffix = PureWindowsPath(identifier).suffix or PurePosixPath(identifier).suffix
+        return "local_file" if suffix else "local_folder"
     return "string_name"
 
 
+def _is_explicit_local_model_path(identifier: str) -> bool:
+    return (
+        identifier.startswith(("./", "../", ".\\", "..\\", "~/", "~\\", "/", "\\\\"))
+        or PureWindowsPath(identifier).is_absolute()
+        or PurePosixPath(identifier).is_absolute()
+    )
+
+
 def _extract_target_metadata(run_config: RunConfig) -> dict[str, Optional[str]]:
     target_system = run_config.engine.target
     return _extract_system_metadata(target_system, "target")
@@ -562,13 +572,11 @@ def _set_metadata_if_present(metadata: dict[str, Any], values: dict[str, Optiona
 def _build_recipe_hash(run_config_json: dict[str, Any]) -> str:
     sanitized = deepcopy(run_config_json)
     _redact_recipe_hash_keys(sanitized)
-    for path in find_all_resources(sanitized):
-        _set_path_value(sanitized, path, RECIPE_HASH_REDACTED_VALUE)
     return hash_dict(sanitized)[:16]
 
 
 def _redact_recipe_hash_keys(value: Any, key: Optional[str] = None) -> Any:
-    if key in RECIPE_HASH_REDACTED_KEYS:
+    if key in RECIPE_HASH_REDACTED_KEYS or _is_path_like_key(key):
         return RECIPE_HASH_REDACTED_VALUE
     if isinstance(value, dict):
         for child_key in list(value):
@@ -577,10 +585,3 @@ def _redact_recipe_hash_keys(value: Any, key: Optional[str] = None) -> Any:
         for index, item in enumerate(value):
             value[index] = _redact_recipe_hash_keys(item, key)
     return value
-
-
-def _set_path_value(container: Any, path: tuple[Any, ...], value: Any) -> None:
-    current = container
-    for key in path[:-1]:
-        current = current[key]
-    current[path[-1]] = value

test/systems/docker/test_docker_system.py

@@ -8,6 +8,7 @@
 
 from olive.systems.docker.docker_system import DockerSystem
 from olive.systems.system_config import DockerTargetUserConfig
+from olive.telemetry.constants import SUPPRESS_WORKFLOW_TELEMETRY_ENV
 from test.utils import ONNX_MODEL_PATH
 
 # pylint: disable=attribute-defined-outside-init,protected-access
@@ -136,10 +137,30 @@ def test_run_workflow(self, mock_find_resources, mock_tempdir, mock_from_env, tm
         command = mock_docker_client.containers.run.call_args[1]["command"]
         assert "workflow_runner.py" in command
         assert "--config" in command
+        assert mock_docker_client.containers.run.call_args.kwargs["environment"][SUPPRESS_WORKFLOW_TELEMETRY_ENV] == "1"
 
         # Verify cleanup
         mock_container.remove.assert_called_once()
 
+    @patch("olive.systems.docker.docker_system.docker.from_env")
+    def test_prepare_environment_forwards_ci_to_workflow_container(self, mock_from_env, monkeypatch):
+        mock_docker_client = MagicMock()
+        mock_from_env.return_value = mock_docker_client
+        mock_docker_client.images.get.return_value = MagicMock()
+        monkeypatch.setenv("TF_BUILD", "True")
+        docker_config = self.get_default_docker_config()
+        docker_system = DockerSystem(
+            image_name=docker_config.image_name,
+            build_context_path=docker_config.build_context_path,
+            dockerfile=docker_config.dockerfile,
+            work_dir=docker_config.work_dir,
+        )
+
+        environment = docker_system._prepare_environment({})
+
+        assert environment["CI"] == "1"
+        assert environment[SUPPRESS_WORKFLOW_TELEMETRY_ENV] == "1"
+
     @patch("olive.systems.docker.docker_system.docker.from_env")
     @patch("olive.systems.docker.docker_system.tempfile.TemporaryDirectory")
     @patch("olive.systems.docker.docker_system.find_all_resources")

test/test_telemetry.py

@@ -55,6 +55,8 @@ def test_telemetry_only_logs_recipe_events_in_ci(monkeypatch):
 
         assert mock_logger.log.call_count == 1
         assert mock_logger.log.call_args.args[0] == RECIPE_EVENT_NAME
+        assert telemetry._cache_handler is None
+        mock_logger.register_payload_transmitted_callback.assert_not_called()
     finally:
         Telemetry._instance = None
 

test/workflows/test_workflow_run.py

@@ -2,12 +2,13 @@
 import sys
 from copy import deepcopy
 from pathlib import Path
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 
 import pytest
 
+from olive.telemetry.constants import SUPPRESS_WORKFLOW_TELEMETRY_ENV
 from olive.workflows import run as olive_run
-from olive.workflows.run.run import _classify_run_config_source
+from olive.workflows.run.run import _build_recipe_hash, _classify_input_model_source, _classify_run_config_source
 from test.utils import (
     get_pytorch_model,
     get_pytorch_model_config,
@@ -229,6 +230,66 @@ def test_run_logs_recipe_result_failure(mock_run_engine, mock_log_recipe_result)
     assert mock_log_recipe_result.call_args.kwargs["exception_type"] == "ValueError"
 
 
+@patch("olive.workflows.run.run.log_recipe_result")
+@patch("olive.workflows.run.run.run_engine")
+def test_run_skips_recipe_result_when_workflow_telemetry_is_suppressed(
+    mock_run_engine, mock_log_recipe_result, monkeypatch
+):
+    monkeypatch.setenv(SUPPRESS_WORKFLOW_TELEMETRY_ENV, "1")
+    expected_output = object()
+    mock_run_engine.return_value = expected_output
+
+    output = olive_run(
+        {
+            "input_model": {
+                "type": "HfModel",
+                "model_path": "Qwen/Qwen2.5-0.5B-Instruct",
+                "task": "text-generation",
+            }
+        }
+    )
+
+    assert output is expected_output
+    mock_log_recipe_result.assert_not_called()
+
+
+@patch("olive.workflows.run.run.log_recipe_result")
+@patch("olive.systems.system_config.SystemConfig.create_system")
+def test_run_logs_single_parent_recipe_result_for_docker_host(mock_create_system, mock_log_recipe_result):
+    expected_output = object()
+    docker_system = Mock()
+
+    def run_workflow(container_run_config):
+        container_run_config.engine.host = container_run_config.engine.target
+        return expected_output
+
+    docker_system.run_workflow.side_effect = run_workflow
+    mock_create_system.return_value = docker_system
+    config = {
+        "input_model": {"type": "ONNXModel", "model_path": "model.onnx"},
+        "systems": {
+            "docker_system": {
+                "type": "Docker",
+                "config": {
+                    "dockerfile": "Dockerfile",
+                    "build_context_path": "build_context",
+                    "image_name": "test-image:latest",
+                    "work_dir": "/olive-ws",
+                },
+            },
+            "local_system": {"type": "LocalSystem"},
+        },
+        "engine": {"host": "docker_system", "target": "local_system"},
+    }
+
+    output = olive_run(config)
+
+    assert output is expected_output
+    mock_log_recipe_result.assert_called_once()
+    metadata = mock_log_recipe_result.call_args.kwargs["metadata"]
+    assert metadata["host_system_type"] == "Docker"
+
+
 @patch("olive.workflows.run.run.log_recipe_result")
 @patch("olive.workflows.run.run.run_engine")
 def test_run_logs_recipe_host_metadata_without_explicit_target(mock_run_engine, mock_log_recipe_result):
@@ -312,3 +373,26 @@ def test_run_logs_package_config_overrides_when_package_config_provided(mock_run
 
 def test_classify_run_config_source_handles_non_pathlike_object():
     assert _classify_run_config_source(object()) == ("config_object", "object")
+
+
+def test_classify_input_model_source_does_not_depend_on_local_filesystem(tmp_path, monkeypatch):
+    assert _classify_input_model_source("Qwen/Qwen2.5-0.5B-Instruct") == "string_name"
+
+    monkeypatch.chdir(tmp_path)
+    (tmp_path / "bert-base-uncased").mkdir()
+
+    assert _classify_input_model_source("bert-base-uncased") == "string_name"
+    assert _classify_input_model_source("./model.onnx") == "local_file"
+
+
+def test_recipe_hash_does_not_depend_on_local_model_path_presence(tmp_path, monkeypatch):
+    config = {
+        "input_model": {"type": "HfModel", "config": {"model_path": "bert-base-uncased"}},
+        "engine": {"output_dir": "output"},
+    }
+    recipe_hash = _build_recipe_hash(config)
+
+    monkeypatch.chdir(tmp_path)
+    (tmp_path / "bert-base-uncased").mkdir()
+
+    assert _build_recipe_hash(config) == recipe_hash