Provide a way to set unit without setting description

[jaskij]

As far as I can tell, currently the only way to set a unit is to use the describe_histogram!() macro. This macro requires three arguments (third being description) and if you try to call it with only two, as describe_histogram!(Self::HISTO_NAME, metrics::Unit::Seconds);, it errors out with an extremely cryptic error about a missing semicolon.

A temporary workaround is to simply set an empty description, but the error over what, to me, is an intuitive use of the API, is extremely confusing.

[tobz]

You're right that the error is cryptic, which is a side effect of using procedural macros, in this case.

The main problem is that the actual Recorder trait doesn't allow for the description to be optionally set. The trait could likely be changed to make the description optional, along with the describe_* macros to take both unit or description separately and optionally. However, my time is a bit fleeting these days so I can't commit to doing so in the near-term.

PRs are definitely welcome and I'd be happy to guide you, or anyone, working on such a PR.

[tobz]

Mulling this over, I'm going to see if maybe there's a more generic approach we can take in the macros. My thought is that "describing" a metric could/should be more like applying labels to it, such that a hypothetical refactor of describe_counter! could look something like this:

describe_counter!(description => "blah blah", unit => Unit::Nanoseconds)

Ultimately, certain recorders (or certain applications) might care about descriptions, some might only care about units, or they might care about entirely different things altogether. The current design is limiting in that regard, notwithstanding the issue you've brought up here around having to specify a description just to set a unit.

I want to think about whether or not there's a way to, perhaps, make it less of a raw key/value map of attributes where every recorder needs to, for example, hope/assume the describe calls use description for the description attributes and so on. Maybe there could be some sort of enum/fixed constants to standardize around for declaring common attributes, and then a catch-all variant for describing "custom" attributes... not sure.

Anyways, yeah, I'm going to give this some thought.

[jaskij]

What you describe here really reminds me of the design of valuable - while it doesn't get much traffic, it has unstable support in tracing, one I have used to much success.

I think your idea with an enum for the common/standard attributes and a catch-all for recorder-spcific attributes is a good idea. I could see something like below, with default implementations for the current describe_x functions providing backwards compatibility.

#[non_exhaustive]
enum MetricAttribute {
    Description(metrics::SharedString),
    Unit(metrics::Unit),
    Custom{key: metrics::SharedString, value: valuable::Value},
}

pub trait Recorder {
    // Required methods
    fn set_counter_attribute(&self, key: KeyName, attribute: MetricAttribute);
    fn set_gauge_attribute(&self, key: KeyName, attribute: MetricAttribute);
    fn set_histogram_attribute(&self, key: KeyName, attribute: MetricAttribute);

    fn register_counter(&self, key: &Key) -> Counter;
    fn register_gauge(&self, key: &Key) -> Gauge;
    fn register_histogram(&self, key: &Key) -> Histogram;
}

[tobz]

Yeah, I'm familiar with valuable, and overall, your sketch is what I was roughly envisioning when typing up my previous reply.

I'm still torn on what should be "standard", and whether or not metrics should enforce it. As an example, is metrics::Unit actually sufficient for all users as-is, or should it also get the valuable treatment?

Like I think it's worth standardizing so that implementors can be reasonably sure that when they're specifying the "unit" (or description, or any other "standard" attribute), downstream applications can accurately interpret the unit for the given metric.. but it's still encoding a fixed set of what units can be set, and obviously you gotta make a choice at some point if you're trying to provide some level of consistency/standardization... but yeah, need to noodle around with it a little more.

[jaskij]

I'm still torn on what should be "standard"

Personally I would be inclined to follow OpenTelemetry since it is an effort to standardize both metric and tracing data.

As an example, is metrics::Unit actually sufficient for all users as-is

Most definitely not, but that's a separate can of worms - one I opened #360 for.