-
Notifications
You must be signed in to change notification settings - Fork 222
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
feat(metrics): move to otel abstractions for metrics recording #1147
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1147 +/- ##
==========================================
+ Coverage 79.31% 79.91% +0.60%
==========================================
Files 36 35 -1
Lines 2562 2435 -127
==========================================
- Hits 2032 1946 -86
+ Misses 434 393 -41
Partials 96 96
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
lgtm, would you mind adding a section to the CHANGELOG under ##unreleased about the metric renaming so that we don't forget at release time?
var ( | ||
// Hit is a counter for cache hits. | ||
Hit = metrics.MustSyncInt64(). | ||
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.
i like this pattern much better than what we were previously having to do
This updates the three existing packages which record prom metrics.
It updates them to record their metrics via the
otel
abstractions.The packages are:
The result is some slight adjustments to how the metrics are named.
Most notable is the database stats ones.
Because it turns out,
otelsql
package we use for tracing has all the instruments for metrics too.Using this package changes the names to be
db_sql_connection_
anddb_sql_latency_
prefixed.They make some good choices regarding metric types. Even rolling latency up into a histogram.