From 55979b97f7ad99420903da04d4143234284fd989 Mon Sep 17 00:00:00 2001 From: bachgarash Date: Wed, 3 Dec 2025 13:33:53 +0200 Subject: [PATCH] fix: sdk should add /v1/traces and /v1/metrics endpoints implicitly This PR fixes #4412 which should add metric and trace endpoints implicitly when endpoint argument is passed. OTel sdk general config https://opentelemetry.io/docs/languages/sdk-configuration/otlp-exporter/#otel_exporter_otlp_endpoint --- CHANGELOG.md | 5 ++ .../otlp/proto/http/_log_exporter/__init__.py | 16 +++-- .../proto/http/metric_exporter/__init__.py | 16 +++-- .../proto/http/trace_exporter/__init__.py | 16 +++-- .../metrics/test_otlp_metrics_exporter.py | 58 ++++++++++++++++++- .../tests/test_proto_log_exporter.py | 56 +++++++++++++++++- .../tests/test_proto_span_exporter.py | 56 +++++++++++++++++- 7 files changed, 204 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c7eee8a29..38b817e845 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- `opentelemetry-exporter-otlp-proto-http`: Fix HTTP OTLP exporters to automatically append signal path + (`/v1/traces`, `/v1/metrics`, `/v1/logs`) to user-provided endpoints per the OpenTelemetry specification. + Previously, users had to manually include these paths in their endpoint configuration. + ([#3200](https://github.com/open-telemetry/opentelemetry-python/issues/3200)) + ([#4412](https://github.com/open-telemetry/opentelemetry-python/issues/4412)) - `opentelemetry-api`: Convert objects of any type other than AnyValue in attributes to string to be exportable ([#4808](https://github.com/open-telemetry/opentelemetry-python/pull/4808)) - docs: Added sqlcommenter example 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 b120a2cca4..ab6d464141 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 @@ -85,11 +85,16 @@ def __init__( ): 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) - ), + OTEL_EXPORTER_OTLP_LOGS_ENDPOINT ) + if self._endpoint: + # User-provided endpoint or signal-specific env var - append path if not present + self._endpoint = _append_logs_path(self._endpoint) + else: + # Use general endpoint with path appended per spec + self._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, @@ -240,6 +245,9 @@ def _compression_from_env() -> Compression: def _append_logs_path(endpoint: str) -> str: + # Don't append path if it's already present + if endpoint.endswith(f"/{DEFAULT_LOGS_EXPORT_PATH}"): + return endpoint if endpoint.endswith("/"): return endpoint + DEFAULT_LOGS_EXPORT_PATH return endpoint + f"/{DEFAULT_LOGS_EXPORT_PATH}" 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 c6d657e7ae..25815c1bb7 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 @@ -125,11 +125,16 @@ def __init__( ): 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) - ), + OTEL_EXPORTER_OTLP_METRICS_ENDPOINT ) + if self._endpoint: + # User-provided endpoint or signal-specific env var - append path if not present + self._endpoint = _append_metrics_path(self._endpoint) + else: + # Use general endpoint with path appended per spec + self._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), @@ -300,6 +305,9 @@ def _compression_from_env() -> Compression: def _append_metrics_path(endpoint: str) -> str: + # Don't append path if it's already present + if endpoint.endswith(f"/{DEFAULT_METRICS_EXPORT_PATH}"): + return endpoint if endpoint.endswith("/"): return endpoint + DEFAULT_METRICS_EXPORT_PATH return endpoint + f"/{DEFAULT_METRICS_EXPORT_PATH}" 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 055e829dab..e8ce8be8b4 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 @@ -81,11 +81,16 @@ def __init__( ): 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) - ), + OTEL_EXPORTER_OTLP_TRACES_ENDPOINT ) + if self._endpoint: + # User-provided endpoint or signal-specific env var - append path if not present + self._endpoint = _append_trace_path(self._endpoint) + else: + # Use general endpoint with path appended per spec + self._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), @@ -233,6 +238,9 @@ def _compression_from_env() -> Compression: def _append_trace_path(endpoint: str) -> str: + # Don't append path if it's already present + if endpoint.endswith(f"/{DEFAULT_TRACES_EXPORT_PATH}"): + return endpoint if endpoint.endswith("/"): return endpoint + DEFAULT_TRACES_EXPORT_PATH return endpoint + f"/{DEFAULT_TRACES_EXPORT_PATH}" 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 eca1aed5d9..b1572e0779 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 @@ -87,7 +87,7 @@ OS_ENV_TIMEOUT = "30" -# pylint: disable=protected-access +# pylint: disable=protected-access,too-many-public-methods class TestOTLPMetricExporter(TestCase): def setUp(self): self.metrics = { @@ -170,7 +170,10 @@ def f(): ) exporter = OTLPMetricExporter() - self.assertEqual(exporter._endpoint, "https://metrics.endpoint.env") + self.assertEqual( + exporter._endpoint, + f"https://metrics.endpoint.env/{DEFAULT_METRICS_EXPORT_PATH}", + ) self.assertEqual(exporter._certificate_file, "metrics/certificate.env") self.assertEqual( exporter._client_certificate_file, "metrics/client-cert.pem" @@ -222,7 +225,10 @@ def test_exporter_constructor_take_priority(self): session=Session(), ) - self.assertEqual(exporter._endpoint, "example.com/1234") + self.assertEqual( + exporter._endpoint, + f"example.com/1234/{DEFAULT_METRICS_EXPORT_PATH}", + ) self.assertEqual(exporter._certificate_file, "path/to/service.crt") self.assertEqual( exporter._client_certificate_file, "path/to/client-cert.pem" @@ -290,6 +296,52 @@ def test_exporter_env_endpoint_with_slash(self): OS_ENV_ENDPOINT + f"/{DEFAULT_METRICS_EXPORT_PATH}", ) + def test_exporter_constructor_endpoint_with_path_appended(self): + """Test that path is appended to user-provided endpoint.""" + exporter = OTLPMetricExporter( + endpoint="http://collector.example.com:4318" + ) + self.assertEqual( + exporter._endpoint, + f"http://collector.example.com:4318/{DEFAULT_METRICS_EXPORT_PATH}", + ) + + def test_exporter_constructor_endpoint_no_duplicate_path(self): + """Test that path is not duplicated if already present.""" + exporter = OTLPMetricExporter( + endpoint=f"http://collector.example.com:4318/{DEFAULT_METRICS_EXPORT_PATH}" + ) + self.assertEqual( + exporter._endpoint, + f"http://collector.example.com:4318/{DEFAULT_METRICS_EXPORT_PATH}", + ) + + @patch.dict( + "os.environ", + {OTEL_EXPORTER_OTLP_METRICS_ENDPOINT: "http://metrics.collector:4318"}, + ) + def test_exporter_signal_specific_env_endpoint_with_path_appended(self): + """Test that path is appended to signal-specific endpoint.""" + exporter = OTLPMetricExporter() + self.assertEqual( + exporter._endpoint, + f"http://metrics.collector:4318/{DEFAULT_METRICS_EXPORT_PATH}", + ) + + @patch.dict( + "os.environ", + { + OTEL_EXPORTER_OTLP_METRICS_ENDPOINT: f"http://metrics.collector:4318/{DEFAULT_METRICS_EXPORT_PATH}" + }, + ) + def test_exporter_signal_specific_env_endpoint_no_duplicate_path(self): + """Test that path is not duplicated when signal-specific endpoint already has path.""" + exporter = OTLPMetricExporter() + self.assertEqual( + exporter._endpoint, + f"http://metrics.collector:4318/{DEFAULT_METRICS_EXPORT_PATH}", + ) + @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 31e824a980..46156beb73 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 @@ -132,7 +132,10 @@ def f(): ) exporter = OTLPLogExporter() - self.assertEqual(exporter._endpoint, "https://logs.endpoint.env") + self.assertEqual( + exporter._endpoint, + f"https://logs.endpoint.env/{DEFAULT_LOGS_EXPORT_PATH}", + ) self.assertEqual(exporter._certificate_file, "logs/certificate.env") self.assertEqual( exporter._client_certificate_file, "logs/client-cert.pem" @@ -214,7 +217,10 @@ def test_exporter_constructor_take_priority(self): session=sess(), ) - self.assertEqual(exporter._endpoint, "endpoint.local:69/logs") + self.assertEqual( + exporter._endpoint, + f"endpoint.local:69/logs/{DEFAULT_LOGS_EXPORT_PATH}", + ) self.assertEqual(exporter._certificate_file, "/hello.crt") self.assertEqual(exporter._client_certificate_file, "/client-cert.pem") self.assertEqual(exporter._client_key_file, "/client-key.pem") @@ -261,6 +267,52 @@ def test_exporter_env(self): ) self.assertIsInstance(exporter._session, requests.Session) + def test_exporter_constructor_endpoint_with_path_appended(self): + """Test that path is appended to user-provided endpoint.""" + exporter = OTLPLogExporter( + endpoint="http://collector.example.com:4318" + ) + self.assertEqual( + exporter._endpoint, + f"http://collector.example.com:4318/{DEFAULT_LOGS_EXPORT_PATH}", + ) + + def test_exporter_constructor_endpoint_no_duplicate_path(self): + """Test that path is not duplicated if already present.""" + exporter = OTLPLogExporter( + endpoint=f"http://collector.example.com:4318/{DEFAULT_LOGS_EXPORT_PATH}" + ) + self.assertEqual( + exporter._endpoint, + f"http://collector.example.com:4318/{DEFAULT_LOGS_EXPORT_PATH}", + ) + + @patch.dict( + "os.environ", + {OTEL_EXPORTER_OTLP_LOGS_ENDPOINT: "http://logs.collector:4318"}, + ) + def test_exporter_signal_specific_env_endpoint_with_path_appended(self): + """Test that path is appended to signal-specific endpoint.""" + exporter = OTLPLogExporter() + self.assertEqual( + exporter._endpoint, + f"http://logs.collector:4318/{DEFAULT_LOGS_EXPORT_PATH}", + ) + + @patch.dict( + "os.environ", + { + OTEL_EXPORTER_OTLP_LOGS_ENDPOINT: f"http://logs.collector:4318/{DEFAULT_LOGS_EXPORT_PATH}" + }, + ) + def test_exporter_signal_specific_env_endpoint_no_duplicate_path(self): + """Test that path is not duplicated when signal-specific endpoint already has path.""" + exporter = OTLPLogExporter() + self.assertEqual( + exporter._endpoint, + f"http://logs.collector:4318/{DEFAULT_LOGS_EXPORT_PATH}", + ) + @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 10dcb1a9e0..a36e9222cf 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 @@ -127,7 +127,10 @@ def f(): ) exporter = OTLPSpanExporter() - self.assertEqual(exporter._endpoint, "https://traces.endpoint.env") + self.assertEqual( + exporter._endpoint, + f"https://traces.endpoint.env/{DEFAULT_TRACES_EXPORT_PATH}", + ) self.assertEqual(exporter._certificate_file, "traces/certificate.env") self.assertEqual( exporter._client_certificate_file, "traces/client-cert.pem" @@ -180,7 +183,10 @@ def test_exporter_constructor_take_priority(self): session=requests.Session(), ) - self.assertEqual(exporter._endpoint, "example.com/1234") + self.assertEqual( + exporter._endpoint, + f"example.com/1234/{DEFAULT_TRACES_EXPORT_PATH}", + ) self.assertEqual(exporter._certificate_file, "path/to/service.crt") self.assertEqual( exporter._client_certificate_file, "path/to/client-cert.pem" @@ -248,6 +254,52 @@ def test_exporter_env_endpoint_with_slash(self): OS_ENV_ENDPOINT + f"/{DEFAULT_TRACES_EXPORT_PATH}", ) + def test_exporter_constructor_endpoint_with_path_appended(self): + """Test that path is appended to user-provided endpoint.""" + exporter = OTLPSpanExporter( + endpoint="http://collector.example.com:4318" + ) + self.assertEqual( + exporter._endpoint, + f"http://collector.example.com:4318/{DEFAULT_TRACES_EXPORT_PATH}", + ) + + def test_exporter_constructor_endpoint_no_duplicate_path(self): + """Test that path is not duplicated if already present.""" + exporter = OTLPSpanExporter( + endpoint=f"http://collector.example.com:4318/{DEFAULT_TRACES_EXPORT_PATH}" + ) + self.assertEqual( + exporter._endpoint, + f"http://collector.example.com:4318/{DEFAULT_TRACES_EXPORT_PATH}", + ) + + @patch.dict( + "os.environ", + {OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: "http://traces.collector:4318"}, + ) + def test_exporter_signal_specific_env_endpoint_with_path_appended(self): + """Test that path is appended to signal-specific endpoint.""" + exporter = OTLPSpanExporter() + self.assertEqual( + exporter._endpoint, + f"http://traces.collector:4318/{DEFAULT_TRACES_EXPORT_PATH}", + ) + + @patch.dict( + "os.environ", + { + OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: f"http://traces.collector:4318/{DEFAULT_TRACES_EXPORT_PATH}" + }, + ) + def test_exporter_signal_specific_env_endpoint_no_duplicate_path(self): + """Test that path is not duplicated when signal-specific endpoint already has path.""" + exporter = OTLPSpanExporter() + self.assertEqual( + exporter._endpoint, + f"http://traces.collector:4318/{DEFAULT_TRACES_EXPORT_PATH}", + ) + @patch.dict( "os.environ", {