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

add support for attributes in tracing-opentelemetry::MetricsLayer #2337

Open
nmoutschen opened this issue Oct 6, 2022 · 5 comments · May be fixed by #2354
Open

add support for attributes in tracing-opentelemetry::MetricsLayer #2337

nmoutschen opened this issue Oct 6, 2022 · 5 comments · May be fixed by #2354

Comments

@nmoutschen
Copy link
Contributor

Feature Request

Crates

tracing-opentelemetry

Motivation

Right now, MetricsLayer doesn't support specifying attributes when recording metrics value, but passes an empty slice instead.

As a consequence, users of this crate have to create multiple metrics instead of using one but with different attributes values. When exposing metrics through an OpenMetrics HTTP endpoint, this means exposing metrics looking like this:

# HELP custom_metric_a custom_metric.a
# TYPE custom_metric_a counter
custom_metric_a 1
# HELP custom_metric_b custom_metric.b
# TYPE custom_metric_b counter
custom_metric_b 1

Instead of like this, which would simplify aggregations, as many observability solutions can aggregate per metrics, then allow to query for specific attribute values:

# HELP custom_metric custom_metric
# TYPE custom_metric counter
custom_metric{my_attribute="a"} 1
custom_metric{my_attribute="b"} 2

Supporting attributes would also allow using multiple attributes, instead of chaining those values in a single custom metric.

Proposal

I propose to utilise the prefix attribute. for capturing attributes in Events, which would then be used to populate attributes when recording metrics:

info!(monotonic_counter.custom_metric = 1, attribute.my_attribute = "a");

Alternatives

Discard the prefix

We could discard the attribute. prefix, but this could cause a significant increase in the number of recorded attributes for high cardinality values. By utilising a prefix, we increase the likelihood of users actually meaning to use this value as an attribute.

For example, if developers records both metrics and messages in a single Event, with the messages being dynamically generated, this would create a new combination of attributes.

For example, this code snippet:

fn my_func(val: &str) {
    info!(monotonic_counter.my_func = 1, "got value: {val}");
}

my_func("a");
my_func("b");
my_func("c");

Would result in the following metrics:

# HELP my_func my_func
# TYPE my_func counter
my_func{message="got value: a"} 1
my_func{message="got value: b"} 1
my_func{message="got value: c"} 1
@aluminous aluminous linked a pull request Oct 24, 2022 that will close this issue
@aluminous
Copy link

I opened PR #2354 to implement your proposed solution after searching for similar functionality myself.

I also think that including the explicit attribute. prefix is preferable to including all keys in order to prevent the risk of inadvertently generating large numbers of metrics.

One case where this could a bit awkward when metrics and messages which are captured together contain overlapping attributes, such as bar in the following example:

info!(
    monotonic_counter.foo = 1,
    attribute.bar = "example",
    bar = "example",
    "Example Event"
);

@dotdat
Copy link

dotdat commented Dec 12, 2022

Hey folks, any updates on the PR from above? This would be an amazing feature to have.

@hawkw
Copy link
Member

hawkw commented Dec 12, 2022

Sorry, I haven't had time to review this PR yet. I'll try to find the time to in the near future!

@aaronArinder
Copy link

howdy, @hawkw, wondering how this is going? we're fairly interested in it; so, let me know how we can help get it merged! ❤️

@ajwerner
Copy link

This seems fixed in https://github.com/tokio-rs/tracing-opentelemetry by tokio-rs/tracing-opentelemetry#43.

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

Successfully merging a pull request may close this issue.

6 participants