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!