opentelemetry-exporter-otlp-proto-http: auto-append signal path when base URL is passed as endpoint#5273
Conversation
…ndpoint When `endpoint` is passed to OTLPSpanExporter, OTLPMetricExporter, or OTLPLogExporter without a signal-specific path (empty path or just `/`), treat it as a base URL and append /v1/traces, /v1/metrics, or /v1/logs respectively — consistent with how OTEL_EXPORTER_OTLP_ENDPOINT behaves. This makes switching from gRPC to HTTP exporters easier: users can pass the same base URL style (e.g. http://host:4318) without having to manually construct signal-specific URLs. Assisted-by: Claude Sonnet 4.6
Assisted-by: Claude Sonnet 4.6
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cdec295aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replace _is_base_endpoint + string-concatenation with _resolve_endpoint_to_signal that uses urlparse/urlunparse, so that an endpoint like http://host:4318?tenant=acme becomes http://host:4318/v1/traces?tenant=acme rather than the malformed http://host:4318?tenant=acme/v1/traces. Assisted-by: Claude Sonnet 4.6
|
AFAIK when an |
Worth noting that the gRPC exporter also doesn't use the constructor endpoint as-is. It silently strips the scheme and path, keeping only host:port. So from the user's perspective, neither transport treats the parameter verbatim today. This PR applies the same normalization idea to HTTP: if no signal path is present, treat it as a base URL and append the correct one. If a path is already there, it's left untouched. The main use case is switching from gRPC to HTTP without having to rethink the endpoint config. With gRPC you just pass http://host:4317 and it works, with HTTP you'd have to manually split it into three signal-specific URLs. |
Taking a look at other SDK implementations, it looks like Java and JavaScript require the full endpoint being passed in. While not an explicit requirement, the preference is to keep the APIs across languages similar unless there is a strong reason to deviate. My other concern here is that (while likely very rare) there may be users which have a signal specific listener configured with no HTTP path. With these changes there would be no way to configure the endpoint anymore to export without a path being appended. I'd potentially be open to adding an optional |
| Uses proper URL manipulation so query strings and fragments are preserved. | ||
| If the endpoint already has a path other than '/', it is returned unchanged. | ||
| """ | ||
| parsed = urlparse(endpoint) |
There was a problem hiding this comment.
Should probably embed this in a try-catch to avoid crashes.
| If the endpoint already has a path other than '/', it is returned unchanged. | ||
| """ | ||
| parsed = urlparse(endpoint) | ||
| if not parsed.path or parsed.path == "/": |
There was a problem hiding this comment.
Could use hasattr to check if path exists or not.
| ) | ||
| self.assertIsInstance(exporter._session, requests.Session) | ||
|
|
||
| def test_endpoint_base_url_no_path(self): |
There was a problem hiding this comment.
Should add tests for some malformed, invalid urls
3 participants
Description
When switching from the gRPC OTLP exporter to the HTTP OTLP exporter, users expect to provide a single base URL (e.g.
http://collector:4318) and have each exporter automatically route to the correct signal endpoint. With gRPC this works because routing is determined by the service stub — the URL path is irrelevant. With HTTP, the path matters.Currently, passing
endpointdirectly toOTLPSpanExporter,OTLPMetricExporter, orOTLPLogExporteruses the value as-is, with no signal path appended:But the
OTEL_EXPORTER_OTLP_ENDPOINTenvironment variable already auto-appends signal paths. This PR makes the programmaticendpoint=parameter behave the same way.Change
If
endpointis provided without a meaningful path (empty path or just/), it is treated as a base URL and the signal-specific path is appended automatically:OTLPSpanExporter(endpoint="http://host:4318")→http://host:4318/v1/tracesOTLPSpanExporter(endpoint="http://host:4318/")→http://host:4318/v1/tracesOTLPSpanExporter(endpoint="http://host:4318/v1/traces")→ unchanged (backward compatible)OTLPSpanExporter(endpoint="http://host:4318/custom")→ unchanged (explicit custom path)The same logic applies to
OTLPMetricExporter(/v1/metrics) andOTLPLogExporter(/v1/logs).A shared helper
_is_base_endpoint()is added to the_commonmodule to avoid duplicating the detection logic.Testing