diff --git a/src/spanner/src/observability/metrics.rs b/src/spanner/src/observability/metrics.rs index a6a4e84f32..33a114c573 100644 --- a/src/spanner/src/observability/metrics.rs +++ b/src/spanner/src/observability/metrics.rs @@ -24,7 +24,10 @@ use { gaxi::options::ClientConfig, google_cloud_monitoring_v3::client::MetricService, opentelemetry::metrics::{Counter, Histogram, Meter, MeterProvider}, - opentelemetry_sdk::metrics::{PeriodicReader, SdkMeterProvider}, + opentelemetry_sdk::{ + error::OTelSdkError, + metrics::{PeriodicReader, SdkMeterProvider}, + }, }; #[cfg(not(feature = "_experimental-builtin-metrics"))] @@ -76,7 +79,7 @@ impl SpannerMetrics { #[derive(Debug)] pub(crate) struct Observability { pub(crate) metrics: Option, - _meter_provider: Option, + meter_provider: Option, } #[cfg(feature = "_experimental-builtin-metrics")] @@ -84,7 +87,7 @@ impl Observability { pub(crate) fn disabled() -> Self { Self { metrics: None, - _meter_provider: None, + meter_provider: None, } } @@ -138,7 +141,7 @@ impl Observability { Self { metrics: Some(metrics), - _meter_provider: Some(meter_provider), + meter_provider: Some(meter_provider), } } @@ -228,6 +231,25 @@ impl Observability { .record(duration.as_secs_f64() * 1000.0, &attributes); metrics.operation_count.add(1, &attributes); } + + pub(crate) fn shutdown(&self) { + if let Some(ref provider) = self.meter_provider + && let Err(err) = provider.shutdown() + && !matches!(err, OTelSdkError::AlreadyShutdown) + { + tracing::warn!( + "Error shutting down OpenTelemetry SdkMeterProvider: {:?}", + err + ); + } + } +} + +#[cfg(feature = "_experimental-builtin-metrics")] +impl Drop for Observability { + fn drop(&mut self) { + self.shutdown(); + } } #[cfg(feature = "_experimental-builtin-metrics")] @@ -307,6 +329,9 @@ impl Observability { { f().await } + + #[allow(dead_code)] + pub(crate) fn shutdown(&self) {} } #[cfg(all(test, feature = "_experimental-builtin-metrics"))] @@ -314,31 +339,53 @@ mod tests { use super::*; #[test] - fn test_parse_server_timing() { + fn parse_server_timing() { assert_eq!( - parse_server_timing("gfet4t7;dur=12.5"), + super::parse_server_timing("gfet4t7;dur=12.5"), ServerTimings { gfe_latency: Some(12.5), afe_latency: None, } ); assert_eq!( - parse_server_timing("gfet4t7;desc=\"test\";dur=12.5,afe;dur=5;desc=\"other\""), + super::parse_server_timing("gfet4t7;desc=\"test\";dur=12.5,afe;dur=5;desc=\"other\""), ServerTimings { gfe_latency: Some(12.5), afe_latency: Some(5.0), } ); assert_eq!( - parse_server_timing("afe;dur=3,some-other;dur=10"), + super::parse_server_timing("afe;dur=3,some-other;dur=10"), ServerTimings { gfe_latency: None, afe_latency: Some(3.0), } ); assert_eq!( - parse_server_timing("invalid_format"), + super::parse_server_timing("invalid_format"), ServerTimings::default() ); } + + #[test] + fn observability_disabled_shutdown_and_drop() { + let o11y = Observability::disabled(); + o11y.shutdown(); + o11y.shutdown(); + // Dropping o11y should invoke Drop and shutdown without panic. + } + + #[cfg(feature = "_experimental-builtin-metrics")] + #[test] + fn observability_double_shutdown_ignores_already_shutdown() { + let meter_provider = SdkMeterProvider::builder().build(); + let o11y = Observability { + metrics: None, + meter_provider: Some(meter_provider), + }; + // Calling shutdown twice should cleanly handle AlreadyShutdown on the second call. + o11y.shutdown(); + o11y.shutdown(); + // Dropping o11y invokes Drop and calls shutdown a third time without panic or warning. + } }