-
Notifications
You must be signed in to change notification settings - Fork 3k
Update micrometer #2223
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
base: dev
Are you sure you want to change the base?
Update micrometer #2223
Conversation
|
Changes suggested in the issue #2225 |
6d319ab to
7cacd0f
Compare
| metricValueFunction.apply(v) | ||
| Collection<GaugeSnapshot.GaugeDataPointSnapshot> gaugeDataPointSnapshots = new ArrayList<>(); | ||
| poolStatsMap.forEach((k, v) -> gaugeDataPointSnapshots.add( | ||
| new GaugeSnapshot.GaugeDataPointSnapshot(metricValueFunction.apply(v), Labels.of(k), null) |
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.
Labels.of(k) expects at least 2 arguments.
I think correct would be Labels.of("pool", k)
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.
makes sense, I pushed the change suggestion
|
Would this change break existing 1.5 micrometer users? It appears like it would. |
7af08ed to
1be30e6
Compare
|
Yes, it will break Micrometer because of breaking changes in Prometheus (and simpleclient). |
d7a9b2f to
e56fa46
Compare
Needed to streamline premetheus versions in the company I work for. Migration guide that could be needed for some use cases: https://github.com/micrometer-metrics/micrometer/wiki/1.13-Migration-Guide