Fix #16109: support Micrometer 1.13+ Prometheus package in MetricsSupportUtil#16205
Closed
barry3406 wants to merge 1 commit intoapache:3.3from
Closed
Fix #16109: support Micrometer 1.13+ Prometheus package in MetricsSupportUtil#16205barry3406 wants to merge 1 commit intoapache:3.3from
barry3406 wants to merge 1 commit intoapache:3.3from
Conversation
…icsSupportUtil Update isSupportPrometheus() to recognize both the legacy io.micrometer.prometheus.PrometheusConfig and the new io.micrometer.prometheusmetrics.PrometheusConfig introduced in Micrometer 1.13 (Spring Boot 3.3+). Drop the hard requirement on the io.prometheus.client.exporter.* classes: they are only exercised in Pushgateway mode and PrometheusMetricsReporterFactory already guards against their absence via NoClassDefFoundError handling, so requiring them here produced false negatives for scrape-only deployments that ship with dubbo-observability-spring-boot-starter in Dubbo 3.3.x. Add a MetricsSupportUtilTest that exercises each classpath layout (legacy only, new only, both, neither) by stubbing ClassUtils so the detection is verified regardless of which Prometheus registry is on the test runtime classpath.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #16205 +/- ##
=========================================
Coverage 60.79% 60.79%
- Complexity 11751 11756 +5
=========================================
Files 1953 1953
Lines 89119 89116 -3
Branches 13444 13441 -3
=========================================
Hits 54177 54177
- Misses 29368 29376 +8
+ Partials 5574 5563 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Author
|
Closing this — sorry, I didn't notice that #16110, #16128, and #16132 (from the original reporter @SavitarC) were already open for the same issue. The minimal-fix approach is already covered by #16110, and shouldn't have added a fourth competing PR. The unit tests might be a useful follow-up to whichever of those ends up being merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change?
Fixes #16109.
MetricsSupportUtil.isSupportPrometheus()currently only looks forio.micrometer.prometheus.PrometheusConfig, which was renamed toio.micrometer.prometheusmetrics.PrometheusConfigin Micrometer 1.13 (Spring Boot 3.3+), and also hard-requires the legacyio.prometheus.client.exporter.*classes that are no longer pulled in bydubbo-observability-spring-boot-starterin Dubbo 3.3.x. The net effect is that Prometheus reporter initialization is silently skipped for recent Spring Boot users, exactly matching the reproducer in the issue.This change updates the check to accept either the legacy
io.micrometer.prometheuspackage or the newio.micrometer.prometheusmetricspackage, and drops the legacyio.prometheus.client.*requirements. Those are only needed in Pushgateway mode, andPrometheusMetricsReporterFactoryalready handles their absence viaNoClassDefFoundError, so the extraisClassPresenthop here was just producing false negatives for scrape-only deployments.Per @RainYuY's note on the issue that a compatible path is what's needed, this keeps the fix minimal and strictly scoped to the detection util so older Spring Boot 3.2.x / Micrometer < 1.13 users keep working.
Checklist
MetricsSupportUtilTestcovers the four classpath layouts (legacy only, new only, both, neither) by stubbingClassUtilswithmockito-inline, so detection is exercised regardless of which Micrometer Prometheus registry happens to be on the test runtime classpath.mvn -pl dubbo-metrics/dubbo-metrics-api -am -Psources,skip-spotless,checkstyle clean install -Dmaven.test.skip=true,mvn -pl dubbo-metrics/dubbo-metrics-api -Pjacoco,skip-spotless test verify,mvn -pl dubbo-metrics/dubbo-metrics-prometheus -Pjacoco,skip-spotless test verify, plusmvn -pl dubbo-config/dubbo-config-api -Dtest=DefaultApplicationDeployerTest testandmvn spotless:checkon JDK 21 — all green.