Unable to reuse the PrometheusLayer with the same registry

[evenyag]

As mentioned in GreptimeTeam/greptimedb#4277 (comment), the PrometheusLayer will create a new PrometheusMetrics each time we register it to an OperatorBuilder.

opendal/core/src/layers/prometheus.rs

Lines 154 to 163 in 174bda5

	PrometheusAccessor {
	inner,
	stats: Arc::new(PrometheusMetrics::new(
	self.registry.clone(),
	self.requests_duration_seconds_buckets.clone(),
	self.bytes_total_buckets.clone(),
	self.path_label_level,
	)),
	scheme,
	}

However, the registry panics if we create PrometheusMetrics with the same registry more than once.

opendal/core/src/layers/prometheus.rs

Lines 181 to 198 in 174bda5

	/// new with prometheus register.
	pub fn new(
	registry: Registry,
	requests_duration_seconds_buckets: Vec<f64>,
	bytes_total_buckets: Vec<f64>,
	path_label_level: usize,
	) -> Self {
	let labels = if path_label_level > 0 {
	vec!["scheme", "operation", "path"]
	} else {
	vec!["scheme", "operation"]
	};
	let requests_total = register_int_counter_vec_with_registry!(
	"requests_total",
	"Total times of create be called",
	&labels,
	registry
	)

So users are unable to create multiple operators with the same layer that collects metrics to the default global registry of rust-prometheus.

One approach to work around this without breaking change is to allow users to set the PrometheusMetrics.

pub struct PrometheusLayer {
    registry: Registry,
    requests_duration_seconds_buckets: Vec<f64>,
    bytes_total_buckets: Vec<f64>,
    path_label_level: usize,
    metrics: Option<Arc<PrometheusMetrics>>,
}

If users set the metrics explicitly, then the layer uses the provided metrics. So we can pre-build the PrometheusMetrics and reuse it without constructing a new one each time.

[sunng87]

I think we can just remove this unwrap https://github.com/apache/opendal/blob/174bda53f79123cd114d2409189423a0a4cf6bf3/core/src/layers/prometheus.rs#L199C9-L199C19 by checking the error type: https://github.com/tikv/rust-prometheus/blob/master/src/registry.rs#L48 and ignore this AlreadyReg

[Xuanwo]

I think we can just remove this unwrap https://github.com/apache/opendal/blob/174bda53f79123cd114d2409189423a0a4cf6bf3/core/src/layers/prometheus.rs#L199C9-L199C19 by checking the error type: https://github.com/tikv/rust-prometheus/blob/master/src/registry.rs#L48 and ignore this AlreadyReg

Thanks, I think this can be addressed in a separate PR. Would you like to help with that?

[sunng87]

Oh, I just see why it's impossible because we need that register call to return the handle. If there is an error we won't be able to get the handle. We need to go upstream for this.

[evenyag]

Oh, I just see why it's impossible because we need that register call to return the handle. If there is an error we won't be able to get the handle. We need to go upstream for this.

We need a way to get the registered metric
tikv/rust-prometheus#248