opentelemetry-exporter-otlp-proto-http: refactor to use shared opentelemetry-exporter-otlp-common and opentelemetry-exporter-http-transport packages#5389
Conversation
…lemetry-exporter-otlp-common and opentelemetry-exporter-http-transport packages
There was a problem hiding this comment.
Pull request overview
Refactors opentelemetry-exporter-otlp-proto-http to delegate HTTP transport, retry/backoff, compression, and configuration resolution to the shared opentelemetry-exporter-otlp-common and opentelemetry-exporter-http-transport packages, and updates tests/build wiring accordingly.
Changes:
- Replaces per-exporter HTTP/retry/compression logic with shared
opentelemetry.exporter.otlp.common._http.OTLPHTTPClientand new helpers inproto.http._common. - Updates
opentelemetry-exporter-otlp-proto-httpdependencies/extras sorequestsis optional (via an extra) while default transport uses urllib3. - Reworks unit/functional tests to validate behavior via transport-level request interception and deterministic retry timing.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds locked workspace deps for opentelemetry-exporter-otlp-common and opentelemetry-exporter-http-transport; moves requests behind an extra. |
| tox.ini | Ensures tox envs install the new shared exporter packages. |
| tests/opentelemetry-docker-tests/tests/otlpexporter/test_otlp_http_exporter_functional.py | Updates functional test helper to hook the new _export_batch path. |
| pyproject.toml | Adds opentelemetry-exporter-otlp-common to workspace dependencies. |
| exporter/opentelemetry-exporter-otlp/test-requirements.txt | Adds editable installs for new shared packages + transport extras for OTLP exporter tests. |
| exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py | Rewrites span exporter tests for shared transport/client behavior (mocket-based). |
| exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py | Rewrites log exporter tests for shared transport/client behavior (mocket-based). |
| exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py | Updates metrics exporter tests for shared transport/client behavior and batch splitting. |
| exporter/opentelemetry-exporter-otlp-proto-http/tests/test_common.py | Adds targeted unit tests for new _common resolution/build helpers. |
| exporter/opentelemetry-exporter-otlp-proto-http/tests/init.py | Adds a shared _mock_clock helper for deterministic retry/backoff testing. |
| exporter/opentelemetry-exporter-otlp-proto-http/test-requirements.txt | Adds mocket and new editable workspace dependencies for this package’s tests. |
| exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/init.py | Switches span exporter implementation to shared OTLP HTTP client + common config helpers. |
| exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/init.py | Switches metrics exporter implementation to shared OTLP HTTP client + common config helpers. |
| exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_log_exporter/init.py | Switches log exporter implementation to shared OTLP HTTP client + common config helpers. |
| exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/init.py | Adds shared endpoint/header/timeout/compression resolution and transport construction helpers. |
| exporter/opentelemetry-exporter-otlp-proto-http/pyproject.toml | Replaces hard dependency on requests with shared packages; adds requests extra. |
| docs/conf.py | Updates intersphinx + nitpick ignores for new types referenced in docs. |
| docs-requirements.txt | Installs opentelemetry-exporter-otlp-proto-http[requests] for doc builds. |
| .changelog/5389.changed | Adds changelog entry for the refactor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _export_batch( | ||
| self, | ||
| export_request: ExportMetricsServiceRequest, | ||
| deadline_sec: float, | ||
| num_items: int, | ||
| ) -> MetricExportResult: |
| if _credential_env: | ||
| # pylint: disable-next=import-outside-toplevel | ||
| import requests # noqa: PLC0415 | ||
|
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| return ( | ||
| RequestsHTTPTransport(verify=verify, cert=cert, session=session) | ||
| if session | ||
| else Urllib3HTTPTransport(verify=verify, cert=cert) |
There was a problem hiding this comment.
Wasn't the default (if no passed in session or credential env var) requests.Session?
There was a problem hiding this comment.
Yes it was, I was thinking we can change the default here to use urllib3. This shouldn't be a breaking change since from the user's perspective exporting is still performed normally over Http respecting all of the parameters passed to the constructor.
| _MAX_RETRYS = 6 | ||
|
|
||
|
|
||
| class OTLPMetricExporter(MetricExporter, OTLPMetricExporterMixin): |
There was a problem hiding this comment.
I'm assuming we will eventually remove OTLPMetricExporterMixin in otlp-proto-common once the similar refactor is done for otlp grpc metrics exporter?
Description
Refactors the
opentelemetry-exporter-otlp-proto-httppackage to use the new sharedopentelemetry-exporter-otlp-commonandopentelemetry-exporter-http-transport packagespackages.Fixes #2990
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Tox
Does This PR Require a Contrib Repo Change?
Checklist: