-
Notifications
You must be signed in to change notification settings - Fork 18
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
Update metrics + metrics-exporter-prometheus #1649
Labels
Comments
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 13, 2024
justinweng-instaclustr
added a commit
that referenced
this issue
Jun 14, 2024
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 17, 2024
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 17, 2024
justinweng-instaclustr
added a commit
that referenced
this issue
Jun 18, 2024
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 19, 2024
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 19, 2024
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 19, 2024
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 19, 2024
justinweng-instaclustr
added a commit
that referenced
this issue
Jun 20, 2024
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 20, 2024
justinweng-instaclustr
added a commit
to justinweng-instaclustr/shotover-proxy
that referenced
this issue
Jun 20, 2024
justinweng-instaclustr
added a commit
that referenced
this issue
Jun 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
These two crates must be upgraded together since metrics-exporter-prometheus exposes types from metrics in its public API that we use.
However, the new version of metrics has improved its diagnostics such that unused diagnostics are shown as compiler warnings.
We have 2 such instances of these warnings.
Since CI will prevent merging with warnings, we need to either ignore the warnings or fix them.
I can see that our usages that are getting warned do have an associated performance cost that we should try to avoid.
Counter
#1650&'static str
- this will definitely be faster.Counter
we create in aHashMap<String, Counter>
this should be faster, we should measure it with a microbenchmark though.But also I think there are legitimate uses of creating a metric without using it so I've made a comment to see what upstream thinks: metrics-rs/metrics#475 (comment)
TODO, all in separate PRs:
Counter
#1650shotover/benches/benches/chain.rs
redis_filter
orloopback
as a base for the new bench.QueryCounter
followed byLoopback
. This is because QueryCounter ignores the responses and Loopback is the fastest transform for when we dont care about the value of responses.String::leak()
the counter_name so we store it as&'static str
- this will definitely be faster, run the benchmark to double check. #1649 Leak query counter name to improve QueryCounter performance #1669Counter
we create in aHashMap<String, Counter>
this should be faster, run the benchmark to see if it is. #1649 Reuse query counters #1671The text was updated successfully, but these errors were encountered: