Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamically nameable metrics #160

Open
alfiejfs opened this issue Aug 3, 2024 · 6 comments
Open

Dynamically nameable metrics #160

alfiejfs opened this issue Aug 3, 2024 · 6 comments

Comments

@alfiejfs
Copy link

alfiejfs commented Aug 3, 2024

Feature Request

I would like a way for metrics to be nameable at runtime.

Motivation

With the current API, I don't believe you can pass in a metric name which is determined at runtime. This is fully within the realm of possibility when using the OpenTelemtry API, so it would be reasonable for this crate to support that somehow.

Proposal

I assume the most reasonable way might be a reserved field name for the metric name and metric value. However, I am not sure how this would interact with the existing interface.

Below is something like what I just described, but if there is any better way I'd prefer that. I just don't know how much flexibility or control we have here as we are using the tracing macros.

info!(metric.name = format!("histogram.{my_string}"), metric.value = 5)
@djc
Copy link
Collaborator

djc commented Aug 3, 2024

I'm open to reviewing a PR in this direction but won't be able to do much else to assist your efforts in this.

@mladedav
Copy link
Contributor

mladedav commented Aug 3, 2024

See tracing docs, search on page for "constant". There's no good heading to generate a better link for you.

You can use constant expressions for field names, so does that solve the issue for you? The example shows just span, but I think it works for events too. I also think there is a bug that if you try to use constants as the first field it does not currently work, so try adding a dummy field first.

@alfiejfs
Copy link
Author

alfiejfs commented Aug 4, 2024

@mladedav I did some more digging. This is what I ran into initially when I first tested this:

#[test]
fn it_works() {
    const METRIC_HANDLE: &str = "counter.test";
    info!({ METRIC_HANDLE} = 1);
}
error: format argument must be a string literal
  --> src/lib.rs:13:15
   |
13 |         info!({ METRIC_HANDLE } = 1);
   |               ^^^^^^^^^^^^^^^^^^^^
   |
help: you might be missing a string literal to format with
   |
13 |         info!("{}", { METRIC_HANDLE } = 1);
   |               +++++

I think I should've been more thorough when checking the docs. I assumed that all syntax for the event macro would be valid on the shorthands, but this is not the case. Sorry - I am new to the Rust ecosystem! The following does work:

#[test]
fn it_works() {
  const METRIC_HANDLE: &str = "counter.test";
  event!(Level::INFO, { METRIC_HANDLE } = 1);
}

I will leave this issue open for now incase a runtime evaluated solution wants to be discussed, but I think this is actually sufficient for most use cases. Thanks :)

I will edit the main issue body to reflect what the issue now represents, but the title can remain the same as the original request was for runtime nameable metrics.

@mladedav
Copy link
Contributor

mladedav commented Aug 4, 2024

The shorthands should work the same as the event! macro, of course except you don't specify the level.

I think you just bumped into the constant name as the first field bug I mentioned, i.e. I think that this would work:

#[test]
fn it_works() {
    const METRIC_HANDLE: &str = "counter.test";
    info!(foo = 1, { METRIC_HANDLE} = 1);
}

@alfiejfs
Copy link
Author

alfiejfs commented Aug 4, 2024

@mladedav does this then only work if you want at least 1 attribute on your metric?

@mladedav
Copy link
Contributor

mladedav commented Aug 4, 2024

From what I remember, just the first thing cannot be a field with const name. I think it was because fields can be optionally enclosed in curly brackets for historical reasons. If you use event!, the first token is the level so that works. If you use normal field and only then const expressions, that's also fine, but reversing the order would break it. I think (but am very unsure) that if you use additional curly brackets as in info!({{ NAME } = "foo"}) then that might also work.

It's just a bug which I think was fixed but not yet released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants