opentelemetry-otlp exporter with http-proto: should it error on unsuccessful status code?

[sigilioso]

I had some issues setting up the OTLP exporter with http-proto tu report metrics (I finally realized that I needed to include not only the reqwest-client but also the reqwest-rustls feature, since I am using TLS)

While debugging the setup, I would had found really useful having more information about the request errors. Therefore, I was considering extending the MetricsClient implementation of OtlpHttpClient to also consider unsuccessful status codes. Something similar to this:

        let response = client
            .send(request)
            .await
            .map_err(|e| MetricsError::ExportErr(Box::new(Error::RequestFailed(e))))?;

        if !response.status().is_success() {
            let body_msg = std::str::from_utf8(response.body().iter().as_slice())
                .unwrap_or("response body is omitted because it cannot be decoded");
            return Err(MetricsError::ExportErr(Box::new(Error::RequestFailed(
                format!("request failed with status {} ({})", response.status(), body_msg).into(),
            ))))?;
        }

I also realized that opentelemetry_http::ResponseExt already provides a function to check the status code. Unfortunately, it is specialized for traces (it returns TraceError). Maybe a refactor in opentelemetry_http::ResponseExt to use a different error that could be mapped to TraceError, MetricError and LogError would be a better approach.

If this is something you also consider useful and you could provide some guidance, I would be willing to work on it.

[djc]

We should definitely log and/or yield errors in these cases, any contribution is welcome!

[cijothomas]

We should definitely log and/or yield errors in these cases, any contribution is welcome!

💯