Merge branch 'main' into 2726-clonebreaking

Author: Puffy1215

.github/workflows/benchmark.yml

@@ -13,7 +13,8 @@ on:
     branches:
       - main
 name: benchmark pull requests
-permissions: read-all
+permissions:
+  contents: read
 
 jobs:
   runBenchmark:

.github/workflows/ci.yml

@@ -1,7 +1,8 @@
 name: CI
 env:
   CI: true
-permissions: read-all
+permissions:
+  contents: read
 on:
   pull_request:
   push:

.github/workflows/codeql-analysis.yml

@@ -3,13 +3,13 @@ name: "CodeQL Analysis"
 env:    
     CODEQL_ENABLE_EXPERIMENTAL_FEATURES : true # CodeQL support for Rust is experimental
 
-permissions: read-all
+permissions:
+  contents: read
 
 on:
+  pull_request:
   push:
     branches: [main]
-  schedule:
-    - cron: '0 0 * * *' # once in a day at 00:00
   workflow_dispatch:
 
 jobs:

.github/workflows/fossa.yml

@@ -5,7 +5,8 @@ on:
     branches:
       - main
 
-permissions: read-all
+permissions:
+  contents: read
 
 jobs:
   fossa:

.github/workflows/integration_tests.yml

@@ -5,7 +5,8 @@ on:
   pull_request:
     types: [ labeled, synchronize, opened, reopened ]
 
-permissions: read-all
+permissions:
+  contents: read
 
 jobs:
   integration_tests:

.github/workflows/markdown-link-check.yml

@@ -8,7 +8,8 @@ on:
     paths:
       - '**/*.md'
 
-permissions: read-all
+permissions:
+  contents: read
 
 jobs:
   markdown-link-check:

.github/workflows/ossf-scorecard.yml

@@ -8,7 +8,8 @@ on:
     - cron: "50 3 * * 0" # once a week
   workflow_dispatch:
 
-permissions: read-all
+permissions:
+  contents: read
 
 jobs:
   analysis:

.github/workflows/pr_naming.yml

@@ -4,7 +4,8 @@ on:
   pull_request:
     types: [opened, synchronize, reopened, edited]
 
-permissions: read-all
+permissions:
+  contents: read
 
 jobs:
   validate-pr-title:

.github/workflows/semver.yml

@@ -1,7 +1,8 @@
 name: Semver compliance
 env:
   CI: true
-permissions: read-all
+permissions:
+  contents: read
 on:
   pull_request:
     types: [ labeled, synchronize, opened, reopened ]

README.md

