jvm-observ-lib: Add java_micrometer_with_suffixes option #1351
Conversation
There is yet another metric pattern combinatino. When micrometer metrics are received through OTEL bridge then prometheus suffixes might not be added. Adding this option to jvm/process lib
Dasomeone
left a comment
Dasomeone
left a comment
There was a problem hiding this comment.
Couple comments, but overall looks very good! If the suggested changes aren't possible, I'm happy enough to approve it, just let me know
| - `otel_with_suffixes` same as otel with add_metric_suffixes=true in otelcollector | ||
| - `java_micrometer` (springboot) (https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jvm/JvmMemoryMetrics.java) | ||
| - `java_micrometer` (springboot) (https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jvm/JvmMemoryMetrics.java). Can be seen when Micrometer to OTEL bridge is used. | ||
| - `java_micrometer_with_suffixes` (springboot) same, but with prometheus suffixes |
There was a problem hiding this comment.
From a readability perspective, could you rename this to java_micrometer_bridge_prom and java_micrometer_bridge_otel or similar? I think it'd be a fair bit cleaner
There was a problem hiding this comment.
Ok, i'll try to carefully rename them.
| dashboardNamePrefix: 'JVM', | ||
| dashboardTags: ['java', 'jvm'], | ||
| metricsSource: 'java_micrometer', // or java_otel, prometheus, | ||
| metricsSource: 'java_micrometer_with_suffixes', // or java_otel, prometheus, |
There was a problem hiding this comment.
See above comment, where at first read it can be confusing what exactly the source is here. Additionally would be good to mention the otel bridge option
| groupLabels: this.config.groupLabels, | ||
| instanceLabels: this.config.instanceLabels, | ||
| uid: this.config.uid, | ||
| uid: this.config.uid - '-jvm', |
There was a problem hiding this comment.
Wouldn't this be a breaking change for any customer bookmarks etc?
There was a problem hiding this comment.
it might break bookmarks. But otherwise alerts groups of jvm (of kafka) and jvm (of zookeeper) might interfere with each other as they would have similar uids.
There was a problem hiding this comment.
Agreed, but we should call it out in the PR body + changelog for the integration so we make sure it's at least mentioned, because we might get support tickets for doing this
|
On request from @v-zhuravlev I'm closing this for the time being as it needs to be revisited later |
4 participants
There is yet another metric pattern combination. When micrometer metrics are received through OTEL bridge then prometheus suffixes might not be added. Adding this option to jvm/process lib