Change `Family::new_with_constructor` to be `impl Fn() -> C` instead of function pointer `fn`

[nastynaz]

Currently Family is defined using a fn pointer instead of Fn which is poor design:

pub struct Family<S, M, C = fn() -> M> { }

This means that this fails to compile:

let buckets = vec![1.0, 2.0, 3.0];
let metric = Family::<S, Histogram>::new_with_constructor(move || Histogram::new(buckets.clone()));

even though it the closure passed in implements Fn (though is not a fn() since closures can't be coerced to function pointers if they capture variables).

This becomes problematic when creating helper methods to instantiate metrics. For example I can write this:

/// Helper function to instantiate a `prometheus_client` metric.
fn instantiate_metric_primitive<
    S: EncodeLabelSet + Debug + Clone + std::hash::Hash + Eq + Send + Sync + 'static,
    M: Debug + Default + EncodeMetric + TypedMetric + Send + Sync + 'static,
>(
    registry: &mut Registry,
    name: &str,
    description: &str,
) -> Family<S, M> {
    let metric = Family::<S, M>::default();
    tracing::info!("registering {} metric with metrics recorder", name);
    registry.register(name, description, metric.clone());
    metric
}

But not this:

// helper function to instantiate a histogram metric
fn instantiate_histogram_metric<
    S: EncodeLabelSet + Debug + Clone + std::hash::Hash + Eq + Send + Sync + 'static,
>(
    registry: &mut Registry,
    name: &str,
    description: &str,
    buckets: Vec<f64>,
) -> Family<S, Histogram> {
    let metric =
       // closure cannot be coerced into an fn pointer (even though it's `impl Fn`
        Family::<S, Histogram>::new_with_constructor(move || Histogram::new(buckets.clone()));
    tracing::info!("registering {} metric with metrics recorder", name);
    registry.register(name, description, metric.clone());
    metric
}

Suggested Fix

The quick fix is to redefine family to be:

pub struct Family<S, M, C: Fn() -> M> { // <-- `C = fn() -> M` became  trait bounds`C: Fn() -> M`
    metrics: Arc<RwLock<HashMap<S, M>>>,
    constructor: C,
}

But in general stating the types in struct definitions is poor design. Prefer to instead refactor Family so there are no bounds on the struct and instead put bounds on the required impls.

[mxinden]

Thanks for the detailed issue. Want to propose a pull request with unit tests showing that both the old and the new behavior then possible?