feat(metrics): Enhance Family's flexibility

[ethercflow]

This PR introduces two new interfaces to Family: get and with_metrics. These additions cater to the following use cases:

1. In crate A (e.g., an embedded database), the get_or_create interface is used to create metric M1. In crate B, which depends on crate A, there's a need to check if metric A exists (with dynamic labels that may not have been created yet, eg., no set_kv happened yet). If it exists, additional business-specific labels from crate B can be added to this metric. For example:

for (xxx, family) in families { // Family is defined in the foreign crate A for method in Method::iter() { // Method is also defined in the foreign crate A if let Some(metrics) = family.get(&Labels { method }) { // Only when the event happend in crate A, we add more labels to it in crate B let labels = [("XXX", xxx.to_string()), ("method", method.to_string())]; let e = metric_encoder.encode_family(&labels)?; metrics.encode(e)?; } } }

2. While Family's definition allows for custom constructors, there are instances where constructor parameters are required. The standard get_or_create implementation and MetricConstructor falls short in such scenarios. The new with_metrics interface enables users to implement a CustomFamily that wraps Family as an inner field. By implementing Deref and DerefMut, CustomFamily can reuse most of Family's interfaces while defining its own constructor trait and get_or_create implementation. This significantly enhances Family's flexibility. For example:

pub trait CustomMetricCreator<S: FamilyLabels, M: FamilyMetric>: Send + Sync + 'static { fn create(&self, labels: S) -> M; } #[derive(Clone)] pub struct CustomFamily<S: FamilyLabels, M: FamilyMetric, C: FamilyMetricCreator<S, M>> { inner: Family<S, M>, custom_metric_creator: C, } impl<S: FamilyLabels, M: FamilyMetric, C: FamilyMetricCreator<S, M>> CustomFamily<S, M, C> { pub fn create(creator: C) -> Self { Self { inner: Family::new_with_constructor(|| unreachable!()), custom_metric_creator: creator, } } pub fn get_or_create(&self, label_set: &S) -> Arc<M> { if let Some(metric) = self.inner.get(label_set) { return metric.clone(); } self.inner.with_metrics(|metrics| { let mut write_guard = metrics.write(); write_guard .entry(label_set.clone()) .or_insert_with(|| Arc::new(self.custome_metric_creator.create(label_set.clone()))) .clone() }) } }

[ethercflow]

@mxinden PTAL, thanks!

[mxinden]

Thanks for the thorough patch.

I am fine with get. I don't think we should expose the internals of Family, e.g. via with_metrics.

[mxinden]

I don't think we should expose the internals of Family on the public API.

Say we want to change self.metrics to a different datastructure. That would be a breaking change.

[ethercflow]

I don't think we should expose the internals of Family on the public API.

Say we want to change self.metrics to a different datastructure. That would be a breaking change.

Agree with your point of view, do you think improvements like the following are acceptable?

pub fn with_metrics<F, R>(&self, f: F) -> R where F: FnOnce(&dyn MetricsView<S, M>) -> R { struct MetricsViewImpl<'a, S, M>(&'a RwLock<HashMap<S, M>>); impl<'a, S, M> MetricsView<S, M> for MetricsViewImpl<'a, S, M> { fn get(&self, key: &S) -> Option<&M> { self.0.read().get(key) } // other methods } f(&MetricsViewImpl(&self.metrics)) } pub trait MetricsView<S, M> { fn get(&self, key: &S) -> Option<&M>; // other methods }

[mxinden]

I don't think the above use-case justifies introducing this much complexity.

Would implementing your own Family metric type suffice for now?

[mxinden]

For the sake of getting Family::get merged, I suggest splitting this pull request by the two new functions.

Note that I am not sure with_metrics is a good idea.

• It adds complexity to the crate that we will have to maintain long in the future.
• It does not enable a new use-case, as power-users can already provide their own fork of a Family metric type.

[ethercflow]

For the sake of getting Family::get merged, I suggest splitting this pull request by the two new functions.

Note that I am not sure with_metrics is a good idea.

• It adds complexity to the crate that we will have to maintain long in the future.
• It does not enable a new use-case, as power-users can already provide their own fork of a Family metric type.

Get it, will split this pr and submit Family::get first soon. Thanks!