opentelemetry-otlp: '/v1/traces' is not appended to endpoint for http exporter

[bouk]

The HTTP exporter doesn't add /v1/traces to the 'endpoint' specified in the config:

opentelemetry-rust/opentelemetry-otlp/src/span.rs

Lines 404 to 419 in 592e77a

	/// Builds a new span exporter with the given configuration
	#[cfg(feature = "http-proto")]
	pub fn new_http(config: ExportConfig, http_config: HttpConfig) -> Result<Self, crate::Error> {
	let url: Uri = config
	.endpoint
	.parse()
	.map_err::<crate::Error, _>(Into::into)?;
	Ok(SpanExporter::Http {
	trace_exporter: http_config.client,
	timeout: config.timeout,
	collector_endpoint: url,
	headers: http_config.headers,
	})
	}
	}

This seems like an oversight because this is done in other places:

opentelemetry-rust/opentelemetry-otlp/src/span.rs

Lines 319 to 328 in 592e77a

	/// Builds a new span exporter with the given configuration.
	#[cfg(feature = "grpc-tonic")]
	pub fn new_tonic(
	config: ExportConfig,
	tonic_config: TonicConfig,
	) -> Result<Self, crate::Error> {
	let endpoint_str = match std::env::var(OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) {
	Ok(val) => val,
	Err(_) => format!("{}{}", config.endpoint, "/v1/traces"),
	};

https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/src/metric.rs#L235-L245

[djc]

Thanks for reporting, would you be able to submit a PR to improve on this?

[jaymell]

I believe this issue was fixed by #1210? As a result, #1056 can probably be closed as well.

I just went on a deep dive looking to see whether this had been addressed in later versions of the crate 😅. Since it has, it might be good to marked it as fixed.

[jaymell]

Actually to be clear, while this was fixed, we've actually found it to have a significant bug 😭:

    format!("{provided_or_default_endpoint}{signal_endpoint_path}")
        .parse()
        .map_err(From::from)

/v1/traces is just added directly to provided_or_default_endpoint, which, if it originated from a Url or Uri, is going to have a trailing slash on it, resulting in something like http://example.com//v1/traces (notice the double slash on the path), which in most cases will be a 404.

One simple approach here is to strip off the path from provided_or_default_endpoint and re-build the Uri with signal_endpoint_path as the path instead. This would break anybody who supplies a path with the endpoint, but I think that wouldn't comply with spec anyway.