@@ -3,10 +3,11 @@
 The Rust [OpenTelemetry](https://opentelemetry.io/) implementation.
 
 [![Crates.io: opentelemetry](https://img.shields.io/crates/v/opentelemetry.svg)](https://crates.io/crates/opentelemetry)
-[![Documentation](https://docs.rs/opentelemetry/badge.svg)](https://docs.rs/opentelemetry)
 [![LICENSE](https://img.shields.io/crates/l/opentelemetry)](./LICENSE)
 [![GitHub Actions CI](https://github.com/open-telemetry/opentelemetry-rust/workflows/CI/badge.svg)](https://github.com/open-telemetry/opentelemetry-rust/actions?query=workflow%3ACI+branch%3Amain)
+[![Documentation](https://docs.rs/opentelemetry/badge.svg)](https://docs.rs/opentelemetry)
 [![codecov](https://codecov.io/gh/open-telemetry/opentelemetry-rust/branch/main/graph/badge.svg)](https://codecov.io/gh/open-telemetry/opentelemetry-rust)
+[![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/open-telemetry/opentelemetry-rust/badge)](https://scorecard.dev/viewer/?uri=github.com/open-telemetry/opentelemetry-rust)
 [![Slack](https://img.shields.io/badge/slack-@cncf/otel/rust-brightgreen.svg?logo=slack)](https://cloud-native.slack.com/archives/C03GDP0H023)
 
 ## Overview

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

@@ -18,12 +18,20 @@ also modified to suppress telemetry before invoking exporters.
   - This feature was previously removed in version 0.28 due to the lack of
     configurability but has now been reintroduced with the ability to configure
     the limit.  
-  - There is ability to configure cardinality limits via Instrument 
+  - There is ability to configure cardinality limits via Instrument
     advisory. [#2903](https://github.com/open-telemetry/opentelemetry-rust/pull/2903)
+  - Fixed the overflow attribute to correctly use the boolean value `true`
+    instead of the string `"true"`.
+    [#2878](https://github.com/open-telemetry/opentelemetry-rust/issues/2878)
 
 - *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/internal/mod.rs

@@ -23,7 +23,7 @@ pub(crate) static STREAM_OVERFLOW_ATTRIBUTES: OnceLock<Vec<KeyValue>> = OnceLock
 
 #[inline]
 fn stream_overflow_attributes() -> &'static Vec<KeyValue> {
-    STREAM_OVERFLOW_ATTRIBUTES.get_or_init(|| vec![KeyValue::new("otel.metric.overflow", "true")])
+    STREAM_OVERFLOW_ATTRIBUTES.get_or_init(|| vec![KeyValue::new("otel.metric.overflow", true)])
 }
 
 pub(crate) trait Aggregator {

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::*;
@@ -122,6 +123,7 @@ mod tests {
     use data::GaugeDataPoint;
     use opentelemetry::metrics::{Counter, Meter, UpDownCounter};
     use opentelemetry::InstrumentationScope;
+    use opentelemetry::Value;
     use opentelemetry::{metrics::MeterProvider as _, KeyValue};
     use rand::{rngs, Rng, SeedableRng};
     use std::cmp::{max, min};
@@ -2399,8 +2401,7 @@ mod tests {
         assert_eq!(sum.data_points.len(), 2002);
 
         let data_point =
-            find_sum_datapoint_with_key_value(&sum.data_points, "otel.metric.overflow", "true")
-                .expect("overflow point expected");
+            find_overflow_sum_datapoint(&sum.data_points).expect("overflow point expected");
         assert_eq!(data_point.value, 300);
 
         // let empty_attrs_data_point = &sum.data_points[0];
@@ -2448,8 +2449,7 @@ mod tests {
             // For cumulative, overflow should still be there, and new points should not be added.
             assert_eq!(sum.data_points.len(), 2002);
             let data_point =
-                find_sum_datapoint_with_key_value(&sum.data_points, "otel.metric.overflow", "true")
-                    .expect("overflow point expected");
+                find_overflow_sum_datapoint(&sum.data_points).expect("overflow point expected");
             assert_eq!(data_point.value, 600);
 
             let data_point = find_sum_datapoint_with_key_value(&sum.data_points, "A", "foo");
@@ -2505,8 +2505,7 @@ mod tests {
         assert_eq!(sum.data_points.len(), cardinality_limit + 1 + 1);
 
         let data_point =
-            find_sum_datapoint_with_key_value(&sum.data_points, "otel.metric.overflow", "true")
-                .expect("overflow point expected");
+            find_overflow_sum_datapoint(&sum.data_points).expect("overflow point expected");
         assert_eq!(data_point.value, 300);
 
         // let empty_attrs_data_point = &sum.data_points[0];
@@ -2554,8 +2553,7 @@ mod tests {
             // For cumulative, overflow should still be there, and new points should not be added.
             assert_eq!(sum.data_points.len(), cardinality_limit + 1 + 1);
             let data_point =
-                find_sum_datapoint_with_key_value(&sum.data_points, "otel.metric.overflow", "true")
-                    .expect("overflow point expected");
+                find_overflow_sum_datapoint(&sum.data_points).expect("overflow point expected");
             assert_eq!(data_point.value, 600);
 
             let data_point = find_sum_datapoint_with_key_value(&sum.data_points, "A", "foo");
@@ -2604,8 +2602,7 @@ mod tests {
         assert_eq!(sum.data_points.len(), cardinality_limit + 1 + 1);
 
         let data_point =
-            find_sum_datapoint_with_key_value(&sum.data_points, "otel.metric.overflow", "true")
-                .expect("overflow point expected");
+            find_overflow_sum_datapoint(&sum.data_points).expect("overflow point expected");
         assert_eq!(data_point.value, 300);
 
         // let empty_attrs_data_point = &sum.data_points[0];
@@ -2653,8 +2650,7 @@ mod tests {
             // For cumulative, overflow should still be there, and new points should not be added.
             assert_eq!(sum.data_points.len(), cardinality_limit + 1 + 1);
             let data_point =
-                find_sum_datapoint_with_key_value(&sum.data_points, "otel.metric.overflow", "true")
-                    .expect("overflow point expected");
+                find_overflow_sum_datapoint(&sum.data_points).expect("overflow point expected");
             assert_eq!(data_point.value, 600);
 
             let data_point = find_sum_datapoint_with_key_value(&sum.data_points, "A", "foo");
@@ -2837,6 +2833,14 @@ mod tests {
         })
     }
 
+    fn find_overflow_sum_datapoint<T>(data_points: &[SumDataPoint<T>]) -> Option<&SumDataPoint<T>> {
+        data_points.iter().find(|&datapoint| {
+            datapoint.attributes.iter().any(|kv| {
+                kv.key.as_str() == "otel.metric.overflow" && kv.value == Value::Bool(true)
+            })
+        })
+    }
+
     fn find_gauge_datapoint_with_key_value<'a, T>(
         data_points: &'a [GaugeDataPoint<T>],
         key: &str,

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,
 };