mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix ENABLE_SENSITIVE_DATA env var ignored when set after module import (#4743)
* Python: Re-read env vars in configure_otel_providers and enable_instrumentation (#4119) Fix ENABLE_SENSITIVE_DATA and VS_CODE_EXTENSION_PORT env vars being ignored when load_dotenv() runs after module import. The module-level OBSERVABILITY_SETTINGS singleton cached env state at import time, and configure_otel_providers() / enable_instrumentation() never re-read from os.environ when parameters were None. Both functions now construct a fresh ObservabilitySettings() to pick up current env vars when explicit parameters are not provided, matching the existing behavior of the env_file_path branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review feedback for #4119: avoid throwaway ObservabilitySettings - Add _read_bool_env/_read_int_env helpers to read env vars without constructing a full ObservabilitySettings (which calls create_resource()) - Replace ObservabilitySettings() in enable_instrumentation() and configure_otel_providers() else-branch with direct env reads - Add enable_console_exporters parameter to configure_otel_providers() for override parity with enable_sensitive_data and vs_code_extension_port - Propagate _resource and _executed_setup in the non-env_file_path branch - Make existing tests hermetic (clear VS_CODE_EXTENSION_PORT and ENABLE_CONSOLE_EXPORTERS env vars) - Add tests: enable_console_exporters env refresh, explicit param overrides for both enable_instrumentation() and configure_otel_providers() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address remaining review feedback for #4119 - Refresh enable_console_exporters in enable_instrumentation() for consistency with configure_otel_providers(), so env var changes after import are picked up by both public API functions - Make test_configure_otel_providers_reads_env_vs_code_port hermetic by clearing ENABLE_CONSOLE_EXPORTERS from the environment - Add test_enable_instrumentation_reads_env_console_exporters to cover the new refresh behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove unconditional enable_console_exporters overwrite from enable_instrumentation() (#4119) enable_instrumentation() is documented as not configuring exporters, so managing enable_console_exporters there was a leaky abstraction. The unconditional _read_bool_env call silently reset the value to False when ENABLE_CONSOLE_EXPORTERS was absent from env, clobbering any value previously set by configure_otel_providers(enable_console_exporters=True). - Remove the unconditional overwrite line from enable_instrumentation() - Replace test_enable_instrumentation_reads_env_console_exporters with test_enable_instrumentation_does_not_touch_console_exporters - Add regression test: enable_instrumentation() does not clobber a previously configured enable_console_exporters value - Add test: explicit enable_sensitive_data param still leaves enable_console_exporters untouched Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Eduard van Valkenburg
committed by
GitHub
Unverified
GitHub
parent
c2fec6b51c
commit
acaf6b7054
@@ -899,6 +899,25 @@ def get_meter(
|
||||
OBSERVABILITY_SETTINGS: ObservabilitySettings = ObservabilitySettings()
|
||||
|
||||
|
||||
def _read_bool_env(name: str, *, default: bool = False) -> bool:
|
||||
"""Read a boolean from an environment variable."""
|
||||
value = os.getenv(name)
|
||||
if value is None:
|
||||
return default
|
||||
return value.lower() in ("true", "1", "yes", "on")
|
||||
|
||||
|
||||
def _read_int_env(name: str, *, default: int | None = None) -> int | None:
|
||||
"""Read an optional integer from an environment variable."""
|
||||
value = os.getenv(name)
|
||||
if value is None:
|
||||
return default
|
||||
try:
|
||||
return int(value)
|
||||
except ValueError:
|
||||
return default
|
||||
|
||||
|
||||
def enable_instrumentation(
|
||||
*,
|
||||
enable_sensitive_data: bool | None = None,
|
||||
@@ -920,11 +939,15 @@ def enable_instrumentation(
|
||||
OBSERVABILITY_SETTINGS.enable_instrumentation = True
|
||||
if enable_sensitive_data is not None:
|
||||
OBSERVABILITY_SETTINGS.enable_sensitive_data = enable_sensitive_data
|
||||
else:
|
||||
# Re-read from current environment in case env vars were set after import (e.g. load_dotenv())
|
||||
OBSERVABILITY_SETTINGS.enable_sensitive_data = _read_bool_env("ENABLE_SENSITIVE_DATA")
|
||||
|
||||
|
||||
def configure_otel_providers(
|
||||
*,
|
||||
enable_sensitive_data: bool | None = None,
|
||||
enable_console_exporters: bool | None = None,
|
||||
exporters: list[LogRecordExporter | SpanExporter | MetricExporter] | None = None,
|
||||
views: list[View] | None = None,
|
||||
vs_code_extension_port: int | None = None,
|
||||
@@ -963,6 +986,8 @@ def configure_otel_providers(
|
||||
Keyword Args:
|
||||
enable_sensitive_data: Enable OpenTelemetry sensitive events. Overrides
|
||||
the environment variable ENABLE_SENSITIVE_DATA if set. Default is None.
|
||||
enable_console_exporters: Enable console exporters for traces, logs, and metrics.
|
||||
Overrides the environment variable ENABLE_CONSOLE_EXPORTERS if set. Default is None.
|
||||
exporters: A list of custom exporters for logs, metrics or spans, or any combination.
|
||||
These will be added in addition to exporters configured via environment variables.
|
||||
Default is None.
|
||||
@@ -1051,6 +1076,8 @@ def configure_otel_providers(
|
||||
settings_kwargs["env_file_encoding"] = env_file_encoding
|
||||
if enable_sensitive_data is not None:
|
||||
settings_kwargs["enable_sensitive_data"] = enable_sensitive_data
|
||||
if enable_console_exporters is not None:
|
||||
settings_kwargs["enable_console_exporters"] = enable_console_exporters
|
||||
if vs_code_extension_port is not None:
|
||||
settings_kwargs["vs_code_extension_port"] = vs_code_extension_port
|
||||
|
||||
@@ -1064,12 +1091,22 @@ def configure_otel_providers(
|
||||
OBSERVABILITY_SETTINGS._resource = updated_settings._resource # type: ignore[reportPrivateUsage]
|
||||
OBSERVABILITY_SETTINGS._executed_setup = False # type: ignore[reportPrivateUsage]
|
||||
else:
|
||||
# Update the observability settings with the provided values
|
||||
# Re-read settings from current environment in case env vars were set
|
||||
# after import (e.g. via load_dotenv()). Explicit parameters take precedence.
|
||||
OBSERVABILITY_SETTINGS.enable_instrumentation = True
|
||||
if enable_sensitive_data is not None:
|
||||
OBSERVABILITY_SETTINGS.enable_sensitive_data = enable_sensitive_data
|
||||
if vs_code_extension_port is not None:
|
||||
OBSERVABILITY_SETTINGS.vs_code_extension_port = vs_code_extension_port
|
||||
OBSERVABILITY_SETTINGS.enable_sensitive_data = (
|
||||
enable_sensitive_data if enable_sensitive_data is not None else _read_bool_env("ENABLE_SENSITIVE_DATA")
|
||||
)
|
||||
OBSERVABILITY_SETTINGS.enable_console_exporters = (
|
||||
enable_console_exporters
|
||||
if enable_console_exporters is not None
|
||||
else _read_bool_env("ENABLE_CONSOLE_EXPORTERS")
|
||||
)
|
||||
OBSERVABILITY_SETTINGS.vs_code_extension_port = (
|
||||
vs_code_extension_port if vs_code_extension_port is not None else _read_int_env("VS_CODE_EXTENSION_PORT")
|
||||
)
|
||||
OBSERVABILITY_SETTINGS._resource = create_resource() # type: ignore[reportPrivateUsage]
|
||||
OBSERVABILITY_SETTINGS._executed_setup = False # type: ignore[reportPrivateUsage]
|
||||
|
||||
OBSERVABILITY_SETTINGS._configure( # type: ignore[reportPrivateUsage]
|
||||
additional_exporters=exporters,
|
||||
|
||||
@@ -1033,6 +1033,272 @@ def test_enable_instrumentation_with_sensitive_data(monkeypatch):
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_sensitive_data is True
|
||||
|
||||
|
||||
def test_enable_instrumentation_reads_env_sensitive_data(monkeypatch):
|
||||
"""Test enable_instrumentation re-reads ENABLE_SENSITIVE_DATA from os.environ when not explicitly passed."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.setenv("ENABLE_SENSITIVE_DATA", "false")
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_sensitive_data is False
|
||||
|
||||
# Simulate load_dotenv() setting env var after import
|
||||
monkeypatch.setenv("ENABLE_SENSITIVE_DATA", "true")
|
||||
|
||||
observability.enable_instrumentation()
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_instrumentation is True
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_sensitive_data is True
|
||||
|
||||
|
||||
def test_configure_otel_providers_reads_env_sensitive_data(monkeypatch):
|
||||
"""Test configure_otel_providers re-reads ENABLE_SENSITIVE_DATA from os.environ when not explicitly passed."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.setenv("ENABLE_SENSITIVE_DATA", "false")
|
||||
monkeypatch.delenv("VS_CODE_EXTENSION_PORT", raising=False)
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
for key in [
|
||||
"OTEL_EXPORTER_OTLP_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_sensitive_data is False
|
||||
|
||||
# Simulate load_dotenv() setting env var after import
|
||||
monkeypatch.setenv("ENABLE_SENSITIVE_DATA", "true")
|
||||
|
||||
observability.configure_otel_providers()
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_instrumentation is True
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_sensitive_data is True
|
||||
|
||||
|
||||
def test_configure_otel_providers_reads_env_vs_code_port(monkeypatch):
|
||||
"""Test configure_otel_providers re-reads VS_CODE_EXTENSION_PORT from os.environ when not explicitly passed."""
|
||||
import importlib
|
||||
from unittest.mock import patch as mock_patch
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.delenv("VS_CODE_EXTENSION_PORT", raising=False)
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
for key in [
|
||||
"OTEL_EXPORTER_OTLP_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
assert observability.OBSERVABILITY_SETTINGS.vs_code_extension_port is None
|
||||
|
||||
# Simulate load_dotenv() setting env var after import
|
||||
monkeypatch.setenv("VS_CODE_EXTENSION_PORT", "4317")
|
||||
|
||||
# Mock _configure to avoid needing optional OTLP gRPC exporter dependency
|
||||
with mock_patch.object(observability.OBSERVABILITY_SETTINGS, "_configure"):
|
||||
observability.configure_otel_providers()
|
||||
assert observability.OBSERVABILITY_SETTINGS.vs_code_extension_port == 4317
|
||||
|
||||
|
||||
def test_configure_otel_providers_explicit_param_overrides_env(monkeypatch):
|
||||
"""Test that explicit parameters to configure_otel_providers override env vars."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.setenv("ENABLE_SENSITIVE_DATA", "true")
|
||||
monkeypatch.delenv("VS_CODE_EXTENSION_PORT", raising=False)
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
for key in [
|
||||
"OTEL_EXPORTER_OTLP_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
# Explicit False should override the env var True
|
||||
observability.configure_otel_providers(enable_sensitive_data=False)
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_sensitive_data is False
|
||||
|
||||
|
||||
def test_enable_instrumentation_explicit_param_overrides_env(monkeypatch):
|
||||
"""Test that explicit enable_sensitive_data parameter to enable_instrumentation overrides env var."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.setenv("ENABLE_SENSITIVE_DATA", "true")
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
# Explicit False should override the env var True
|
||||
observability.enable_instrumentation(enable_sensitive_data=False)
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_instrumentation is True
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_sensitive_data is False
|
||||
|
||||
|
||||
def test_enable_instrumentation_does_not_touch_console_exporters(monkeypatch):
|
||||
"""Test enable_instrumentation does not modify enable_console_exporters (it is an exporter concern)."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is False
|
||||
|
||||
# Simulate load_dotenv() setting env var after import
|
||||
monkeypatch.setenv("ENABLE_CONSOLE_EXPORTERS", "true")
|
||||
|
||||
observability.enable_instrumentation()
|
||||
# enable_console_exporters is not managed by enable_instrumentation;
|
||||
# it is only read by configure_otel_providers.
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is False
|
||||
|
||||
|
||||
def test_enable_instrumentation_does_not_clobber_console_exporters(monkeypatch):
|
||||
"""Test enable_instrumentation does not reset enable_console_exporters set by prior configure call."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
monkeypatch.delenv("ENABLE_SENSITIVE_DATA", raising=False)
|
||||
monkeypatch.delenv("VS_CODE_EXTENSION_PORT", raising=False)
|
||||
for key in [
|
||||
"OTEL_EXPORTER_OTLP_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
# Set console exporters via configure_otel_providers
|
||||
observability.configure_otel_providers(enable_console_exporters=True)
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is True
|
||||
|
||||
# Calling enable_instrumentation should not clobber the value
|
||||
observability.enable_instrumentation()
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is True
|
||||
|
||||
|
||||
def test_enable_instrumentation_with_sensitive_data_does_not_touch_console_exporters(monkeypatch):
|
||||
"""Test enable_console_exporters is untouched even when enable_sensitive_data is explicitly passed."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
monkeypatch.delenv("ENABLE_SENSITIVE_DATA", raising=False)
|
||||
monkeypatch.delenv("VS_CODE_EXTENSION_PORT", raising=False)
|
||||
for key in [
|
||||
"OTEL_EXPORTER_OTLP_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
# Set console exporters via configure_otel_providers
|
||||
observability.configure_otel_providers(enable_console_exporters=True)
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is True
|
||||
|
||||
# Calling enable_instrumentation with explicit sensitive_data should not clobber console exporters
|
||||
observability.enable_instrumentation(enable_sensitive_data=True)
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is True
|
||||
|
||||
|
||||
def test_enable_instrumentation_preserves_console_exporters_after_env_removed(monkeypatch):
|
||||
"""Test enable_instrumentation preserves enable_console_exporters when env var is removed after reload."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.setenv("ENABLE_CONSOLE_EXPORTERS", "true")
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is True
|
||||
|
||||
# Remove the env var after reload
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
|
||||
# enable_instrumentation should not reset the value
|
||||
observability.enable_instrumentation()
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is True
|
||||
|
||||
|
||||
def test_configure_otel_providers_reads_env_console_exporters(monkeypatch):
|
||||
"""Test configure_otel_providers re-reads ENABLE_CONSOLE_EXPORTERS from os.environ when not explicitly passed."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.delenv("VS_CODE_EXTENSION_PORT", raising=False)
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
for key in [
|
||||
"OTEL_EXPORTER_OTLP_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is False
|
||||
|
||||
# Simulate load_dotenv() setting env var after import
|
||||
monkeypatch.setenv("ENABLE_CONSOLE_EXPORTERS", "true")
|
||||
|
||||
observability.configure_otel_providers()
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is True
|
||||
|
||||
|
||||
def test_configure_otel_providers_explicit_console_exporters_overrides_env(monkeypatch):
|
||||
"""Test that explicit enable_console_exporters parameter overrides the environment variable."""
|
||||
import importlib
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
monkeypatch.setenv("ENABLE_CONSOLE_EXPORTERS", "true")
|
||||
monkeypatch.delenv("VS_CODE_EXTENSION_PORT", raising=False)
|
||||
for key in [
|
||||
"OTEL_EXPORTER_OTLP_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT",
|
||||
"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
observability = importlib.import_module("agent_framework.observability")
|
||||
importlib.reload(observability)
|
||||
|
||||
# Explicit False should override the env var True
|
||||
observability.configure_otel_providers(enable_console_exporters=False)
|
||||
assert observability.OBSERVABILITY_SETTINGS.enable_console_exporters is False
|
||||
|
||||
|
||||
# region Test _to_otel_part content types
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user