API for accessing the contextual Recorder

[asmello]

Another one for ergonomics.

Scenario:

• Install a Prometheus exporter, but skip the HTTP server, as we have our own.
• Inside our HTTP server, we add an endpoint that renders the metrics in Prometheus format, which requires access to a PrometheusHandle.

Currently, to make this work, we need to register the Prometheus Recorder and produce a handle to it in the very outer layers of the program (e.g. inside of main()) and then pass that all the way into the internal HTTP server state. Ideally we'd be able to get a handle internally by producing a reference to the contextual Recorder, downcasting it to the correct type and then calling PrometheusRecorder::handle.

This would actually be "safer", because if for some reason one creates multiple Recorders in the same program, there's a possibility of mismatch between the one in scope and the one being used to render the metrics. If we fetch the Recorder contextually that risk goes away.

[tobz]

Apologies for the delayed response.

For the sake of avoiding misinformation percolating: the only reason it's currently possible to have multiple "installed" recorders is due to the nature of being pre-1.0, and how often (relatively speaking) we push minor versions for breaking changes. At 1.0, this footgun goes away completely.

As far as being able to get the contextual recorder: seems like it should be possible to do, following the footsteps of what tracing does with Subscriber and surfacing metrics::with_recorder in the documentation (currently hidden).

I'd certainly be willing to review a PR adding something along those lines.

[asmello]

For the sake of avoiding misinformation percolating: the only reason it's currently possible to have multiple "installed" recorders is due to the nature of being pre-1.0, and how often (relatively speaking) we push minor versions for breaking changes. At 1.0, this footgun goes away completely.

I think I may have miscommunicated this part. I wasn't aware it's possible to have multiple "installed" recorders [1], and that's not really what I'm concerned about.

I was actually thinking more of a testing scenario again, where we want to isolate each test to use a separate Recorder instance. In this case you end up with multiple Recorders in the same binary (they'll all be local though).

Consider how this must work in practice:

fn main() {
    let recorder = PrometheusBuilder::new().build_recorder();
    let handle = recorder.handle();
    metrics::with_local_recorder(&recorder, || {
        let service = Service::builder().prometheus_handle(handle).build();
        // ...
    });
}

The "unsafe" part here is that the handle we pass down to use in the HTTP server may not match the local recorder that's in scope. Not a huge deal, but it's always nice to make invalid states non-representable.

As far as being able to get the contextual recorder: seems like it should be possible to do, following the footsteps of what tracing does with Subscriber and surfacing metrics::with_recorder in the documentation (currently hidden).

I think this would indeed solve the problem. Using this function, the previous example could become something like:

fn main() {
    let recorder = PrometheusBuilder::new().build_recorder();
    metrics::with_local_recorder(&recorder, || {
        let service = Service::new();
        // ...
    });
}

// somewhere
impl Service {
    fn setup_server() {
        let routes = /* ... */;
        metrics::with_recorder(|recorder| {
            if let Some(pr) = recorder.downcast::<PrometheusRecorder>() {
                let handle = pr.handle();
                routes.register(metrics_endpoint(handle)); // or something similar
            }
        });
        // ...
    }
}

(Note that in this version the metrics endpoint is only enabled if the contextual recorder is a PrometheusRecorder)

I'd certainly be willing to review a PR adding something along those lines.

Meaning just making metrics::with_recorder public?

[1]: curious how that happens? Also I do think something like Layer from tracing-subscriber would be highly useful, but that's a separate discussion.

[tobz]

The "unsafe" part here is that the handle we pass down to use in the HTTP server may not match the local recorder that's in scope. Not a huge deal, but it's always nice to make invalid states non-representable.

Recorders are inherently specific to the thread they're used on when paired with metrics::with_local_recorder. You can't pass an "incorrect" handle down unless you're creating the service on one thread (using with_local_recorder, which sets recorder A) and then using that handle on another thread where recorder B is set, and expecting that recorder B is what the handle will access.

If you are running everything on a single thread, then everything should be fine... save for how difficult it is to get the handle where it needs to go, and thus the downcasting.

Meaning just making metrics::with_recorder public?

Yes.

Also I do think something like Layer from tracing-subscriber would be highly useful, but that's a separate discussion.

Already exists: https://docs.rs/metrics-util/latest/metrics_util/layers/index.html