-
Notifications
You must be signed in to change notification settings - Fork 66
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
enhancement: add gauge metric behind feature #117
Conversation
Any chance it will be merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but the checks didn't run for some reason? Mind pushing an empty commit?
async fn i64_gauge_with_attributes_is_exported() { | ||
let (subscriber, exporter) = init_subscriber( | ||
"hello_world".to_string(), | ||
InstrumentKind::Counter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these tests use Gauge
?
This shouldn't even work because the metric shouldn't be downcastable to Sum
or maybe I'm missing something there, but this seems suspicious to me.
I think it would be best to add a test that calls tracing::info
twice with different values to test that the value is overwritten and not summed like for counter metrics.
Hello guys, I made an another pr based on the branch |
since #129 is merged - this one can be closed |
@academiaresf thanks for jumpstarting this! |
Motivation
The opentelemetry crate actually supports
Gauge
metric behindotel_unstable
feature.Solution
Extend the current implementation with the treatment for the new
Gauge
metric.The tests are passing but i don't know if there are necessary other type of tests (at this point, the property
start_time
inside theDataPoints
of theGauge
type areNone
in all the test, for example. L664 of tests/metrics_publishing.rs file).