Separate Recorder method for timing data

[lilymara-onesignal]

Currently, the timing! macro simply calls the record_histogram method on the Recorder with a nanosecond duration as its argument. This works fine if you want to store nanoseconds directly, but if you want to turn those nanosecond values into seconds before storing them, to keep compatibility with other existing metrics, you would have to write a custom recorder that inspected keys and changed the value at runtime for certain histogram metrics, but not all. To solve this, I propose a new method on the Recorder trait:

fn record_timing_histogram(&self, nanoseconds: u64) {
    // Default impl to avoid a breaking change
    self.record_histogram(nanoseconds)
}

This would allow Recorder writers to easily convert the nanosecond measurement to whatever unit they wanted before forwarding the value to their store. Adding a default impl that forwards to record_histogram avoids a breaking change and allows existing Recorders to continue working.

The timing! macro would be changed to internally call record_timing_histogram instead of record_histogram.

[tobz]

So, I've marked this as a duplicate for a simple reason: we plan to change the Recorder trait as part of #67. The general approach for many downstream metrics storage systems and services seems to be to take a f64 for duration values, and so that's the approach we're also going to take.

That said, I'll leave this issue open because the redesign obviously hasn't happened yet and may possibly not happen as described above.

[tobz]

Ultimately, I'm going to close this ticket but wanted to add some color before doing so.

Thinking this through, and having been working on a refactor/overhaul of metrics, I want to avoid baking too much subjectivity into the API surface.

We started out with timing! and value! macros with the goal of having them be dedicated to their namesake, as well as offering appropriate ergonomics i.e. timing! could take a traditional start/end and do the math for you. In the current refactor, we've gone back to a single histogram! macro as a way to simplify things overall. Our implicit conversions are still present -- taking a Duration and mapping it to nanoseconds, for example -- but they're called out explicitly, and ultimately, it's up to the user to properly understand the timebase they're working with as well as naming their metrics to match, etc.

I realize that this is a slightly dogmatic view -- we're providing an implicit conversion! -- but I feel comfortable that, despite not matching the shape of popular systems like Prometheus, the idiosyncracies are small enough that they can be trivially papered over with little effort.