diff --git a/.changelog/5273.added b/.changelog/5273.added new file mode 100644 index 00000000000..41e9dc44473 --- /dev/null +++ b/.changelog/5273.added @@ -0,0 +1 @@ +`opentelemetry-exporter-otlp-proto-http`: auto-append signal path when a base URL (no path or path `/`) is passed as `endpoint` to HTTP OTLP exporters diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/__init__.py index 57bd7ca065a..af19e4ee3b5 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/__init__.py @@ -3,6 +3,7 @@ from os import environ from typing import Literal +from urllib.parse import urlparse, urlunparse import requests @@ -12,6 +13,24 @@ from opentelemetry.util._importlib_metadata import entry_points +def _resolve_endpoint_to_signal(endpoint: str, signal_path: str) -> str: + """Append signal_path to endpoint if it has no signal-specific path. + + Uses proper URL manipulation so query strings and fragments are preserved. + If the endpoint already has a path other than '/', or cannot be parsed as + a URL, it is returned unchanged. + """ + try: + parsed = urlparse(endpoint) + except ValueError: + return endpoint + path = getattr(parsed, "path", "") + if not path or path == "/": + base = path.rstrip("/") + return urlunparse(parsed._replace(path=f"{base}/{signal_path}")) + return endpoint + + def _is_retryable(resp: requests.Response) -> bool: if resp.status_code == 408: return True diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_log_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_log_exporter/__init__.py index 56906b96501..7dcc57120a1 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_log_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_log_exporter/__init__.py @@ -27,6 +27,7 @@ from opentelemetry.exporter.otlp.proto.http._common import ( _is_retryable, _load_session_from_envvar, + _resolve_endpoint_to_signal, ) from opentelemetry.metrics import MeterProvider from opentelemetry.sdk._logs import ReadableLogRecord @@ -88,12 +89,17 @@ def __init__( meter_provider: MeterProvider | None = None, ): self._shutdown_is_occuring = threading.Event() - self._endpoint = endpoint or environ.get( - OTEL_EXPORTER_OTLP_LOGS_ENDPOINT, - _append_logs_path( - environ.get(OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_ENDPOINT) - ), - ) + if endpoint is not None: + self._endpoint = _resolve_endpoint_to_signal( + endpoint, DEFAULT_LOGS_EXPORT_PATH + ) + else: + self._endpoint = environ.get( + OTEL_EXPORTER_OTLP_LOGS_ENDPOINT, + _append_logs_path( + environ.get(OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_ENDPOINT) + ), + ) # Keeping these as instance variables because they are used in tests self._certificate_file = certificate_file or environ.get( OTEL_EXPORTER_OTLP_LOGS_CERTIFICATE, diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py index eb1e69cfe4f..4abcfc7cce2 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py @@ -41,6 +41,7 @@ from opentelemetry.exporter.otlp.proto.http._common import ( _is_retryable, _load_session_from_envvar, + _resolve_endpoint_to_signal, ) from opentelemetry.metrics import MeterProvider from opentelemetry.proto.collector.metrics.v1.metrics_service_pb2 import ( # noqa: F401 @@ -147,12 +148,17 @@ def __init__( If it is set and the number of data points exceeds the max, the request will be split. """ self._shutdown_in_progress = threading.Event() - self._endpoint = endpoint or environ.get( - OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, - _append_metrics_path( - environ.get(OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_ENDPOINT) - ), - ) + if endpoint is not None: + self._endpoint = _resolve_endpoint_to_signal( + endpoint, DEFAULT_METRICS_EXPORT_PATH + ) + else: + self._endpoint = environ.get( + OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, + _append_metrics_path( + environ.get(OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_ENDPOINT) + ), + ) self._certificate_file = certificate_file or environ.get( OTEL_EXPORTER_OTLP_METRICS_CERTIFICATE, environ.get(OTEL_EXPORTER_OTLP_CERTIFICATE, True), diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py index 2be240103c0..7949df1f0e0 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py @@ -29,6 +29,7 @@ from opentelemetry.exporter.otlp.proto.http._common import ( _is_retryable, _load_session_from_envvar, + _resolve_endpoint_to_signal, ) from opentelemetry.metrics import MeterProvider from opentelemetry.sdk.environment_variables import ( @@ -84,12 +85,17 @@ def __init__( meter_provider: MeterProvider | None = None, ): self._shutdown_in_progress = threading.Event() - self._endpoint = endpoint or environ.get( - OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, - _append_trace_path( - environ.get(OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_ENDPOINT) - ), - ) + if endpoint is not None: + self._endpoint = _resolve_endpoint_to_signal( + endpoint, DEFAULT_TRACES_EXPORT_PATH + ) + else: + self._endpoint = environ.get( + OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, + _append_trace_path( + environ.get(OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_ENDPOINT) + ), + ) self._certificate_file = certificate_file or environ.get( OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE, environ.get(OTEL_EXPORTER_OTLP_CERTIFICATE, True), diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py index 84a11e8ae90..8551f8b1ccf 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py @@ -303,6 +303,30 @@ def test_exporter_env_endpoint_with_slash(self): OS_ENV_ENDPOINT + f"/{DEFAULT_METRICS_EXPORT_PATH}", ) + def test_endpoint_base_url_no_path(self): + exporter = OTLPMetricExporter(endpoint="http://collector:4318") + self.assertEqual( + exporter._endpoint, "http://collector:4318/v1/metrics" + ) + + def test_endpoint_base_url_trailing_slash(self): + exporter = OTLPMetricExporter(endpoint="http://collector:4318/") + self.assertEqual( + exporter._endpoint, "http://collector:4318/v1/metrics" + ) + + def test_endpoint_full_url_unchanged(self): + exporter = OTLPMetricExporter( + endpoint="http://collector:4318/v1/metrics" + ) + self.assertEqual( + exporter._endpoint, "http://collector:4318/v1/metrics" + ) + + def test_endpoint_custom_path_unchanged(self): + exporter = OTLPMetricExporter(endpoint="http://collector:4318/custom") + self.assertEqual(exporter._endpoint, "http://collector:4318/custom") + @patch.dict( "os.environ", { diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py index d7f3592e288..b9f3b3729de 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py @@ -17,6 +17,9 @@ from opentelemetry._logs import LogRecord, SeverityNumber from opentelemetry.exporter.otlp.proto.http import Compression +from opentelemetry.exporter.otlp.proto.http._common import ( + _resolve_endpoint_to_signal, +) from opentelemetry.exporter.otlp.proto.http._log_exporter import ( DEFAULT_COMPRESSION, DEFAULT_ENDPOINT, @@ -259,6 +262,38 @@ def test_exporter_env(self): ) self.assertIsInstance(exporter._session, requests.Session) + def test_endpoint_base_url_no_path(self): + exporter = OTLPLogExporter(endpoint="http://collector:4318") + self.assertEqual(exporter._endpoint, "http://collector:4318/v1/logs") + + def test_endpoint_base_url_trailing_slash(self): + exporter = OTLPLogExporter(endpoint="http://collector:4318/") + self.assertEqual(exporter._endpoint, "http://collector:4318/v1/logs") + + def test_endpoint_full_url_unchanged(self): + exporter = OTLPLogExporter(endpoint="http://collector:4318/v1/logs") + self.assertEqual(exporter._endpoint, "http://collector:4318/v1/logs") + + def test_endpoint_custom_path_unchanged(self): + exporter = OTLPLogExporter(endpoint="http://collector:4318/custom") + self.assertEqual(exporter._endpoint, "http://collector:4318/custom") + + def test_endpoint_unparsable_url_unchanged(self): + # Unclosed IPv6 bracket makes urlparse raise ValueError; the + # helper must pass the endpoint through unchanged. + self.assertEqual( + _resolve_endpoint_to_signal( + "http://[invalid:4318", DEFAULT_LOGS_EXPORT_PATH + ), + "http://[invalid:4318", + ) + + def test_endpoint_schemeless_host_port_unchanged(self): + # Without a scheme, urlparse reads "collector" as the scheme and + # "4318" as the path, so the endpoint is left untouched. + exporter = OTLPLogExporter(endpoint="collector:4318") + self.assertEqual(exporter._endpoint, "collector:4318") + @staticmethod def export_log_and_deserialize(log): with patch("requests.Session.post") as mock_post: diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py index 1580e5a1802..a3b33f5ef74 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py @@ -13,6 +13,9 @@ from requests.models import Response from opentelemetry.exporter.otlp.proto.http import Compression +from opentelemetry.exporter.otlp.proto.http._common import ( + _resolve_endpoint_to_signal, +) from opentelemetry.exporter.otlp.proto.http.trace_exporter import ( DEFAULT_COMPRESSION, DEFAULT_ENDPOINT, @@ -247,6 +250,47 @@ def test_exporter_env_endpoint_with_slash(self): OS_ENV_ENDPOINT + f"/{DEFAULT_TRACES_EXPORT_PATH}", ) + def test_endpoint_base_url_no_path(self): + exporter = OTLPSpanExporter(endpoint="http://collector:4318") + self.assertEqual(exporter._endpoint, "http://collector:4318/v1/traces") + + def test_endpoint_base_url_trailing_slash(self): + exporter = OTLPSpanExporter(endpoint="http://collector:4318/") + self.assertEqual(exporter._endpoint, "http://collector:4318/v1/traces") + + def test_endpoint_full_url_unchanged(self): + exporter = OTLPSpanExporter(endpoint="http://collector:4318/v1/traces") + self.assertEqual(exporter._endpoint, "http://collector:4318/v1/traces") + + def test_endpoint_custom_path_unchanged(self): + exporter = OTLPSpanExporter(endpoint="http://collector:4318/custom") + self.assertEqual(exporter._endpoint, "http://collector:4318/custom") + + def test_endpoint_base_url_with_query_string(self): + exporter = OTLPSpanExporter( + endpoint="http://collector:4318?tenant=acme" + ) + self.assertEqual( + exporter._endpoint, + "http://collector:4318/v1/traces?tenant=acme", + ) + + def test_endpoint_unparsable_url_unchanged(self): + # Unclosed IPv6 bracket makes urlparse raise ValueError; the + # helper must pass the endpoint through unchanged. + self.assertEqual( + _resolve_endpoint_to_signal( + "http://[invalid:4318", DEFAULT_TRACES_EXPORT_PATH + ), + "http://[invalid:4318", + ) + + def test_endpoint_schemeless_host_port_unchanged(self): + # Without a scheme, urlparse reads "collector" as the scheme and + # "4318" as the path, so the endpoint is left untouched. + exporter = OTLPSpanExporter(endpoint="collector:4318") + self.assertEqual(exporter._endpoint, "collector:4318") + @patch.dict( "os.environ", {