You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "koushiro (via GitHub)" <gi...@apache.org> on 2025/03/28 04:12:34 UTC
[PR] refactor(layer/otelmetrics): Migrate OtelMetricsLayer to support IO metrics [opendal]
koushiro opened a new pull request, #5901:
URL: https://github.com/apache/opendal/pull/5901
# Which issue does this PR close?
Closes #5889
# Rationale for this change
Part of #5883
# What changes are included in this PR?
Migrate OtelMetricsLayer to support IO metrics
# Are there any user-facing changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(layer/otelmetrics): Migrate OtelMetricsLayer to support IO metrics [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #5901:
URL: https://github.com/apache/opendal/pull/5901#discussion_r2017916101
##########
core/src/layers/otelmetrics.rs:
##########
@@ -69,196 +65,89 @@ impl OtelMetricsLayer {
/// # async fn main() -> Result<()> {
/// let meter = opentelemetry::global::meter("opendal");
/// let op = Operator::new(services::Memory::default())?
- /// .layer(OtelMetricsLayer::builder().path_label(1).register(&meter))
+ /// .layer(
+ /// OtelMetricsLayer::builder()
+ /// .bytes_boundaries(exponential_boundaries(1024f64, 2.0f64, 10))
Review Comment:
In fact, we can tell users that we provide a solid default configuration. They don't need to make any changes unless they have specific requirements.
##########
core/src/layers/otelmetrics.rs:
##########
@@ -334,101 +279,207 @@ impl<A: Access> Layer<A> for OtelMetricsLayer {
#[derive(Clone, Debug)]
pub struct OtelMetricsInterceptor {
- operation_duration_seconds: Histogram<f64>,
operation_bytes: Histogram<u64>,
- http_request_duration_seconds: Histogram<f64>,
+ operation_bytes_rate: Histogram<f64>,
+ operation_entries: Histogram<u64>,
+ operation_entries_rate: Histogram<f64>,
+ operation_duration_seconds: Histogram<f64>,
+ operation_errors_total: Counter<u64>,
+ operation_executing: UpDownCounter<i64>,
+ operation_ttfb_seconds: Histogram<f64>,
+
+ http_executing: UpDownCounter<i64>,
http_request_bytes: Histogram<u64>,
- errors: Counter<u64>,
- path_label_level: usize,
+ http_request_bytes_rate: Histogram<f64>,
+ http_request_duration_seconds: Histogram<f64>,
+ http_response_bytes: Histogram<u64>,
+ http_response_bytes_rate: Histogram<f64>,
+ http_response_duration_seconds: Histogram<f64>,
+ http_connection_errors_total: Counter<u64>,
+ http_status_errors_total: Counter<u64>,
}
impl observe::MetricsIntercept for OtelMetricsInterceptor {
- fn observe_operation_duration_seconds(
- &self,
- info: Arc<AccessorInfo>,
- path: &str,
- op: Operation,
- duration: Duration,
- ) {
- let attributes = self.create_attributes(info, path, op, None);
- self.operation_duration_seconds
- .record(duration.as_secs_f64(), &attributes);
- }
-
- fn observe_operation_bytes(
- &self,
- info: Arc<AccessorInfo>,
- path: &str,
- op: Operation,
- bytes: usize,
- ) {
- let attributes = self.create_attributes(info, path, op, None);
- self.operation_bytes.record(bytes as u64, &attributes);
- }
-
- fn observe_operation_errors_total(
- &self,
- info: Arc<AccessorInfo>,
- path: &str,
- op: Operation,
- error: ErrorKind,
- ) {
- let attributes = self.create_attributes(info, path, op, Some(error));
- self.errors.add(1, &attributes);
- }
+ fn observe(&self, labels: observe::MetricLabels, value: observe::MetricValue) {
+ let attributes = self.create_attributes(labels);
- fn observe_http_request_duration_seconds(
- &self,
- info: Arc<AccessorInfo>,
- op: Operation,
- duration: Duration,
- ) {
- let attributes = self.create_attributes(info, "", op, None);
- self.http_request_duration_seconds
- .record(duration.as_secs_f64(), &attributes);
- }
+ match value {
+ observe::MetricValue::OperationBytes(v) => self.operation_bytes.record(v, &attributes),
+ observe::MetricValue::OperationBytesRate(v) => {
+ self.operation_bytes_rate.record(v, &attributes)
+ }
+ observe::MetricValue::OperationEntries(v) => {
+ self.operation_entries.record(v, &attributes)
+ }
+ observe::MetricValue::OperationEntriesRate(v) => {
+ self.operation_entries_rate.record(v, &attributes)
+ }
+ observe::MetricValue::OperationDurationSeconds(v) => self
+ .operation_duration_seconds
+ .record(v.as_secs_f64(), &attributes),
+ observe::MetricValue::OperationErrorsTotal => {
+ self.operation_errors_total.add(1, &attributes)
+ }
+ observe::MetricValue::OperationExecuting(v) => {
+ self.operation_executing.add(v as i64, &attributes)
+ }
+ observe::MetricValue::OperationTtfbSeconds(v) => self
+ .operation_ttfb_seconds
+ .record(v.as_secs_f64(), &attributes),
- fn observe_http_request_bytes(&self, info: Arc<AccessorInfo>, op: Operation, bytes: usize) {
- let attributes = self.create_attributes(info, "", op, None);
- self.http_request_bytes.record(bytes as u64, &attributes);
+ observe::MetricValue::HttpExecuting(v) => {
+ self.http_executing.add(v as i64, &attributes)
+ }
+ observe::MetricValue::HttpRequestBytes(v) => {
+ self.http_request_bytes.record(v, &attributes)
+ }
+ observe::MetricValue::HttpRequestBytesRate(v) => {
+ self.http_request_bytes_rate.record(v, &attributes)
+ }
+ observe::MetricValue::HttpRequestDurationSeconds(v) => self
+ .http_request_duration_seconds
+ .record(v.as_secs_f64(), &attributes),
+ observe::MetricValue::HttpResponseBytes(v) => {
+ self.http_response_bytes.record(v, &attributes)
+ }
+ observe::MetricValue::HttpResponseBytesRate(v) => {
+ self.http_response_bytes_rate.record(v, &attributes)
+ }
+ observe::MetricValue::HttpResponseDurationSeconds(v) => self
+ .http_response_duration_seconds
+ .record(v.as_secs_f64(), &attributes),
+ observe::MetricValue::HttpConnectionErrorsTotal => {
+ self.http_connection_errors_total.add(1, &attributes)
+ }
+ observe::MetricValue::HttpStatusErrorsTotal => {
+ self.http_status_errors_total.add(1, &attributes)
+ }
+ }
}
}
impl OtelMetricsInterceptor {
- fn create_attributes(
- &self,
- info: Arc<AccessorInfo>,
- path: &str,
- operation: Operation,
- error: Option<ErrorKind>,
- ) -> Vec<KeyValue> {
+ fn create_attributes(&self, attrs: observe::MetricLabels) -> Vec<KeyValue> {
let mut attributes = Vec::with_capacity(6);
attributes.extend([
- KeyValue::new(observe::LABEL_SCHEME, info.scheme().into_static()),
- KeyValue::new(observe::LABEL_NAMESPACE, info.name().clone()),
- KeyValue::new(observe::LABEL_ROOT, info.root().clone()),
- KeyValue::new(observe::LABEL_OPERATION, operation.into_static()),
+ KeyValue::new(observe::LABEL_SCHEME, attrs.scheme.into_static()),
+ KeyValue::new(observe::LABEL_NAMESPACE, attrs.namespace),
+ KeyValue::new(observe::LABEL_ROOT, attrs.root),
+ KeyValue::new(observe::LABEL_OPERATION, attrs.operation),
]);
- if let Some(path) = observe::path_label_value(path, self.path_label_level) {
- attributes.push(KeyValue::new(observe::LABEL_PATH, path.to_owned()));
+ if let Some(error) = attrs.error {
+ attributes.push(KeyValue::new(observe::LABEL_ERROR, error.into_static()));
}
- if let Some(error) = error {
- attributes.push(KeyValue::new(observe::LABEL_ERROR, error.into_static()));
+ if let Some(status_code) = attrs.status_code {
+ attributes.push(KeyValue::new(
+ observe::LABEL_STATUS_CODE,
+ status_code.as_u16() as i64,
+ ));
}
attributes
}
}
-fn exponential_boundary(start: f64, factor: f64, count: usize) -> Vec<f64> {
- let mut boundaries = Vec::with_capacity(count);
- let mut current = start;
- for _ in 0..count {
- boundaries.push(current);
- current *= factor;
+/// Creates exponentially spaced histogram boundaries.
+///
+/// # Arguments
+///
+/// * `start` - The upper bound of the first boundary (must be positive)
+/// * `factor` - The factor to multiply each boundary by (must be greater than 1)
+/// * `count` - The number of boundaries to generate (must be at least 1)
+///
+/// # Panics
+///
+/// Panics if:
+/// * `start` is not positive
+/// * `factor` is not greater than 1
+/// * `count` is less than 1
+pub fn exponential_boundaries(start: f64, factor: f64, count: usize) -> Vec<f64> {
Review Comment:
Let's remove this API. This function is just an easy way to generate a bucket, and should not be a public API at the Layer level.
##########
core/src/layers/otelmetrics.rs:
##########
@@ -69,196 +65,89 @@ impl OtelMetricsLayer {
/// # async fn main() -> Result<()> {
/// let meter = opentelemetry::global::meter("opendal");
/// let op = Operator::new(services::Memory::default())?
- /// .layer(OtelMetricsLayer::builder().path_label(1).register(&meter))
+ /// .layer(
+ /// OtelMetricsLayer::builder()
+ /// .bytes_boundaries(exponential_boundaries(1024f64, 2.0f64, 10))
Review Comment:
Users can pass `Vec<f64>` directly, we don't need to provide this helper functions.
##########
core/src/layers/mod.rs:
##########
@@ -90,7 +90,7 @@ pub use self::fastrace::FastraceLayer;
#[cfg(feature = "layers-otel-metrics")]
mod otelmetrics;
#[cfg(feature = "layers-otel-metrics")]
-pub use self::otelmetrics::OtelMetricsLayer;
+pub use self::otelmetrics::{exponential_boundaries, OtelMetricsLayer};
Review Comment:
Can we avoid exposing this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(layer/otelmetrics): Migrate OtelMetricsLayer to support IO metrics [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #5901:
URL: https://github.com/apache/opendal/pull/5901
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org