Add support for Prometheus 1.x #4846
Conversation
jonatan-ivanov
added
enhancement
| libs.prometheusMetricsExpositionFormats, | ||
| libs.prometheusMetricsTracerCommon, | ||
| libs.prometheusSimpleClient, | ||
| libs.prometheusSimpleClientPushgateway, |
There was a problem hiding this comment.
This can be dealt with separate from this PR.
I think with the version catalog, we should move away from putting dependencies in here to be managed globally, especially when they're not transitive dependencies. I've been slowly removing them from here when doing related changes to dependencies that don't need to be here. We should use the version catalog reference in each build.gradle file instead, and then there's no need for them to be here.
There was a problem hiding this comment.
I think we should be able to remove things other than the PLATFORM_BOMS and the guava hacks from this file if we use the catalog references everywhere.
I created an issue for this: #4849
|
|
||
| @Override | ||
| public List<MetricFamilySamples> describe() { |
There was a problem hiding this comment.
I think the equivalent of this in 1.x is implementing getPrometheusNames. I don't think it's critical but it may help optimize calls to scrape with a name filter. We can follow-up on this post-merge.
| static class Family { | ||
|
|
||
| final Type type; | ||
| static class Family<T extends DataPointSnapshot> { |
There was a problem hiding this comment.
I think it'd be good to move away from the child/family naming to move things closer to the naming in the 1.x versions. Since these are all internal implementation details, we can change this later so no need to change anything before merging.
...eter-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusMeterRegistry.java
Outdated
Show resolved
Hide resolved
...eter-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusMeterRegistry.java
Outdated
Show resolved
Hide resolved
|
I updated the base package in micrometer-registry-prometheus to avoid a split package. It is now When doing this, the osgi test started failing because the IDE updated the imports to the new package, whereas before two classes were there with the same package, and apparently the -simpleclient version was being used before and passing. I'm not sure exactly why it is failing, but we'll have to note it as another known issue to be fixed later. |
jonatan-ivanov
added
the
release notes
We rename micrometer-registry-prometheus to micrometer-registry-prometheus-simpleclient so that we can add the new Prometheus client to micrometer-registry-prometheus.
This changeset adds back micrometer-registry-prometheus as it was before the rename in the previous commit. This makes it possible to add the new Prometheus client.
jonatan-ivanov
force-pushed
the
prometheus-1.x
branch
from
cd175f3 to
3b43e5a
Compare
Dependencies were upgraded to the 1.x versions of the Prometheus client artifacts in micrometer-registry-prometheus. This is a breaking change compared to the code that was there before. This is unavoidable due to the breaking changes in the Prometheus client from 0.x to 1.x. The previous code from micrometer-registry-prometheus was moved to a new module micrometer-registry-prometheus-simpleclient that is deprecated but available to give users a backward compatible option in case they need to make changes to use the new Prometheus client. The base package used in micrometer-registry-prometheus was changed to io.micrometer.prometheusmetrics to differentiate it from the prior package now used in micrometer-registry-prometheus-simpleclient. This avoids split packages and allows both modules to be used in the same application even. Tests were adapted to the new API and differences in the scrape output. Tests involving exemplars are commented out until exemplar support is added. PrometheusHistogram's cumulative buckets were converted to delta in the registry. The new Prometheus client wants delta buckets which is weird since it converts them back to cumulative internally. PrometheusNamingConvention behavior was changed, it does not append "_total" to counters anymore. The new Prometheus client validates Counter names and check if they end with "_total". If they don't, it appends "_total". If they do, it throws an exception: java.lang.IllegalArgumentException: 'cnt_total': Illegal metric name... Because of this behavior, if we want to use PrometheusNamingConvention, we need to modify it so that it does not append "_total". Known issues in the micrometer-registry-prometheus module after this PR: - Exemplars and Native Histograms do not work. - VictoriaMetrics histograms are not supported. - The OSGi test was failing when the new Prometheus client was used, so it was updated to use the -simpleclient module for now. See micrometer-metricsgh-3987 See micrometer-metricsgh-4406 Co-authored-by: Tommy Ludwig <[email protected]>
|
I made some changes before merge:
|
jonatan-ivanov
force-pushed
the
prometheus-1.x
branch
from
f3fd576 to
e738b0b
Compare
| * must match the regex [a-zA-Z_:][a-zA-Z0-9_:]* | ||
| */ | ||
| @Override | ||
| public String name(String name, Meter.Type type, @Nullable String baseUnit) { |
There was a problem hiding this comment.
I think we can delegate to PrometheusNaming in the implementation of our default NamingConvention. The important point was for the configured NamingConvention to be used instead of hard-coding using PrometheusNaming in PrometheusMeterRegistry and ignoring what the user configured.
There was a problem hiding this comment.
We can utilize PrometheusNaming but we still need to keep our custom logic in the naming convention where we append units and other things.
Upgrades to the 1.x versions of the Prometheus client artifacts in micrometer-registry-prometheus. This is a breaking change compared to the code that was there before. This is unavoidable due to the breaking changes in the Prometheus client from 0.x to 1.x. This moves the previous code from micrometer-registry-prometheus to a new module micrometer-registry-prometheus-simpleclient that is deprecated but available to give users a backward compatible option in case they need to make changes to use the new Prometheus client versions.
The base package used in micrometer-registry-prometheus was changed to
io.micrometer.prometheusmetricsto differentiate it from the prior package now used in micrometer-registry-prometheus-simpleclient. This avoids split packages and allows both modules to be used in the same application even.Known issues in the micrometer-registry-prometheus module after this PR:
Connected issues/PRs:
it was updated to use the -simpleclient module for now.