mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix OTLP HTTP base-endpoint losing /v1/{signal} auto-append (#5913)
* Python: Fix OTLP HTTP base-endpoint losing /v1/{signal} auto-append
Per the OTel spec, OTEL_EXPORTER_OTLP_ENDPOINT is a *base* URL for HTTP —
the SDK auto-appends /v1/traces, /v1/metrics, /v1/logs when it reads the
env var directly. Signal-specific endpoint env vars are *full* URLs used
verbatim.
_get_exporters_from_env read the base endpoint and forwarded it as the
constructor ``endpoint=`` argument, which the SDK always treats as a full
signal URL. As a result, with OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
and HTTP protocol, the exporter sent to http://localhost:4318 instead of
http://localhost:4318/v1/traces (and likewise for metrics/logs).
Replicate the spec's auto-append here when falling back to the base
endpoint under HTTP. gRPC behavior is unchanged.
* Python: Fix mypy type errors in OTLP endpoint assignment
Pre-declare traces_endpoint, metrics_endpoint, logs_endpoint as
str | None before the if/else block. Mypy inferred str from the
if-branch f-string assignments and then rejected the str | None
expressions in the else-branch as incompatible.
This commit is contained in:
committed by
GitHub
Unverified
parent
0cf48923cd
commit
a5f355e04a
@@ -498,14 +498,34 @@ def _get_exporters_from_env(
|
||||
# Get base endpoint
|
||||
base_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT")
|
||||
|
||||
# Get signal-specific endpoints (these override base endpoint)
|
||||
traces_endpoint = os.getenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT") or base_endpoint
|
||||
metrics_endpoint = os.getenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT") or base_endpoint
|
||||
logs_endpoint = os.getenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT") or base_endpoint
|
||||
# Get signal-specific endpoints (these override base endpoint 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()
|
||||
|
||||
# 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
|
||||
# signal-specific endpoint env vars are *full* URLs used verbatim. Because we read
|
||||
# the env vars here and forward them as the ``endpoint=`` constructor argument
|
||||
# (which the SDK always treats as a full URL), we must replicate the auto-append
|
||||
# ourselves for HTTP when falling back to the base endpoint. For gRPC, the base
|
||||
# endpoint is used as-is.
|
||||
traces_endpoint: str | None
|
||||
metrics_endpoint: str | None
|
||||
logs_endpoint: str | None
|
||||
if 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"
|
||||
logs_endpoint = logs_endpoint_specific or f"{base_for_http}/v1/logs"
|
||||
else:
|
||||
traces_endpoint = traces_endpoint_specific or base_endpoint
|
||||
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)
|
||||
|
||||
@@ -761,6 +761,115 @@ def test_get_exporters_from_env_missing_grpc_dependency(monkeypatch):
|
||||
_get_exporters_from_env()
|
||||
|
||||
|
||||
# region Test OTLP endpoint computation (base-URL auto-append for HTTP)
|
||||
|
||||
|
||||
def test_get_exporters_from_env_http_base_endpoint_appends_signal_paths(monkeypatch):
|
||||
"""OTEL_EXPORTER_OTLP_ENDPOINT is a base URL for HTTP; SDK auto-appends
|
||||
/v1/{traces,metrics,logs}. Because we read the env var and forward it as the
|
||||
constructor ``endpoint=`` arg (which the SDK treats as a full URL), we must
|
||||
replicate the auto-append ourselves.
|
||||
"""
|
||||
from unittest.mock import patch
|
||||
|
||||
from agent_framework import observability
|
||||
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318")
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf")
|
||||
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()
|
||||
|
||||
kwargs = create.call_args.kwargs
|
||||
assert kwargs["protocol"] == "http/protobuf"
|
||||
assert kwargs["traces_endpoint"] == "http://localhost:4318/v1/traces"
|
||||
assert kwargs["metrics_endpoint"] == "http://localhost:4318/v1/metrics"
|
||||
assert kwargs["logs_endpoint"] == "http://localhost:4318/v1/logs"
|
||||
|
||||
|
||||
def test_get_exporters_from_env_http_base_endpoint_trailing_slash(monkeypatch):
|
||||
"""A trailing slash on the base endpoint should not produce a doubled slash."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from agent_framework import observability
|
||||
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318/")
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf")
|
||||
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()
|
||||
|
||||
kwargs = create.call_args.kwargs
|
||||
assert kwargs["traces_endpoint"] == "http://localhost:4318/v1/traces"
|
||||
assert kwargs["metrics_endpoint"] == "http://localhost:4318/v1/metrics"
|
||||
assert kwargs["logs_endpoint"] == "http://localhost:4318/v1/logs"
|
||||
|
||||
|
||||
def test_get_exporters_from_env_http_signal_specific_used_verbatim(monkeypatch):
|
||||
"""Signal-specific endpoint env vars are full URLs and must be used verbatim,
|
||||
even when a base endpoint is also set.
|
||||
"""
|
||||
from unittest.mock import patch
|
||||
|
||||
from agent_framework import observability
|
||||
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318")
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://traces.example.com/custom/path")
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf")
|
||||
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
|
||||
# Signal-specific is verbatim — no path appended
|
||||
assert kwargs["traces_endpoint"] == "http://traces.example.com/custom/path"
|
||||
# Others fall back to base, with path appended
|
||||
assert kwargs["metrics_endpoint"] == "http://localhost:4318/v1/metrics"
|
||||
assert kwargs["logs_endpoint"] == "http://localhost:4318/v1/logs"
|
||||
|
||||
|
||||
def test_get_exporters_from_env_grpc_base_endpoint_unchanged(monkeypatch):
|
||||
"""For gRPC, the base endpoint applies to all signals as-is (no path append)."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from agent_framework import observability
|
||||
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317")
|
||||
monkeypatch.setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc")
|
||||
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()
|
||||
|
||||
kwargs = create.call_args.kwargs
|
||||
assert kwargs["protocol"] == "grpc"
|
||||
assert kwargs["traces_endpoint"] == "http://localhost:4317"
|
||||
assert kwargs["metrics_endpoint"] == "http://localhost:4317"
|
||||
assert kwargs["logs_endpoint"] == "http://localhost:4317"
|
||||
|
||||
|
||||
# region Test create_resource
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user