Skip to content
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

Avoid classloading LatencyUtils when not configured #3262

Merged

Conversation

shakuzen
Copy link
Member

@shakuzen shakuzen commented Jul 5, 2022

The default MeterRegistry configuration uses a NoPauseDetector, which does not need to use any LatencyUtils classes. This change avoids classloading LatencyUtils classes, which makes it possible for end users to exclude the LatencyUtils dependency and not have any issues at runtime.

Without this change, when excluding the LatencyUtils dependency from your application classpath, the following exception is thrown when registering a Timer.

java.lang.NoClassDefFoundError: org/LatencyUtils/PauseDetector
	at io.micrometer.core.instrument.AbstractTimer.initPauseDetector(AbstractTimer.java:104) ~[micrometer-core-1.9.1.jar:1.9.1]
	at io.micrometer.core.instrument.AbstractTimer.<init>(AbstractTimer.java:84) ~[micrometer-core-1.9.1.jar:1.9.1]
	at io.micrometer.prometheus.PrometheusTimer.<init>(PrometheusTimer.java:57) ~[micrometer-registry-prometheus-1.9.1.jar:1.9.1]
	at io.micrometer.prometheus.PrometheusMeterRegistry.newTimer(PrometheusMeterRegistry.java:320) ~[micrometer-registry-prometheus-1.9.1.jar:1.9.1]
	at io.micrometer.core.instrument.MeterRegistry.lambda$timer$2(MeterRegistry.java:318) ~[micrometer-core-1.9.1.jar:1.9.1]
	at io.micrometer.core.instrument.MeterRegistry.getOrCreateMeter(MeterRegistry.java:618) ~[micrometer-core-1.9.1.jar:1.9.1]
	at io.micrometer.core.instrument.MeterRegistry.registerMeterIfNecessary(MeterRegistry.java:570) ~[micrometer-core-1.9.1.jar:1.9.1]
	at io.micrometer.core.instrument.MeterRegistry.timer(MeterRegistry.java:316) ~[micrometer-core-1.9.1.jar:1.9.1]
	at io.micrometer.core.instrument.Timer$Builder.register(Timer.java:399) ~[micrometer-core-1.9.1.jar:1.9.1]
	at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.getTimer(WebMvcMetricsFilter.java:161) ~[spring-boot-actuator-2.7.1.jar:2.7.1]

Closes #1599

The default configuration uses a NoPauseDetector, which need not use any LatencyUtils classes. This avoids classloading LatencyUtils classes, which makes it possible for end users to exclude the LatencyUtils dependency and not have any issues at runtime.
The Prometheus sample should work without LatencyUtils or HdrHistogram since configuration that would use either is not present in the sample.
@shakuzen
Copy link
Member Author

shakuzen commented Jul 5, 2022

I would like to directly test that a user can exclude e.g. LatencyUtils from their classpath and not have issues as long as they are not configuring a PauseDetector. However, I could not find an easy way to run one test with a different classpath. Spring Boot has the ModifiedClassPathExtension to achieve what we want. We may want to consider copying that. It does seem to have some reflection depending on JUnit internals, which is not ideal, but it's also not ideal to not directly test behavior we intend to provide to users.

@shakuzen shakuzen merged commit bce0e58 into micrometer-metrics:1.8.x Jul 5, 2022
@shakuzen shakuzen deleted the avoid-classloading-optional-deps branch July 5, 2022 07:52
@shakuzen shakuzen linked an issue Jul 5, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accidental strict dep on LatencyUtils
1 participant