From bcc1b96dbc0e8e99f5a737be03cac610478a32f2 Mon Sep 17 00:00:00 2001 From: droideronline Date: Tue, 18 Aug 2026 01:01:15 +0530 Subject: [PATCH 1/4] Python: Allow programmatic OTel service name, resource attributes, and OTLP exporter config in configure_otel_providers() Previously, configure_otel_providers() only read service.name, resource attributes, OTLP endpoint, protocol, headers, timeout, and compression from environment variables (OTEL_SERVICE_NAME, OTEL_RESOURCE_ATTRIBUTES, OTEL_EXPORTER_OTLP_*), even though the lower-level create_resource() helper already supported passing these programmatically. This forced any caller of the documented one-call setup entry point to rely on env vars/.env files for basic telemetry identification and exporter configuration. Add service_name, service_version, resource_attributes, otlp_endpoint, otlp_protocol, otlp_headers, otlp_timeout, and otlp_compression keyword arguments to configure_otel_providers() and ObservabilitySettings, threaded through to create_resource() and the OTLP exporter construction path. Explicit parameters take precedence over the corresponding base environment variable; signal-specific environment variables (e.g. OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) still take precedence over both, matching standard OTel env var rules. mTLS/certificate options remain out of scope for this convenience layer; those can still be set by constructing exporters directly and passing them via configure_otel_providers(exporters=...). --- .../core/agent_framework/observability.py | 257 +++++++++++++++++- .../core/tests/core/test_observability.py | 123 +++++++++ 2 files changed, 365 insertions(+), 15 deletions(-) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index 5b40626875f..117b3e7188d 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -402,6 +402,38 @@ def _parse_headers(header_str: str) -> dict[str, str]: return headers +def _grpc_compression(compression: str | None) -> Any: + """Map a compression name ("gzip"/"deflate"/"none") to the grpc.Compression enum.""" + if compression is None: + return None + import grpc + + try: + return { + "gzip": grpc.Compression.Gzip, + "deflate": grpc.Compression.Deflate, + "none": grpc.Compression.NoCompression, + }[compression.lower()] + except KeyError as exc: + raise ValueError(f"Invalid compression '{compression}'. Expected 'gzip', 'deflate' or 'none'.") from exc + + +def _http_compression(compression: str | None) -> Any: + """Map a compression name ("gzip"/"deflate"/"none") to the HTTP exporter's Compression enum.""" + if compression is None: + return None + from opentelemetry.exporter.otlp.proto.http import Compression + + try: + return { + "gzip": Compression.Gzip, + "deflate": Compression.Deflate, + "none": Compression.NoCompression, + }[compression.lower()] + except KeyError as exc: + raise ValueError(f"Invalid compression '{compression}'. Expected 'gzip', 'deflate' or 'none'.") from exc + + def _create_otlp_exporters( endpoint: str | None = None, protocol: str = "grpc", @@ -412,6 +444,8 @@ def _create_otlp_exporters( metrics_headers: dict[str, str] | None = None, logs_endpoint: str | None = None, logs_headers: dict[str, str] | None = None, + timeout: float | None = None, + compression: str | None = None, ) -> list[LogRecordExporter | SpanExporter | MetricExporter]: """Create OTLP exporters for a given endpoint and protocol. @@ -425,12 +459,18 @@ def _create_otlp_exporters( metrics_headers: Optional specific headers for metrics. Overrides headers parameter. logs_endpoint: Optional specific endpoint for logs. Overrides endpoint parameter. logs_headers: Optional specific headers for logs. Overrides headers parameter. + timeout: Optional export timeout in seconds, applied to all exporters. If None, each + exporter falls back to reading OTEL_EXPORTER_OTLP_TIMEOUT itself. Default is None. + compression: Optional compression ("gzip", "deflate" or "none"), applied to all + exporters. If None, each exporter falls back to reading + OTEL_EXPORTER_OTLP_COMPRESSION itself. Default is None. Returns: List containing OTLPLogExporter, OTLPSpanExporter, and OTLPMetricExporter. Raises: ImportError: If the required OTLP exporter package is not installed. + ValueError: If `compression` is not a recognized value. """ # Determine actual endpoints and headers to use actual_traces_endpoint = traces_endpoint or endpoint @@ -463,11 +503,15 @@ def _create_otlp_exporters( "Install it with: pip install opentelemetry-exporter-otlp-proto-grpc" ) from exc + grpc_compression = _grpc_compression(compression) + if actual_logs_endpoint: exporters.append( GRPCLogExporter( endpoint=actual_logs_endpoint, headers=actual_logs_headers if actual_logs_headers else None, + timeout=timeout, + compression=grpc_compression, ) ) if actual_traces_endpoint: @@ -475,6 +519,8 @@ def _create_otlp_exporters( GRPCSpanExporter( endpoint=actual_traces_endpoint, headers=actual_traces_headers if actual_traces_headers else None, + timeout=timeout, + compression=grpc_compression, ) ) if actual_metrics_endpoint: @@ -482,6 +528,8 @@ def _create_otlp_exporters( GRPCMetricExporter( endpoint=actual_metrics_endpoint, headers=actual_metrics_headers if actual_metrics_headers else None, + timeout=timeout, + compression=grpc_compression, ) ) @@ -503,11 +551,15 @@ def _create_otlp_exporters( "Install it with: pip install opentelemetry-exporter-otlp-proto-http" ) from exc + http_compression = _http_compression(compression) + if actual_logs_endpoint: exporters.append( HTTPLogExporter( endpoint=actual_logs_endpoint, headers=actual_logs_headers if actual_logs_headers else None, + timeout=timeout, + compression=http_compression, ) ) if actual_traces_endpoint: @@ -515,6 +567,8 @@ def _create_otlp_exporters( HTTPSpanExporter( endpoint=actual_traces_endpoint, headers=actual_traces_headers if actual_traces_headers else None, + timeout=timeout, + compression=http_compression, ) ) if actual_metrics_endpoint: @@ -522,6 +576,8 @@ def _create_otlp_exporters( HTTPMetricExporter( endpoint=actual_metrics_endpoint, headers=actual_metrics_headers if actual_metrics_headers else None, + timeout=timeout, + compression=http_compression, ) ) @@ -531,11 +587,20 @@ def _create_otlp_exporters( def _get_exporters_from_env( env_file_path: str | None = None, env_file_encoding: str | None = None, + endpoint: str | None = None, + protocol: str | None = None, + headers: dict[str, str] | None = None, + timeout: float | None = None, + compression: str | None = None, ) -> list[LogRecordExporter | SpanExporter | MetricExporter]: """Parse OpenTelemetry environment variables and create exporters. This function reads standard OpenTelemetry environment variables to configure - OTLP exporters for traces, logs, and metrics. + OTLP exporters for traces, logs, and metrics. The ``endpoint``, ``protocol``, + ``headers``, ``timeout`` and ``compression`` parameters let callers override the + corresponding *base* (all-signal) environment variable programmatically; signal-specific + environment variables (e.g. ``OTEL_EXPORTER_OTLP_TRACES_ENDPOINT``) still take precedence + over both, matching the normal OTel env var precedence rules. The following environment variables are supported: - OTEL_EXPORTER_OTLP_ENDPOINT: Base endpoint for all signals @@ -547,15 +612,35 @@ def _get_exporters_from_env( - OTEL_EXPORTER_OTLP_TRACES_HEADERS: Headers specifically for traces - OTEL_EXPORTER_OTLP_METRICS_HEADERS: Headers specifically for metrics - OTEL_EXPORTER_OTLP_LOGS_HEADERS: Headers specifically for logs + - OTEL_EXPORTER_OTLP_TIMEOUT: Export timeout in seconds, for all signals + - OTEL_EXPORTER_OTLP_COMPRESSION: Compression to use ("gzip" or "deflate"), for all signals + + Note: + Signal-specific timeout/compression env vars, and mTLS/certificate/insecure-channel + options, are not resolved here. They are still honored because the underlying + ``OTLPSpanExporter``/``OTLPLogExporter``/``OTLPMetricExporter`` constructors read + those environment variables themselves whenever the corresponding constructor + argument is left unset. Callers needing programmatic control over those options + can construct exporters directly and pass them via ``configure_otel_providers(exporters=...)``. Args: env_file_path: Path to a .env file to load environment variables from. Default is None, which does not load a .env file. env_file_encoding: Encoding to use when reading the .env file. Default is None, which uses the system default encoding. + endpoint: Override the base OTLP endpoint. Takes precedence over + OTEL_EXPORTER_OTLP_ENDPOINT if set. Default is None. + protocol: Override the OTLP protocol ("grpc" or "http/protobuf"). Takes + precedence over OTEL_EXPORTER_OTLP_PROTOCOL if set. Default is None. + headers: Override the base OTLP headers. Merged with (and taking precedence + over) OTEL_EXPORTER_OTLP_HEADERS if set. Default is None. + timeout: Override the base OTLP export timeout, in seconds. Takes precedence + over OTEL_EXPORTER_OTLP_TIMEOUT if set. Default is None. + compression: Override the base OTLP compression ("gzip" or "deflate"). Takes + precedence over OTEL_EXPORTER_OTLP_COMPRESSION if set. Default is None. Returns: - List of configured exporters (empty if no relevant env vars are set). + List of configured exporters (empty if no relevant env vars/parameters are set). References: - https://opentelemetry.io/docs/languages/sdk-configuration/general/ @@ -565,16 +650,16 @@ def _get_exporters_from_env( if env_file_path is not None: load_dotenv(dotenv_path=env_file_path, encoding=env_file_encoding) - # Get base endpoint - base_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT") + # Get base endpoint (explicit parameter takes precedence over the env var) + base_endpoint = endpoint if endpoint is not None else os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT") - # Get signal-specific endpoints (these override base endpoint and are used verbatim) + # Get signal-specific endpoints (these override base endpoint/param and are used verbatim) traces_endpoint_specific = os.getenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT") metrics_endpoint_specific = os.getenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT") logs_endpoint_specific = os.getenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT") - # Get protocol (default is grpc) - protocol = os.getenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc").lower() + # Get protocol (explicit parameter takes precedence over the env var; default is grpc) + resolved_protocol = (protocol if protocol is not None else os.getenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc")).lower() # Per the OTel spec, OTEL_EXPORTER_OTLP_ENDPOINT is a *base* URL for HTTP — the SDK # auto-appends /v1/{traces,metrics,logs} when it reads the env var directly. The @@ -586,7 +671,7 @@ def _get_exporters_from_env( traces_endpoint: str | None metrics_endpoint: str | None logs_endpoint: str | None - if protocol in ("http/protobuf", "http") and base_endpoint: + if resolved_protocol in ("http/protobuf", "http") and base_endpoint: base_for_http = base_endpoint.rstrip("/") traces_endpoint = traces_endpoint_specific or f"{base_for_http}/v1/traces" metrics_endpoint = metrics_endpoint_specific or f"{base_for_http}/v1/metrics" @@ -596,11 +681,10 @@ def _get_exporters_from_env( metrics_endpoint = metrics_endpoint_specific or base_endpoint logs_endpoint = logs_endpoint_specific or base_endpoint - # Get base headers - base_headers_str = os.getenv("OTEL_EXPORTER_OTLP_HEADERS", "") - base_headers = _parse_headers(base_headers_str) + # Get base headers (explicit parameter takes precedence over the env var) + base_headers = headers if headers is not None else _parse_headers(os.getenv("OTEL_EXPORTER_OTLP_HEADERS", "")) - # Get signal-specific headers (these merge with base headers) + # Get signal-specific headers (these merge with, and take precedence over, base headers) traces_headers_str = os.getenv("OTEL_EXPORTER_OTLP_TRACES_HEADERS", "") metrics_headers_str = os.getenv("OTEL_EXPORTER_OTLP_METRICS_HEADERS", "") logs_headers_str = os.getenv("OTEL_EXPORTER_OTLP_LOGS_HEADERS", "") @@ -609,15 +693,19 @@ def _get_exporters_from_env( metrics_headers = {**base_headers, **_parse_headers(metrics_headers_str)} logs_headers = {**base_headers, **_parse_headers(logs_headers_str)} - # Create exporters using helper function + # Create exporters using helper function. `timeout`/`compression` are forwarded as-is + # (including None) — when None, the underlying OTLP exporter classes resolve them from + # OTEL_EXPORTER_OTLP_TIMEOUT / OTEL_EXPORTER_OTLP_COMPRESSION themselves. return _create_otlp_exporters( - protocol=protocol, + protocol=resolved_protocol, traces_endpoint=traces_endpoint, traces_headers=traces_headers if traces_headers else None, metrics_endpoint=metrics_endpoint, metrics_headers=metrics_headers if metrics_headers else None, logs_endpoint=logs_endpoint, logs_headers=logs_headers if logs_headers else None, + timeout=timeout, + compression=compression, ) @@ -757,6 +845,29 @@ class ObservabilitySettings: vs_code_extension_port: The port the AI Toolkit or Microsoft Foundry VS Code extensions are listening on. Default is None. Can be set via environment variable VS_CODE_EXTENSION_PORT. + service_name: Override the service name reported in telemetry. Default is None, which + falls back to the environment variable OTEL_SERVICE_NAME, or "agent_framework". + service_version: Override the service version reported in telemetry. Default is None, + which falls back to the environment variable OTEL_SERVICE_VERSION, or the installed + package version. + resource_attributes: Additional OpenTelemetry resource attributes to attach to every + span, log and metric. Default is None. These are merged with (and take precedence + over) attributes from the environment variable OTEL_RESOURCE_ATTRIBUTES. + otlp_endpoint: Override the base OTLP endpoint. Default is None, which falls back to + the environment variable OTEL_EXPORTER_OTLP_ENDPOINT. Signal-specific endpoint + environment variables (e.g. OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) still take + precedence over this, matching standard OTel env var rules. + otlp_protocol: Override the OTLP protocol ("grpc" or "http/protobuf"). Default is + None, which falls back to the environment variable OTEL_EXPORTER_OTLP_PROTOCOL, + or "grpc". + otlp_headers: Override the base OTLP headers. Default is None, which falls back to + the environment variable OTEL_EXPORTER_OTLP_HEADERS. Signal-specific header + environment variables still merge with, and take precedence over, this. + otlp_timeout: Override the OTLP export timeout, in seconds. Default is None, which + falls back to the environment variable OTEL_EXPORTER_OTLP_TIMEOUT, or 10 seconds. + otlp_compression: Override the OTLP compression ("gzip", "deflate" or "none"). + Default is None, which falls back to the environment variable + OTEL_EXPORTER_OTLP_COMPRESSION, or no compression. Examples: .. code-block:: python @@ -776,6 +887,22 @@ def __init__(self, **kwargs: Any) -> None: """Initialize the settings.""" env_file_path = kwargs.pop("env_file_path", None) env_file_encoding = kwargs.pop("env_file_encoding", None) + # service_name/service_version/resource_attributes deliberately bypass + # `load_settings()`: that helper falls back to a generic `` env var + # (e.g. SERVICE_NAME), which doesn't match the OTel-standard OTEL_SERVICE_NAME / + # OTEL_RESOURCE_ATTRIBUTES vars that `create_resource()` already reads. Keeping + # them as plain overrides avoids introducing a second, conflicting env var. + service_name = kwargs.pop("service_name", None) + service_version = kwargs.pop("service_version", None) + resource_attributes = kwargs.pop("resource_attributes", None) + # Same rationale as above: these mirror OTEL_EXPORTER_OTLP_ENDPOINT / _PROTOCOL / + # _HEADERS / _TIMEOUT / _COMPRESSION, which `_get_exporters_from_env()` already + # reads directly, so they bypass `load_settings()` too. + otlp_endpoint = kwargs.pop("otlp_endpoint", None) + otlp_protocol = kwargs.pop("otlp_protocol", None) + otlp_headers = kwargs.pop("otlp_headers", None) + otlp_timeout = kwargs.pop("otlp_timeout", None) + otlp_compression = kwargs.pop("otlp_compression", None) data = load_settings( _ObservabilitySettingsData, env_file_path=env_file_path, @@ -803,6 +930,14 @@ def __init__(self, **kwargs: Any) -> None: self.vs_code_extension_port: int | None = data.get("vs_code_extension_port") self.env_file_path = env_file_path self.env_file_encoding = env_file_encoding + self.service_name: str | None = service_name + self.service_version: str | None = service_version + self.resource_attributes: dict[str, Any] | None = resource_attributes + self.otlp_endpoint: str | None = otlp_endpoint + self.otlp_protocol: str | None = otlp_protocol + self.otlp_headers: dict[str, str] | None = otlp_headers + self.otlp_timeout: float | None = otlp_timeout + self.otlp_compression: str | None = otlp_compression self._executed_setup = False @property @@ -903,11 +1038,17 @@ def _configure( exporters: list[LogRecordExporter | SpanExporter | MetricExporter] = [] - # 1. Add exporters from standard OTEL environment variables + # 1. Add exporters from standard OTEL environment variables, with programmatic + # overrides for endpoint/protocol/headers/timeout/compression taking precedence. exporters.extend( _get_exporters_from_env( env_file_path=self.env_file_path, env_file_encoding=self.env_file_encoding, + endpoint=self.otlp_endpoint, + protocol=self.otlp_protocol, + headers=self.otlp_headers, + timeout=self.otlp_timeout, + compression=self.otlp_compression, ) ) @@ -971,8 +1112,11 @@ def _configure_providers( log_exporters: list[LogRecordExporter] = [] metric_exporters: list[MetricExporter] = [] resource = create_resource( + service_name=self.service_name, + service_version=self.service_version, env_file_path=self.env_file_path, env_file_encoding=self.env_file_encoding, + **(self.resource_attributes or {}), ) for exp in exporters: if isinstance(exp, SpanExporter): @@ -1232,6 +1376,14 @@ def enable_instrumentation( def configure_otel_providers( *, + service_name: str | None = None, + service_version: str | None = None, + resource_attributes: dict[str, Any] | None = None, + otlp_endpoint: str | None = None, + otlp_protocol: str | None = None, + otlp_headers: dict[str, str] | None = None, + otlp_timeout: float | None = None, + otlp_compression: str | None = None, enable_sensitive_data: bool | None = None, enable_console_exporters: bool | None = None, exporters: list[LogRecordExporter | SpanExporter | MetricExporter] | None = None, @@ -1270,6 +1422,31 @@ def configure_otel_providers( the `create_metric_views()` helper function to get default views. Keyword Args: + service_name: Override the service name reported in telemetry. Overrides the + environment variable OTEL_SERVICE_NAME if set. Default is None, which falls + back to OTEL_SERVICE_NAME or "agent_framework". + service_version: Override the service version reported in telemetry. Overrides + the environment variable OTEL_SERVICE_VERSION if set. Default is None, which + falls back to OTEL_SERVICE_VERSION or the installed package version. + resource_attributes: Additional OpenTelemetry resource attributes (e.g. + `deployment_environment`) to attach to every span, log and metric. These are + merged with (and take precedence over) attributes from the environment + variable OTEL_RESOURCE_ATTRIBUTES. Default is None. + otlp_endpoint: Override the base OTLP endpoint used by the environment-variable-driven + exporters. Overrides OTEL_EXPORTER_OTLP_ENDPOINT if set. Default is None. + Signal-specific endpoint environment variables (e.g. + OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) still take precedence over this. + otlp_protocol: Override the OTLP protocol ("grpc" or "http/protobuf"). Overrides + OTEL_EXPORTER_OTLP_PROTOCOL if set. Default is None, which falls back to "grpc". + otlp_headers: Override the base OTLP headers (e.g. for auth tokens). Overrides + OTEL_EXPORTER_OTLP_HEADERS if set. Default is None. Signal-specific header + environment variables still merge with, and take precedence over, this. + otlp_timeout: Override the OTLP export timeout, in seconds. Overrides + OTEL_EXPORTER_OTLP_TIMEOUT if set. Default is None, which falls back to 10 seconds. + otlp_compression: Override the OTLP compression ("gzip", "deflate" or "none"). + Overrides OTEL_EXPORTER_OTLP_COMPRESSION if set. Default is None, which falls + back to no compression. For mTLS/certificate options or other exporter settings + not covered here, construct exporters directly and pass them via `exporters=`. 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. @@ -1302,6 +1479,22 @@ def configure_otel_providers( # Set ENABLE_CONSOLE_EXPORTERS=true configure_otel_providers() + # With a custom service name/version and resource attributes, passed + # programmatically instead of via OTEL_SERVICE_NAME / OTEL_RESOURCE_ATTRIBUTES + configure_otel_providers( + service_name="my_service", + service_version="1.0.0", + resource_attributes={"deployment_environment": "production"}, + ) + + # With a custom OTLP endpoint, headers and compression, passed programmatically + # instead of via OTEL_EXPORTER_OTLP_ENDPOINT / _HEADERS / _COMPRESSION + configure_otel_providers( + otlp_endpoint="https://otel-collector.example.com:4317", + otlp_headers={"Authorization": "Bearer "}, + otlp_compression="gzip", + ) + # With custom exporters from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter from opentelemetry.exporter.otlp.proto.grpc._log_exporter import OTLPLogExporter @@ -1372,6 +1565,22 @@ def configure_otel_providers( 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 + if service_name is not None: + settings_kwargs["service_name"] = service_name + if service_version is not None: + settings_kwargs["service_version"] = service_version + if resource_attributes is not None: + settings_kwargs["resource_attributes"] = resource_attributes + if otlp_endpoint is not None: + settings_kwargs["otlp_endpoint"] = otlp_endpoint + if otlp_protocol is not None: + settings_kwargs["otlp_protocol"] = otlp_protocol + if otlp_headers is not None: + settings_kwargs["otlp_headers"] = otlp_headers + if otlp_timeout is not None: + settings_kwargs["otlp_timeout"] = otlp_timeout + if otlp_compression is not None: + settings_kwargs["otlp_compression"] = otlp_compression updated_settings = ObservabilitySettings(**settings_kwargs) OBSERVABILITY_SETTINGS.enable_instrumentation = updated_settings.enable_instrumentation @@ -1380,6 +1589,14 @@ def configure_otel_providers( OBSERVABILITY_SETTINGS.vs_code_extension_port = updated_settings.vs_code_extension_port OBSERVABILITY_SETTINGS.env_file_path = updated_settings.env_file_path OBSERVABILITY_SETTINGS.env_file_encoding = updated_settings.env_file_encoding + OBSERVABILITY_SETTINGS.service_name = updated_settings.service_name + OBSERVABILITY_SETTINGS.service_version = updated_settings.service_version + OBSERVABILITY_SETTINGS.resource_attributes = updated_settings.resource_attributes + OBSERVABILITY_SETTINGS.otlp_endpoint = updated_settings.otlp_endpoint + OBSERVABILITY_SETTINGS.otlp_protocol = updated_settings.otlp_protocol + OBSERVABILITY_SETTINGS.otlp_headers = updated_settings.otlp_headers + OBSERVABILITY_SETTINGS.otlp_timeout = updated_settings.otlp_timeout + OBSERVABILITY_SETTINGS.otlp_compression = updated_settings.otlp_compression OBSERVABILITY_SETTINGS._executed_setup = False # type: ignore[reportPrivateUsage] else: # Re-read settings from current environment in case env vars were set @@ -1396,6 +1613,16 @@ def configure_otel_providers( 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") ) + # These have no generic env-var fallback here (the OTel exporter/resource + # construction code itself resolves the standard OTEL_* env vars when left as None). + OBSERVABILITY_SETTINGS.service_name = service_name + OBSERVABILITY_SETTINGS.service_version = service_version + OBSERVABILITY_SETTINGS.resource_attributes = resource_attributes + OBSERVABILITY_SETTINGS.otlp_endpoint = otlp_endpoint + OBSERVABILITY_SETTINGS.otlp_protocol = otlp_protocol + OBSERVABILITY_SETTINGS.otlp_headers = otlp_headers + OBSERVABILITY_SETTINGS.otlp_timeout = otlp_timeout + OBSERVABILITY_SETTINGS.otlp_compression = otlp_compression OBSERVABILITY_SETTINGS._executed_setup = False # type: ignore[reportPrivateUsage] OBSERVABILITY_SETTINGS._configure( # type: ignore[reportPrivateUsage] diff --git a/python/packages/core/tests/core/test_observability.py b/python/packages/core/tests/core/test_observability.py index 4256e62b2df..4c2025fe607 100644 --- a/python/packages/core/tests/core/test_observability.py +++ b/python/packages/core/tests/core/test_observability.py @@ -1138,6 +1138,86 @@ def test_get_exporters_from_env_grpc_base_endpoint_unchanged(monkeypatch): assert kwargs["logs_endpoint"] == "http://localhost:4317" +def test_get_exporters_from_env_params_override_env(monkeypatch): + """endpoint/protocol/headers/timeout/compression params should override the base env vars.""" + from unittest.mock import patch + + from agent_framework import observability + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://env-endpoint:4317") + monkeypatch.setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + monkeypatch.setenv("OTEL_EXPORTER_OTLP_HEADERS", "from-env=1") + for key in ( + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", + "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", + ): + monkeypatch.delenv(key, raising=False) + + with patch.object(observability, "_create_otlp_exporters", return_value=[]) as create: + observability._get_exporters_from_env( + endpoint="http://param-endpoint:4317", + protocol="http/protobuf", + headers={"from-param": "2"}, + timeout=5.0, + compression="gzip", + ) + + kwargs = create.call_args.kwargs + # Param endpoint/protocol win over env, with HTTP path-append applied to the param endpoint + assert kwargs["protocol"] == "http/protobuf" + assert kwargs["traces_endpoint"] == "http://param-endpoint:4317/v1/traces" + # Param headers win over (replace) base env headers + assert kwargs["traces_headers"] == {"from-param": "2"} + # timeout/compression forwarded as-is + assert kwargs["timeout"] == 5.0 + assert kwargs["compression"] == "gzip" + + +def test_get_exporters_from_env_signal_specific_env_wins_over_param(monkeypatch): + """Signal-specific env vars still take precedence over a programmatic base endpoint override.""" + from unittest.mock import patch + + from agent_framework import observability + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://traces-env:4317") + for key in ("OTEL_EXPORTER_OTLP_ENDPOINT", "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT"): + monkeypatch.delenv(key, raising=False) + + with patch.object(observability, "_create_otlp_exporters", return_value=[]) as create: + observability._get_exporters_from_env(endpoint="http://param-endpoint:4317") + + kwargs = create.call_args.kwargs + assert kwargs["traces_endpoint"] == "http://traces-env:4317" + assert kwargs["metrics_endpoint"] == "http://param-endpoint:4317" + + +def test_configure_otel_providers_otlp_params(monkeypatch): + """configure_otel_providers(otlp_endpoint=..., otlp_headers=..., ...) should be forwarded.""" + from unittest.mock import patch + + from agent_framework import observability + from agent_framework.observability import OBSERVABILITY_SETTINGS, configure_otel_providers + + monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) + OBSERVABILITY_SETTINGS._executed_setup = False + + with patch.object(observability, "_create_otlp_exporters", return_value=[]) as create: + configure_otel_providers( + otlp_endpoint="http://custom:4317", + otlp_protocol="grpc", + otlp_headers={"Authorization": "Bearer token"}, + otlp_timeout=7.5, + otlp_compression="deflate", + ) + + kwargs = create.call_args.kwargs + assert kwargs["traces_endpoint"] == "http://custom:4317" + assert kwargs["traces_headers"] == {"Authorization": "Bearer token"} + assert kwargs["timeout"] == 7.5 + assert kwargs["compression"] == "deflate" + + # region Test create_resource @@ -2640,6 +2720,49 @@ def test_observability_settings_configure_already_setup(monkeypatch): assert settings.is_setup is True +def test_observability_settings_service_name_overrides_env(monkeypatch): + """ObservabilitySettings(service_name=...) should be forwarded to create_resource, not just env vars.""" + from agent_framework import observability + from agent_framework.observability import ObservabilitySettings + + monkeypatch.setenv("OTEL_SERVICE_NAME", "env-service") + monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) + + settings = ObservabilitySettings( + service_name="param-service", + service_version="3.2.1", + resource_attributes={"deployment_environment": "test"}, + enable_console_exporters=True, + ) + with patch.object(observability, "create_resource", wraps=observability.create_resource) as create_resource: + settings._configure() + + assert create_resource.call_args.kwargs["service_name"] == "param-service" + assert create_resource.call_args.kwargs["service_version"] == "3.2.1" + assert create_resource.call_args.kwargs["deployment_environment"] == "test" + + +def test_configure_otel_providers_service_name_param(monkeypatch): + """configure_otel_providers(service_name=...) should be forwarded to create_resource.""" + from agent_framework import observability + from agent_framework.observability import OBSERVABILITY_SETTINGS, configure_otel_providers + + monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False) + OBSERVABILITY_SETTINGS._executed_setup = False + + with patch.object(observability, "create_resource", wraps=observability.create_resource) as create_resource: + configure_otel_providers( + service_name="my-service", + service_version="1.2.3", + resource_attributes={"deployment_environment": "prod"}, + enable_console_exporters=True, + ) + + assert create_resource.call_args.kwargs["service_name"] == "my-service" + assert create_resource.call_args.kwargs["service_version"] == "1.2.3" + assert create_resource.call_args.kwargs["deployment_environment"] == "prod" + + # region Test _to_otel_part edge cases From d2a16216646725e0fbb2efb2efd5468a72150f8d Mon Sep 17 00:00:00 2001 From: droideronline Date: Tue, 18 Aug 2026 14:38:53 +0530 Subject: [PATCH 2/4] Python: Address review feedback on OTel programmatic config PR - create_resource(): apply OTEL_RESOURCE_ATTRIBUTES as a base before explicit service_name/service_version/attributes, instead of overlaying it last, so an explicit value is never silently replaced by the environment. - create_resource(): accept resource attributes via a new attributes= dict parameter, in addition to **kwargs, so callers can pass a dictionary whose keys might collide with create_resource's own parameter names (e.g. service_name, env_file_path) without raising TypeError. Update _configure_providers() to pass resource_attributes via attributes= instead of unpacking it into **kwargs. - configure_otel_providers(): consolidate the service_name/service_version/ resource_attributes/otlp_* assignments that were duplicated across the env-file and non-env-file branches into a single shared block, since these fields don't need the env-file loading that the rest of the branch exists for. Addresses PR review comments from @moonbox3. --- .../core/agent_framework/observability.py | 85 ++++++++++--------- .../core/tests/core/test_observability.py | 64 +++++++++++++- 2 files changed, 106 insertions(+), 43 deletions(-) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index 117b3e7188d..cef51fad4be 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -714,7 +714,8 @@ def create_resource( service_version: str | None = None, env_file_path: str | None = None, env_file_encoding: str | None = None, - **attributes: Any, + attributes: dict[str, Any] | None = None, + **kwargs: Any, ) -> Resource: """Create an OpenTelemetry Resource from environment variables and parameters. @@ -726,6 +727,11 @@ def create_resource( - OTEL_SERVICE_VERSION: The version of the service (defaults to package version) - OTEL_RESOURCE_ATTRIBUTES: Additional resource attributes as key=value pairs + Explicit parameters always take precedence over OTEL_RESOURCE_ATTRIBUTES: the + environment variable is applied first as a base, then overlaid with `attributes`/`**kwargs`, + and finally with `service_name`/`service_version`, so a caller-supplied value is never + silently replaced by the environment. + Args: service_name: Override the service name. If not provided, reads from OTEL_SERVICE_NAME environment variable or defaults to "agent_framework". @@ -735,8 +741,15 @@ def create_resource( Default is None, which does not load a .env file. env_file_encoding: Encoding to use when reading the .env file. Default is None, which uses the system default encoding. - **attributes: Additional resource attributes to include. These will be merged - with attributes from OTEL_RESOURCE_ATTRIBUTES environment variable. + attributes: Additional resource attributes to include, as a dictionary. Prefer this + over `**kwargs` when the attribute keys come from caller-controlled data (e.g. a + dict that might happen to contain a key like "service_name" or "env_file_path"), + since `**kwargs` keys share the keyword namespace with this function's own + parameters and would raise `TypeError` on a collision. Merged with (and taking + precedence over) attributes from the OTEL_RESOURCE_ATTRIBUTES environment variable. + **kwargs: Additional resource attributes to include, as keyword arguments. Merged + with (and taking precedence over) attributes from the OTEL_RESOURCE_ATTRIBUTES + environment variable and `attributes`. Returns: A configured OpenTelemetry Resource instance. @@ -757,6 +770,9 @@ def create_resource( service_name="my_service", service_version="1.0.0", deployment_environment="production" ) + # Add custom attributes from a dict whose keys are not known ahead of time + resource = create_resource(service_name="my_service", attributes={"deployment_environment": "production"}) + # Load from custom .env file resource = create_resource(env_file_path="config/.env") """ @@ -771,7 +787,16 @@ def create_resource( if env_file_path is not None: load_dotenv(dotenv_path=env_file_path, encoding=env_file_encoding) - resource_attributes: dict[str, Any] = dict(attributes) + # Apply OTEL_RESOURCE_ATTRIBUTES first, as a base — everything applied after this + # (attributes/kwargs, then service_name/service_version) takes precedence over it, so an + # explicit value is never silently replaced by the environment. + resource_attributes: dict[str, Any] = {} + if resource_attrs_env := os.getenv("OTEL_RESOURCE_ATTRIBUTES"): + resource_attributes.update(_parse_headers(resource_attrs_env)) + + if attributes: + resource_attributes.update(attributes) + resource_attributes.update(kwargs) if service_name is None: service_name = os.getenv("OTEL_SERVICE_NAME", "agent_framework") @@ -781,8 +806,6 @@ def create_resource( service_version = os.getenv("OTEL_SERVICE_VERSION", version_info) resource_attributes[OtelAttr.SERVICE_VERSION] = service_version - if resource_attrs_env := os.getenv("OTEL_RESOURCE_ATTRIBUTES"): - resource_attributes.update(_parse_headers(resource_attrs_env)) return Resource.create(resource_attributes) @@ -1116,7 +1139,7 @@ def _configure_providers( service_version=self.service_version, env_file_path=self.env_file_path, env_file_encoding=self.env_file_encoding, - **(self.resource_attributes or {}), + attributes=self.resource_attributes, ) for exp in exporters: if isinstance(exp, SpanExporter): @@ -1565,22 +1588,6 @@ def configure_otel_providers( 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 - if service_name is not None: - settings_kwargs["service_name"] = service_name - if service_version is not None: - settings_kwargs["service_version"] = service_version - if resource_attributes is not None: - settings_kwargs["resource_attributes"] = resource_attributes - if otlp_endpoint is not None: - settings_kwargs["otlp_endpoint"] = otlp_endpoint - if otlp_protocol is not None: - settings_kwargs["otlp_protocol"] = otlp_protocol - if otlp_headers is not None: - settings_kwargs["otlp_headers"] = otlp_headers - if otlp_timeout is not None: - settings_kwargs["otlp_timeout"] = otlp_timeout - if otlp_compression is not None: - settings_kwargs["otlp_compression"] = otlp_compression updated_settings = ObservabilitySettings(**settings_kwargs) OBSERVABILITY_SETTINGS.enable_instrumentation = updated_settings.enable_instrumentation @@ -1589,14 +1596,6 @@ def configure_otel_providers( OBSERVABILITY_SETTINGS.vs_code_extension_port = updated_settings.vs_code_extension_port OBSERVABILITY_SETTINGS.env_file_path = updated_settings.env_file_path OBSERVABILITY_SETTINGS.env_file_encoding = updated_settings.env_file_encoding - OBSERVABILITY_SETTINGS.service_name = updated_settings.service_name - OBSERVABILITY_SETTINGS.service_version = updated_settings.service_version - OBSERVABILITY_SETTINGS.resource_attributes = updated_settings.resource_attributes - OBSERVABILITY_SETTINGS.otlp_endpoint = updated_settings.otlp_endpoint - OBSERVABILITY_SETTINGS.otlp_protocol = updated_settings.otlp_protocol - OBSERVABILITY_SETTINGS.otlp_headers = updated_settings.otlp_headers - OBSERVABILITY_SETTINGS.otlp_timeout = updated_settings.otlp_timeout - OBSERVABILITY_SETTINGS.otlp_compression = updated_settings.otlp_compression OBSERVABILITY_SETTINGS._executed_setup = False # type: ignore[reportPrivateUsage] else: # Re-read settings from current environment in case env vars were set @@ -1613,18 +1612,22 @@ def configure_otel_providers( 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") ) - # These have no generic env-var fallback here (the OTel exporter/resource - # construction code itself resolves the standard OTEL_* env vars when left as None). - OBSERVABILITY_SETTINGS.service_name = service_name - OBSERVABILITY_SETTINGS.service_version = service_version - OBSERVABILITY_SETTINGS.resource_attributes = resource_attributes - OBSERVABILITY_SETTINGS.otlp_endpoint = otlp_endpoint - OBSERVABILITY_SETTINGS.otlp_protocol = otlp_protocol - OBSERVABILITY_SETTINGS.otlp_headers = otlp_headers - OBSERVABILITY_SETTINGS.otlp_timeout = otlp_timeout - OBSERVABILITY_SETTINGS.otlp_compression = otlp_compression OBSERVABILITY_SETTINGS._executed_setup = False # type: ignore[reportPrivateUsage] + # These options have no generic env-var fallback of their own in ObservabilitySettings — + # the OTel exporter/resource construction code itself resolves the standard OTEL_* env + # vars when left as None — so, unlike the fields above, they don't need the env-file + # loading that ObservabilitySettings(**settings_kwargs) provides and can be assigned + # directly from the function parameters in a single shared block for both code paths. + OBSERVABILITY_SETTINGS.service_name = service_name + OBSERVABILITY_SETTINGS.service_version = service_version + OBSERVABILITY_SETTINGS.resource_attributes = resource_attributes + OBSERVABILITY_SETTINGS.otlp_endpoint = otlp_endpoint + OBSERVABILITY_SETTINGS.otlp_protocol = otlp_protocol + OBSERVABILITY_SETTINGS.otlp_headers = otlp_headers + OBSERVABILITY_SETTINGS.otlp_timeout = otlp_timeout + OBSERVABILITY_SETTINGS.otlp_compression = otlp_compression + OBSERVABILITY_SETTINGS._configure( # type: ignore[reportPrivateUsage] additional_exporters=exporters, views=views, diff --git a/python/packages/core/tests/core/test_observability.py b/python/packages/core/tests/core/test_observability.py index 4c2025fe607..04dcda77f31 100644 --- a/python/packages/core/tests/core/test_observability.py +++ b/python/packages/core/tests/core/test_observability.py @@ -1261,6 +1261,66 @@ def test_create_resource_with_custom_attributes(monkeypatch): assert resource.attributes["another_attr"] == 123 +def test_create_resource_explicit_values_win_over_resource_attributes_env(monkeypatch): + """OTEL_RESOURCE_ATTRIBUTES must not silently override explicit service_name/service_version/attributes. + + Regression test for a PR review comment: applying OTEL_RESOURCE_ATTRIBUTES *after* the + explicit values let the env var overwrite a caller-supplied service.name (or any other + caller-supplied attribute) if it happened to redefine that key, contradicting the documented + "explicit parameters take precedence" behavior. + """ + from agent_framework.observability import create_resource + + monkeypatch.setenv( + "OTEL_RESOURCE_ATTRIBUTES", + "service.name=env-service,service.version=0.0.1,deployment.environment=staging", + ) + + resource = create_resource( + service_name="checkout", + service_version="2.0.0", + attributes={"deployment.environment": "production"}, + ) + + assert resource.attributes["service.name"] == "checkout" + assert resource.attributes["service.version"] == "2.0.0" + assert resource.attributes["deployment.environment"] == "production" + + +def test_create_resource_env_attributes_still_apply_when_not_overridden(monkeypatch): + """OTEL_RESOURCE_ATTRIBUTES entries with no explicit override should still come through.""" + from agent_framework.observability import create_resource + + monkeypatch.setenv("OTEL_RESOURCE_ATTRIBUTES", "host.name=server1") + + resource = create_resource(service_name="checkout") + + assert resource.attributes["service.name"] == "checkout" + assert resource.attributes["host.name"] == "server1" + + +def test_create_resource_attributes_param_avoids_keyword_collision(monkeypatch): + """The `attributes=` dict param must accept keys that collide with create_resource's own + parameter names (e.g. "service_name"), which is not possible via **kwargs. + + Regression test for a PR review comment: `_configure_providers()` used to call + `create_resource(service_name=..., **resource_attributes)`, which raised `TypeError: + got multiple values for argument 'service_name'` if a caller's `resource_attributes` dict + happened to contain a key like "service_name" or "env_file_path". + """ + from agent_framework.observability import create_resource + + # Would raise TypeError if passed as **kwargs alongside the explicit service_name= below. + colliding_attributes = {"service_name": "from-dict", "env_file_path": "from-dict"} + + resource = create_resource(service_name="checkout", attributes=colliding_attributes) + + # The explicit service_name= parameter wins over the same-named dict entry. + assert resource.attributes["service.name"] == "checkout" + # The non-colliding key still makes it into the resource, under its literal name. + assert resource.attributes["env_file_path"] == "from-dict" + + # region Test _create_otlp_exporters @@ -2739,7 +2799,7 @@ def test_observability_settings_service_name_overrides_env(monkeypatch): assert create_resource.call_args.kwargs["service_name"] == "param-service" assert create_resource.call_args.kwargs["service_version"] == "3.2.1" - assert create_resource.call_args.kwargs["deployment_environment"] == "test" + assert create_resource.call_args.kwargs["attributes"] == {"deployment_environment": "test"} def test_configure_otel_providers_service_name_param(monkeypatch): @@ -2760,7 +2820,7 @@ def test_configure_otel_providers_service_name_param(monkeypatch): assert create_resource.call_args.kwargs["service_name"] == "my-service" assert create_resource.call_args.kwargs["service_version"] == "1.2.3" - assert create_resource.call_args.kwargs["deployment_environment"] == "prod" + assert create_resource.call_args.kwargs["attributes"] == {"deployment_environment": "prod"} # region Test _to_otel_part edge cases From ebd0d8157b0c585628c5c6b27816c361fcd029a5 Mon Sep 17 00:00:00 2001 From: droideronline Date: Wed, 19 Aug 2026 22:19:33 +0530 Subject: [PATCH 3/4] Python: Fix credential-leak and compat issues flagged in OTel config PR review - _create_otlp_exporters()/_get_exporters_from_env(): an explicit otlp_headers={} override was collapsed to None before reaching the OTLP exporter constructors, which then fell back to reading OTEL_EXPORTER_OTLP_HEADERS themselves (an empty dict is just as falsy as None to their own "headers or environ.get(...)" fallback). This silently resurrected an environment-configured credential the caller explicitly tried to suppress. Fixed by distinguishing "not resolved" (None, exporter may check env) from "resolved to nothing" (an authoritative {}, must not check env) throughout, and added _shield_env()/_construct_otlp_exporter() to temporarily hide the relevant header env vars for the duration of construction when a signal's headers were authoritatively resolved. - _get_exporters_from_env(): when both a programmatic otlp_endpoint and otlp_headers are given, withhold those headers from any signal whose endpoint resolves to a different origin (e.g. because a stray OTEL_EXPORTER_OTLP_TRACES_ENDPOINT points elsewhere), so a credential meant for one collector can't be sent to a different, unintended host. Pure env-var-driven configuration (OTEL_EXPORTER_OTLP_ENDPOINT + OTEL_EXPORTER_OTLP_HEADERS) is unaffected and keeps its existing, spec-conformant behavior. - create_resource(): the new attributes= parameter previously assumed a mapping and called dict.update() on whatever was passed, which raised ValueError for the pre-existing call shape create_resource(attributes= "some_value") (before attributes had a dedicated parameter, it was only reachable via **kwargs and set a literal resource attribute named "attributes"). Non-mapping values are now handled the same way as before, preserving that call shape alongside the new mapping form. Added 10 regression tests covering all three issues, verified end-to-end against the real opentelemetry-exporter-otlp-proto-grpc/http packages. Addresses automated review comments on PR #7703. --- .../core/agent_framework/observability.py | 225 ++++++++++++++---- .../core/tests/core/test_observability.py | 169 ++++++++++++- 2 files changed, 343 insertions(+), 51 deletions(-) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index cef51fad4be..0953e0f9378 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -36,6 +36,7 @@ cast, overload, ) +from urllib.parse import urlparse from dotenv import load_dotenv from opentelemetry import metrics, trace @@ -402,6 +403,29 @@ def _parse_headers(header_str: str) -> dict[str, str]: return headers +@contextlib.contextmanager +def _shield_env(*names: str) -> Generator[None, None, None]: + """Temporarily hide the given environment variables. + + The raw OTLP exporter constructors (grpc and http) fall back to reading + OTEL_EXPORTER_OTLP_HEADERS (and, for the http exporters, the signal-specific + OTEL_EXPORTER_OTLP_*_HEADERS too) themselves whenever the ``headers=`` argument they were + given is falsy — and an *explicitly empty* dict (``{}``) is just as falsy as ``None`` to + that check. So once we've already authoritatively resolved a signal's headers ourselves + (merging any programmatic override with the relevant env vars), we must prevent the + exporter's own env lookup from re-introducing a credential we intentionally left out — + otherwise ``otlp_headers={}`` would not actually suppress an environment-configured header. + """ + sentinel = object() + previous: dict[str, str | object] = {name: os.environ.pop(name, sentinel) for name in names} + try: + yield + finally: + for name, value in previous.items(): + if isinstance(value, str): + os.environ[name] = value + + def _grpc_compression(compression: str | None) -> Any: """Map a compression name ("gzip"/"deflate"/"none") to the grpc.Compression enum.""" if compression is None: @@ -434,6 +458,28 @@ def _http_compression(compression: str | None) -> Any: raise ValueError(f"Invalid compression '{compression}'. Expected 'gzip', 'deflate' or 'none'.") from exc +def _construct_otlp_exporter( + exporter_cls: Any, + endpoint: str, + headers: dict[str, str] | None, + timeout: float | None, + compression: Any, + *headers_env_vars: str, +) -> Any: + """Construct one OTLP exporter, shielding it from re-reading header env vars we've already resolved ourselves. + + When `headers` is not None, it is an authoritative, already-fully-resolved value (even if + it's `{}`) — see `_shield_env` for why that value alone can't be trusted to suppress the + exporter constructor's own env fallback, and why `headers_env_vars` must be hidden for the + duration of the call. When `headers` is None, no resolution was attempted for this signal + (the caller has no opinion), so the exporter is left free to resolve headers from env itself. + """ + if headers is not None: + with _shield_env(*headers_env_vars): + return exporter_cls(endpoint=endpoint, headers=headers, timeout=timeout, compression=compression) + return exporter_cls(endpoint=endpoint, headers=None, timeout=timeout, compression=compression) + + def _create_otlp_exporters( endpoint: str | None = None, protocol: str = "grpc", @@ -472,13 +518,20 @@ def _create_otlp_exporters( ImportError: If the required OTLP exporter package is not installed. ValueError: If `compression` is not a recognized value. """ - # Determine actual endpoints and headers to use + # Determine actual endpoints to use actual_traces_endpoint = traces_endpoint or endpoint actual_metrics_endpoint = metrics_endpoint or endpoint actual_logs_endpoint = logs_endpoint or endpoint - actual_traces_headers = traces_headers or headers - actual_metrics_headers = metrics_headers or headers - actual_logs_headers = logs_headers or headers + + # Determine actual headers to use. `is not None` (not `or`) matters here: a caller that + # authoritatively resolved a signal's headers to an *empty* dict (e.g. because they passed + # `otlp_headers={}` to explicitly suppress an environment-configured credential) must have + # that `{}` preserved rather than falling through to `headers`/env — see `_shield_env` below + # for why an empty dict alone isn't sufficient to stop the exporter constructors from + # re-reading the env var themselves. + actual_traces_headers = traces_headers if traces_headers is not None else headers + actual_metrics_headers = metrics_headers if metrics_headers is not None else headers + actual_logs_headers = logs_headers if logs_headers is not None else headers exporters: list[LogRecordExporter | SpanExporter | MetricExporter] = [] @@ -505,31 +558,39 @@ def _create_otlp_exporters( grpc_compression = _grpc_compression(compression) + # The raw grpc exporter classes only read the base OTEL_EXPORTER_OTLP_HEADERS + # themselves (not a signal-specific one), but shield it regardless of signal. if actual_logs_endpoint: exporters.append( - GRPCLogExporter( - endpoint=actual_logs_endpoint, - headers=actual_logs_headers if actual_logs_headers else None, - timeout=timeout, - compression=grpc_compression, + _construct_otlp_exporter( + GRPCLogExporter, + actual_logs_endpoint, + actual_logs_headers, + timeout, + grpc_compression, + "OTEL_EXPORTER_OTLP_HEADERS", ) ) if actual_traces_endpoint: exporters.append( - GRPCSpanExporter( - endpoint=actual_traces_endpoint, - headers=actual_traces_headers if actual_traces_headers else None, - timeout=timeout, - compression=grpc_compression, + _construct_otlp_exporter( + GRPCSpanExporter, + actual_traces_endpoint, + actual_traces_headers, + timeout, + grpc_compression, + "OTEL_EXPORTER_OTLP_HEADERS", ) ) if actual_metrics_endpoint: exporters.append( - GRPCMetricExporter( - endpoint=actual_metrics_endpoint, - headers=actual_metrics_headers if actual_metrics_headers else None, - timeout=timeout, - compression=grpc_compression, + _construct_otlp_exporter( + GRPCMetricExporter, + actual_metrics_endpoint, + actual_metrics_headers, + timeout, + grpc_compression, + "OTEL_EXPORTER_OTLP_HEADERS", ) ) @@ -553,31 +614,42 @@ def _create_otlp_exporters( http_compression = _http_compression(compression) + # Unlike the grpc exporters, the http exporter classes also read a signal-specific + # OTEL_EXPORTER_OTLP_*_HEADERS env var internally, so shield that one too. if actual_logs_endpoint: exporters.append( - HTTPLogExporter( - endpoint=actual_logs_endpoint, - headers=actual_logs_headers if actual_logs_headers else None, - timeout=timeout, - compression=http_compression, + _construct_otlp_exporter( + HTTPLogExporter, + actual_logs_endpoint, + actual_logs_headers, + timeout, + http_compression, + "OTEL_EXPORTER_OTLP_HEADERS", + "OTEL_EXPORTER_OTLP_LOGS_HEADERS", ) ) if actual_traces_endpoint: exporters.append( - HTTPSpanExporter( - endpoint=actual_traces_endpoint, - headers=actual_traces_headers if actual_traces_headers else None, - timeout=timeout, - compression=http_compression, + _construct_otlp_exporter( + HTTPSpanExporter, + actual_traces_endpoint, + actual_traces_headers, + timeout, + http_compression, + "OTEL_EXPORTER_OTLP_HEADERS", + "OTEL_EXPORTER_OTLP_TRACES_HEADERS", ) ) if actual_metrics_endpoint: exporters.append( - HTTPMetricExporter( - endpoint=actual_metrics_endpoint, - headers=actual_metrics_headers if actual_metrics_headers else None, - timeout=timeout, - compression=http_compression, + _construct_otlp_exporter( + HTTPMetricExporter, + actual_metrics_endpoint, + actual_metrics_headers, + timeout, + http_compression, + "OTEL_EXPORTER_OTLP_HEADERS", + "OTEL_EXPORTER_OTLP_METRICS_HEADERS", ) ) @@ -623,6 +695,16 @@ def _get_exporters_from_env( argument is left unset. Callers needing programmatic control over those options can construct exporters directly and pass them via ``configure_otel_providers(exporters=...)``. + Note: + If both ``endpoint`` and ``headers`` are given programmatically, those headers are + withheld from any signal whose endpoint resolves to a different origin (scheme, host or + port) than ``endpoint`` — e.g. because ``OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`` happens to + point elsewhere. This prevents a credential meant for one collector from being sent to a + different, unintended host. It does not apply when configuration is purely + env-var-driven (``OTEL_EXPORTER_OTLP_ENDPOINT`` + ``OTEL_EXPORTER_OTLP_HEADERS``), which + keeps its existing, spec-conformant behavior of applying headers to all signals + regardless of which endpoint they use. + Args: env_file_path: Path to a .env file to load environment variables from. Default is None, which does not load a .env file. @@ -684,37 +766,68 @@ def _get_exporters_from_env( # Get base headers (explicit parameter takes precedence over the env var) base_headers = headers if headers is not None else _parse_headers(os.getenv("OTEL_EXPORTER_OTLP_HEADERS", "")) + # If the caller passed *both* a programmatic base endpoint and base headers (i.e. the + # otlp_endpoint=/otlp_headers= parameters, not env vars), don't let those headers follow a + # signal whose endpoint resolves to a different origin — e.g. because a stray + # OTEL_EXPORTER_OTLP_TRACES_ENDPOINT happens to point elsewhere. Otherwise a credential + # meant for one collector could be sent to a different, unintended host. This only guards + # the new programmatic parameters: env-var-only configuration (OTEL_EXPORTER_OTLP_ENDPOINT + + # OTEL_EXPORTER_OTLP_HEADERS) keeps its existing, spec-conformant behavior of applying + # headers to all signals regardless of which endpoint they use. + programmatic_origin = _url_origin(endpoint) if (endpoint is not None and headers is not None) else None + + def _signal_base_headers(resolved_endpoint: str | None) -> dict[str, str]: + if programmatic_origin is not None and _url_origin(resolved_endpoint) != programmatic_origin: + return {} + return base_headers + # Get signal-specific headers (these merge with, and take precedence over, base headers) traces_headers_str = os.getenv("OTEL_EXPORTER_OTLP_TRACES_HEADERS", "") metrics_headers_str = os.getenv("OTEL_EXPORTER_OTLP_METRICS_HEADERS", "") logs_headers_str = os.getenv("OTEL_EXPORTER_OTLP_LOGS_HEADERS", "") - traces_headers = {**base_headers, **_parse_headers(traces_headers_str)} - metrics_headers = {**base_headers, **_parse_headers(metrics_headers_str)} - logs_headers = {**base_headers, **_parse_headers(logs_headers_str)} + traces_headers = {**_signal_base_headers(traces_endpoint), **_parse_headers(traces_headers_str)} + metrics_headers = {**_signal_base_headers(metrics_endpoint), **_parse_headers(metrics_headers_str)} + logs_headers = {**_signal_base_headers(logs_endpoint), **_parse_headers(logs_headers_str)} # Create exporters using helper function. `timeout`/`compression` are forwarded as-is # (including None) — when None, the underlying OTLP exporter classes resolve them from - # OTEL_EXPORTER_OTLP_TIMEOUT / OTEL_EXPORTER_OTLP_COMPRESSION themselves. + # OTEL_EXPORTER_OTLP_TIMEOUT / OTEL_EXPORTER_OTLP_COMPRESSION themselves. The header dicts + # are forwarded as-is too (even when empty) rather than collapsed to None — see + # `_construct_otlp_exporter`/`_shield_env` for why that distinction matters: an empty dict + # here means "we've authoritatively resolved this signal's headers to nothing", not + # "we have no opinion, the exporter may check the env var itself". return _create_otlp_exporters( protocol=resolved_protocol, traces_endpoint=traces_endpoint, - traces_headers=traces_headers if traces_headers else None, + traces_headers=traces_headers, metrics_endpoint=metrics_endpoint, - metrics_headers=metrics_headers if metrics_headers else None, + metrics_headers=metrics_headers, logs_endpoint=logs_endpoint, - logs_headers=logs_headers if logs_headers else None, + logs_headers=logs_headers, timeout=timeout, compression=compression, ) +def _url_origin(url: str | None) -> tuple[str, str | None, int | None] | None: + """Return (scheme, hostname, port) for `url`, or None if `url` is falsy. + + Used to compare endpoints by origin (ignoring path) so an HTTP endpoint's auto-appended + ``/v1/{traces,metrics,logs}`` suffix doesn't register as a different origin. + """ + if not url: + return None + parsed = urlparse(url) + return (parsed.scheme, parsed.hostname, parsed.port) + + def create_resource( service_name: str | None = None, service_version: str | None = None, env_file_path: str | None = None, env_file_encoding: str | None = None, - attributes: dict[str, Any] | None = None, + attributes: Mapping[str, Any] | Any = None, **kwargs: Any, ) -> Resource: """Create an OpenTelemetry Resource from environment variables and parameters. @@ -741,12 +854,17 @@ def create_resource( Default is None, which does not load a .env file. env_file_encoding: Encoding to use when reading the .env file. Default is None, which uses the system default encoding. - attributes: Additional resource attributes to include, as a dictionary. Prefer this - over `**kwargs` when the attribute keys come from caller-controlled data (e.g. a - dict that might happen to contain a key like "service_name" or "env_file_path"), - since `**kwargs` keys share the keyword namespace with this function's own - parameters and would raise `TypeError` on a collision. Merged with (and taking - precedence over) attributes from the OTEL_RESOURCE_ATTRIBUTES environment variable. + attributes: Additional resource attributes to include, as a mapping. Prefer this over + `**kwargs` when the attribute keys come from caller-controlled data (e.g. a dict + that might happen to contain a key like "service_name" or "env_file_path"), since + `**kwargs` keys share the keyword namespace with this function's own parameters and + would raise `TypeError` on a collision. Merged with (and taking precedence over) + attributes from the OTEL_RESOURCE_ATTRIBUTES environment variable. + For backward compatibility with versions where `attributes` had no dedicated + parameter (and was only reachable as part of `**kwargs`, i.e. + `create_resource(attributes=)` set a literal resource attribute named + "attributes"), a non-mapping value is treated the same way: as the value of a + resource attribute literally named "attributes", not merged as a mapping. **kwargs: Additional resource attributes to include, as keyword arguments. Merged with (and taking precedence over) attributes from the OTEL_RESOURCE_ATTRIBUTES environment variable and `attributes`. @@ -794,8 +912,15 @@ def create_resource( if resource_attrs_env := os.getenv("OTEL_RESOURCE_ATTRIBUTES"): resource_attributes.update(_parse_headers(resource_attrs_env)) - if attributes: - resource_attributes.update(attributes) + if attributes is not None: + if isinstance(attributes, Mapping): + resource_attributes.update(attributes) + else: + # Backward compatibility: before `attributes` was a dedicated parameter, it was + # only reachable via **kwargs — `create_resource(attributes=)` set a + # literal resource attribute named "attributes". Preserve that call shape for any + # non-mapping value instead of raising when `dict.update()` rejects it. + resource_attributes["attributes"] = attributes resource_attributes.update(kwargs) if service_name is None: diff --git a/python/packages/core/tests/core/test_observability.py b/python/packages/core/tests/core/test_observability.py index 04dcda77f31..c7f9554aebe 100644 --- a/python/packages/core/tests/core/test_observability.py +++ b/python/packages/core/tests/core/test_observability.py @@ -1181,7 +1181,11 @@ def test_get_exporters_from_env_signal_specific_env_wins_over_param(monkeypatch) from agent_framework import observability monkeypatch.setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://traces-env:4317") - for key in ("OTEL_EXPORTER_OTLP_ENDPOINT", "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT"): + for key in ( + "OTEL_EXPORTER_OTLP_ENDPOINT", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", + "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", + ): monkeypatch.delenv(key, raising=False) with patch.object(observability, "_create_otlp_exporters", return_value=[]) as create: @@ -1218,6 +1222,141 @@ def test_configure_otel_providers_otlp_params(monkeypatch): assert kwargs["compression"] == "deflate" +def test_get_exporters_from_env_passes_empty_headers_through_not_none(monkeypatch): + """An explicit empty otlp_headers={} override must be forwarded as {}, not collapsed to None. + + Regression test for a PR review comment: collapsing an authoritatively-resolved-but-empty + headers dict to None before calling `_create_otlp_exporters()` let the OTLP exporter + constructors fall back to reading OTEL_EXPORTER_OTLP_HEADERS themselves, silently + reattaching an environment-configured credential the caller explicitly tried to suppress. + """ + from unittest.mock import patch + + from agent_framework import observability + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-secret") + for key in ( + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", + "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", + "OTEL_EXPORTER_OTLP_TRACES_HEADERS", + "OTEL_EXPORTER_OTLP_METRICS_HEADERS", + "OTEL_EXPORTER_OTLP_LOGS_HEADERS", + ): + monkeypatch.delenv(key, raising=False) + + with patch.object(observability, "_create_otlp_exporters", return_value=[]) as create: + observability._get_exporters_from_env(endpoint="http://collector:4317", headers={}) + + kwargs = create.call_args.kwargs + # Explicitly empty, not None -- must not be collapsed, or the exporter would re-read + # OTEL_EXPORTER_OTLP_HEADERS itself and resurrect "Authorization: Bearer env-secret". + assert kwargs["traces_headers"] == {} + assert kwargs["metrics_headers"] == {} + assert kwargs["logs_headers"] == {} + + +def test_construct_otlp_exporter_shields_headers_env_for_resolved_headers(monkeypatch): + """When headers were authoritatively resolved (even to {}), construction must not let the + exporter class see OTEL_EXPORTER_OTLP_HEADERS -- an empty dict is just as falsy to the SDK's + own `headers or environ.get(...)` fallback as None, so the env var must be hidden instead. + """ + import os + + from agent_framework.observability import _construct_otlp_exporter + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-secret") + seen: dict[str, str | None] = {} + + class FakeExporter: + def __init__(self, endpoint: str, headers: dict | None, timeout: float | None, compression: object) -> None: + seen["header_env_during_construction"] = os.environ.get("OTEL_EXPORTER_OTLP_HEADERS") + self.endpoint = endpoint + self.headers = headers + + exporter = _construct_otlp_exporter( + FakeExporter, "http://collector:4317", {}, None, None, "OTEL_EXPORTER_OTLP_HEADERS" + ) + + assert exporter.headers == {} + # The env var was hidden from the exporter class during __init__... + assert seen["header_env_during_construction"] is None + # ...and restored immediately afterward. + assert os.environ.get("OTEL_EXPORTER_OTLP_HEADERS") == "Authorization=Bearer env-secret" + + +def test_construct_otlp_exporter_does_not_shield_when_no_headers_resolved(monkeypatch): + """When the caller has no opinion on headers (headers=None), the exporter must be left free + to read OTEL_EXPORTER_OTLP_HEADERS itself, preserving prior behavior for callers (like the + vs_code_extension_port path) that never pass headers to `_create_otlp_exporters()` at all. + """ + import os + + from agent_framework.observability import _construct_otlp_exporter + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-secret") + seen: dict[str, str | None] = {} + + class FakeExporter: + def __init__(self, endpoint: str, headers: dict | None, timeout: float | None, compression: object) -> None: + seen["header_env_during_construction"] = os.environ.get("OTEL_EXPORTER_OTLP_HEADERS") + + _construct_otlp_exporter(FakeExporter, "http://collector:4317", None, None, None, "OTEL_EXPORTER_OTLP_HEADERS") + + assert seen["header_env_during_construction"] == "Authorization=Bearer env-secret" + + +def test_get_exporters_from_env_withholds_programmatic_headers_on_origin_mismatch(monkeypatch): + """Programmatic otlp_headers must not follow a signal to a different-origin endpoint. + + Regression test for a PR review comment: if OTEL_EXPORTER_OTLP_TRACES_ENDPOINT happens to + point at a different host than the programmatic otlp_endpoint, the programmatic headers + (e.g. an Authorization credential) must not be sent to that other host. + """ + from unittest.mock import patch + + from agent_framework import observability + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://other-host:4317") + for key in ("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT"): + monkeypatch.delenv(key, raising=False) + + with patch.object(observability, "_create_otlp_exporters", return_value=[]) as create: + observability._get_exporters_from_env( + endpoint="http://my-collector:4317", + headers={"Authorization": "Bearer secret"}, + ) + + kwargs = create.call_args.kwargs + # traces uses a different origin (other-host) -- the programmatic credential is withheld. + assert kwargs["traces_headers"] == {} + # metrics/logs still use the programmatic base endpoint's origin -- credential is attached. + assert kwargs["metrics_headers"] == {"Authorization": "Bearer secret"} + assert kwargs["logs_headers"] == {"Authorization": "Bearer secret"} + + +def test_get_exporters_from_env_env_only_headers_unaffected_by_origin_guard(monkeypatch): + """The origin-mismatch guard only applies to the new programmatic otlp_endpoint/otlp_headers + parameters; pure env-var-driven configuration keeps its existing, spec-conformant behavior of + applying OTEL_EXPORTER_OTLP_HEADERS to every signal regardless of which endpoint it uses. + """ + from unittest.mock import patch + + from agent_framework import observability + + monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://my-collector:4317") + monkeypatch.setenv("OTEL_EXPORTER_OTLP_HEADERS", "Authorization=Bearer env-secret") + monkeypatch.setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://other-host:4317") + for key in ("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT"): + monkeypatch.delenv(key, raising=False) + + with patch.object(observability, "_create_otlp_exporters", return_value=[]) as create: + observability._get_exporters_from_env() + + kwargs = create.call_args.kwargs + assert kwargs["traces_headers"] == {"Authorization": "Bearer env-secret"} + + # region Test create_resource @@ -1321,6 +1460,34 @@ def test_create_resource_attributes_param_avoids_keyword_collision(monkeypatch): assert resource.attributes["env_file_path"] == "from-dict" +def test_create_resource_attributes_param_backward_compatible_non_mapping(): + """create_resource(attributes=) must keep its pre-existing meaning. + + Regression test for a PR review comment: before `attributes` was a dedicated parameter, it + was only reachable through **kwargs, so `create_resource(attributes="some_value")` set a + literal resource attribute named "attributes" with that string value. Now that `attributes` + is a named parameter documented to accept a mapping, a non-mapping value must not be handed + to `dict.update()` (which would raise `ValueError: dictionary update sequence element #0 has + length 1; 2 is required` for a string) -- the old call shape must keep working unchanged. + """ + from agent_framework.observability import create_resource + + resource = create_resource(attributes="some_value") + + assert resource.attributes["attributes"] == "some_value" + + +def test_create_resource_attributes_param_mapping_form(): + """create_resource(attributes={...}) merges the mapping's entries as resource attributes.""" + from agent_framework.observability import create_resource + + resource = create_resource(attributes={"deployment_environment": "production", "team": "platform"}) + + assert resource.attributes["deployment_environment"] == "production" + assert resource.attributes["team"] == "platform" + assert "attributes" not in resource.attributes + + # region Test _create_otlp_exporters From ffa19e92b207a11899def67425b75d71c42dfbdd Mon Sep 17 00:00:00 2001 From: droideronline Date: Thu, 20 Aug 2026 12:08:30 +0530 Subject: [PATCH 4/4] Python: Fix failing CI checks (pyupgrade, pyright) on OTel config PR - pyupgrade: simplify _shield_env's return annotation from Generator[None, None, None] to Generator[None], matching the codebase's existing style (py310-plus target). - pyright: cast the isinstance(attributes, Mapping)-narrowed value to Mapping[str, Any] before calling dict.update(), since attributes' declared type (Mapping[str, Any] | Any) collapses under Any and pyright otherwise narrows a bare Mapping isinstance check to Mapping[Unknown, Unknown], triggering reportUnknownArgumentType. Verified locally: pyupgrade --py310-plus is a no-op, pyright reports 0 errors on observability.py (with the grpc/http OTLP exporter packages installed, matching CI), ruff check/format clean, and all 230 tests in test_observability.py still pass. --- python/packages/core/agent_framework/observability.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index 0953e0f9378..cae550ffbd7 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -404,7 +404,7 @@ def _parse_headers(header_str: str) -> dict[str, str]: @contextlib.contextmanager -def _shield_env(*names: str) -> Generator[None, None, None]: +def _shield_env(*names: str) -> Generator[None]: """Temporarily hide the given environment variables. The raw OTLP exporter constructors (grpc and http) fall back to reading @@ -914,7 +914,7 @@ def create_resource( if attributes is not None: if isinstance(attributes, Mapping): - resource_attributes.update(attributes) + resource_attributes.update(cast("Mapping[str, Any]", attributes)) else: # Backward compatibility: before `attributes` was a dedicated parameter, it was # only reachable via **kwargs — `create_resource(attributes=)` set a