fix: cleanup MetricError (#2906)

Author: cijothomas

opentelemetry-otlp/src/exporter/http/metrics.rs

@@ -19,8 +19,8 @@ impl MetricsClient for OtlpHttpClient {
                 _ => Err(OTelSdkError::AlreadyShutdown),
             })?;
 
-        let (body, content_type) = self.build_metrics_export_body(metrics).map_err(|e| {
-            OTelSdkError::InternalFailure(format!("Failed to serialize metrics: {e:?}"))
+        let (body, content_type) = self.build_metrics_export_body(metrics).ok_or_else(|| {
+            OTelSdkError::InternalFailure("Failed to serialize metrics".to_string())
         })?;
         let mut request = http::Request::builder()
             .method(Method::POST)

opentelemetry-otlp/src/exporter/http/mod.rs

@@ -4,6 +4,8 @@ use super::{
 };
 use crate::{ExportConfig, Protocol, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS};
 use http::{HeaderName, HeaderValue, Uri};
+#[cfg(feature = "http-json")]
+use opentelemetry::otel_debug;
 use opentelemetry_http::HttpClient;
 use opentelemetry_proto::transform::common::tonic::ResourceAttributesWithSchema;
 #[cfg(feature = "logs")]
@@ -325,20 +327,21 @@ impl OtlpHttpClient {
     fn build_metrics_export_body(
         &self,
         metrics: &mut ResourceMetrics,
-    ) -> opentelemetry_sdk::metrics::MetricResult<(Vec<u8>, &'static str)> {
+    ) -> Option<(Vec<u8>, &'static str)> {
         use opentelemetry_proto::tonic::collector::metrics::v1::ExportMetricsServiceRequest;
 
         let req: ExportMetricsServiceRequest = (&*metrics).into();
 
         match self.protocol {
             #[cfg(feature = "http-json")]
             Protocol::HttpJson => match serde_json::to_string_pretty(&req) {
-                Ok(json) => Ok((json.into(), "application/json")),
-                Err(e) => Err(opentelemetry_sdk::metrics::MetricError::Other(
-                    e.to_string(),
-                )),
+                Ok(json) => Some((json.into(), "application/json")),
+                Err(e) => {
+                    otel_debug!(name: "JsonSerializationFaied", error = e.to_string());
+                    None
+                }
             },
-            _ => Ok((req.encode_to_vec(), "application/x-protobuf")),
+            _ => Some((req.encode_to_vec(), "application/x-protobuf")),
         }
     }
 }

opentelemetry-sdk/CHANGELOG.md

@@ -24,6 +24,11 @@ also modified to suppress telemetry before invoking exporters.
 - *Breaking* change for custom `MetricReader` authors.
   The `shutdown_with_timeout` method is added to `MetricReader` trait.
   `collect` method on `MetricReader` modified to return `OTelSdkResult`.
+  [#2905](https://github.com/open-telemetry/opentelemetry-rust/pull/2905)
+- *Breaking* `MetricError`, `MetricResult` no longer public (except when
+  `spec_unstable_metrics_views` feature flag is enabled). `OTelSdkResult` should
+  be used instead, wherever applicable.
+  [#2905](https://github.com/open-telemetry/opentelemetry-rust/pull/2905)
 
 ## 0.29.0
 

opentelemetry-sdk/src/metrics/aggregation.rs

@@ -1,7 +1,7 @@
 use std::fmt;
 
+use crate::metrics::error::{MetricError, MetricResult};
 use crate::metrics::internal::{EXPO_MAX_SCALE, EXPO_MIN_SCALE};
-use crate::metrics::{MetricError, MetricResult};
 
 /// The way recorded measurements are summarized.
 #[derive(Clone, Debug, PartialEq)]
@@ -109,7 +109,8 @@ impl fmt::Display for Aggregation {
 
 impl Aggregation {
     /// Validate that this aggregation has correct configuration
-    pub fn validate(&self) -> MetricResult<()> {
+    #[allow(unused)]
+    pub(crate) fn validate(&self) -> MetricResult<()> {
         match self {
             Aggregation::Drop => Ok(()),
             Aggregation::Default => Ok(()),
@@ -149,11 +150,11 @@ impl Aggregation {
 
 #[cfg(test)]
 mod tests {
+    use crate::metrics::error::{MetricError, MetricResult};
     use crate::metrics::{
         internal::{EXPO_MAX_SCALE, EXPO_MIN_SCALE},
         Aggregation,
     };
-    use crate::metrics::{MetricError, MetricResult};
 
     #[test]
     fn validate_aggregation() {

opentelemetry-sdk/src/metrics/error.rs

@@ -2,12 +2,14 @@ use std::result;
 use std::sync::PoisonError;
 use thiserror::Error;
 
-use crate::ExportError;
-
 /// A specialized `Result` type for metric operations.
+#[cfg(feature = "spec_unstable_metrics_views")]
 pub type MetricResult<T> = result::Result<T, MetricError>;
+#[cfg(not(feature = "spec_unstable_metrics_views"))]
+pub(crate) type MetricResult<T> = result::Result<T, MetricError>;
 
 /// Errors returned by the metrics API.
+#[cfg(feature = "spec_unstable_metrics_views")]
 #[derive(Error, Debug)]
 #[non_exhaustive]
 pub enum MetricError {
@@ -17,20 +19,27 @@ pub enum MetricError {
     /// Invalid configuration
     #[error("Config error {0}")]
     Config(String),
-    /// Fail to export metrics
-    #[error("Metrics exporter {0} failed with {name}", name = .0.exporter_name())]
-    ExportErr(Box<dyn ExportError>),
     /// Invalid instrument configuration such invalid instrument name, invalid instrument description, invalid instrument unit, etc.
     /// See [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#general-characteristics)
     /// for full list of requirements.
     #[error("Invalid instrument configuration: {0}")]
     InvalidInstrumentConfiguration(&'static str),
 }
 
-impl<T: ExportError> From<T> for MetricError {
-    fn from(err: T) -> Self {
-        MetricError::ExportErr(Box::new(err))
-    }
+#[cfg(not(feature = "spec_unstable_metrics_views"))]
+#[derive(Error, Debug)]
+pub(crate) enum MetricError {
+    /// Other errors not covered by specific cases.
+    #[error("Metrics error: {0}")]
+    Other(String),
+    /// Invalid configuration
+    #[error("Config error {0}")]
+    Config(String),
+    /// Invalid instrument configuration such invalid instrument name, invalid instrument description, invalid instrument unit, etc.
+    /// See [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#general-characteristics)
+    /// for full list of requirements.
+    #[error("Invalid instrument configuration: {0}")]
+    InvalidInstrumentConfiguration(&'static str),
 }
 
 impl<T> From<PoisonError<T>> for MetricError {

opentelemetry-sdk/src/metrics/meter.rs

@@ -12,10 +12,10 @@ use opentelemetry::{
 };
 
 use crate::metrics::{
+    error::{MetricError, MetricResult},
     instrument::{Instrument, InstrumentKind, Observable, ResolvedMeasures},
     internal::{self, Number},
     pipeline::{Pipelines, Resolver},
-    MetricError, MetricResult,
 };
 
 use super::noop::NoopSyncInstrument;

opentelemetry-sdk/src/metrics/mod.rs

@@ -66,6 +66,7 @@ pub mod in_memory_exporter;
 pub use in_memory_exporter::{InMemoryMetricExporter, InMemoryMetricExporterBuilder};
 
 pub use aggregation::*;
+#[cfg(feature = "spec_unstable_metrics_views")]
 pub use error::{MetricError, MetricResult};
 pub use manual_reader::*;
 pub use meter_provider::*;

opentelemetry-sdk/src/metrics/pipeline.rs

@@ -12,11 +12,11 @@ use crate::{
     metrics::{
         aggregation,
         data::{Metric, ResourceMetrics, ScopeMetrics},
+        error::{MetricError, MetricResult},
         instrument::{Instrument, InstrumentId, InstrumentKind, Stream},
         internal::{self, AggregateBuilder, Number},
         reader::{MetricReader, SdkProducer},
         view::View,
-        MetricError, MetricResult,
     },
     Resource,
 };