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

Accidental strict dep on LatencyUtils #1599

Closed
codefromthecrypt opened this issue Sep 23, 2019 · 7 comments · Fixed by #3262
Closed

Accidental strict dep on LatencyUtils #1599

codefromthecrypt opened this issue Sep 23, 2019 · 7 comments · Fixed by #3262
Labels
bug A general bug module: micrometer-core An issue that is related to our core module
Milestone

Comments

@codefromthecrypt
Copy link

It seems there are some guards to try to not use LatencyUtils unless pause detection is in use. However, there's still a compile dep.

[INFO] Caused by: java.lang.ClassNotFoundException: org.LatencyUtils.IntervalEstimator
[INFO] 	at java.net.URLClassLoader.findClass(URLClassLoader.java:471) ~[?:?]
[INFO] 	at java.lang.ClassLoader.loadClass(ClassLoader.java:588) ~[?:?]
[INFO] 	at org.springframework.boot.loader.LaunchedURLClassLoader.loadClass(LaunchedURLClassLoader.java:93) ~[zipkin-server-2.16.3-SNAPSHOT-slim.jar:?]
[INFO] 	at java.lang.ClassLoader.loadClass(ClassLoader.java:521) ~[?:?]
[INFO] 	at io.micrometer.prometheus.PrometheusMeterRegistry.newTimer(PrometheusMeterRegistry.java:176) ~[micrometer-registry-prometheus-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.MeterRegistry.lambda$timer$2(MeterRegistry.java:271) ~[micrometer-core-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.MeterRegistry.getOrCreateMeter(MeterRegistry.java:576) ~[micrometer-core-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.MeterRegistry.registerMeterIfNecessary(MeterRegistry.java:529) ~[micrometer-core-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.MeterRegistry.timer(MeterRegistry.java:269) ~[micrometer-core-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.Timer$Builder.register(Timer.java:477) ~[micrometer-core-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.MeterRegistry.timer(MeterRegistry.java:396) ~[micrometer-core-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.internal.TimedExecutorService.<init>(TimedExecutorService.java:40) ~[micrometer-core-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.binder.jvm.ExecutorServiceMetrics.monitor(ExecutorServiceMetrics.java:94) ~[micrometer-core-1.2.1.jar!/:1.2.1]
[INFO] 	at io.micrometer.core.instrument.binder.jvm.ExecutorServiceMetrics.monitor(ExecutorServiceMetrics.java:107) ~[micrometer-core-1.2.1.jar!/:1.2.1]
@codefromthecrypt
Copy link
Author

here's an example of another project working around a similar accidental strict dep line/armeria#2107

@codefromthecrypt
Copy link
Author

It looks like HdrHistogram is also effectively mandatory, It would be nice for these to be optional.

java.lang.NoClassDefFoundError: org/HdrHistogram/DoubleRecorder
	at io.micrometer.core.instrument.distribution.TimeWindowPercentileHistogram.<init>(TimeWindowPercentileHistogram.java:37) ~[micrometer-core-1.2.1.jar!/:1.2.1]
	at io.micrometer.core.instrument.AbstractTimer.<init>(AbstractTimer.java:85) ~[micrometer-core-1.2.1.jar!/:1.2.1]
	at io.micrometer.prometheus.PrometheusTimer.<init>(PrometheusTimer.java:40) ~[micrometer-registry-prometheus-1.2.1.jar!/:1.2.1]
	at io.micrometer.prometheus.PrometheusMeterRegistry.newTimer(PrometheusMeterRegistry.java:176) ~[micrometer-registry-prometheus-1.2.1.jar!/:1.2.1]

@codefromthecrypt codefromthecrypt changed the title Accidental strict dep on LatencyUtils Accidental strict dep on LatencyUtils and HdrHistogram Sep 24, 2019
@shakuzen shakuzen added this to the 1.4.0 milestone Sep 29, 2019
@adriangonz
Copy link

Is this task currently prioritized for the next release? Is there any time estimate of when it would be ready?

If I can get a "deep dive tour" around the relevant areas of the codebase I would be happy to jump on this one!

@shakuzen shakuzen modified the milestones: 1.4.0, 1.5.0 Mar 8, 2020
@shakuzen shakuzen modified the milestones: 1.5.0, 1.6.0 Apr 23, 2020
@shakuzen shakuzen modified the milestones: 1.6.0, 1.7.0 Oct 29, 2020
@shakuzen shakuzen modified the milestones: 1.7.0-M1, 1.x Mar 2, 2021
@sheepdreamofandroids
Copy link

sheepdreamofandroids commented Jul 4, 2022

This problem still exists. At least declare the dependencies until you find a good way to make them optional.
My bad. I see now that the LatencyUtils dependency is declared. For some reason gradle 7.4.2 didn't include it in the jar until I explicitly declared as well. HdrHistogram was included out of the box for me.

@shakuzen
Copy link
Member

shakuzen commented Jul 5, 2022

LatencyUtils is currently a declared runtime dependency in micrometer-core. If you are using the default configuration (NoPauseDetector) then you should be able to exclude the LatencyUtils dependency in your application. In other words, it should be optional although it is declared. I tried excluding it and ran into an issue at runtime. I think we can easily improve that, at least, without affecting backwards compatibility.

@shakuzen shakuzen modified the milestones: 1.x, 1.8.8 Jul 5, 2022
@shakuzen shakuzen changed the title Accidental strict dep on LatencyUtils and HdrHistogram Accidental strict dep on LatencyUtils Jul 5, 2022
@shakuzen shakuzen added bug A general bug module: micrometer-core An issue that is related to our core module labels Jul 5, 2022
@shakuzen shakuzen linked a pull request Jul 5, 2022 that will close this issue
@shakuzen
Copy link
Member

shakuzen commented Jul 5, 2022

Resolved by #3262

@shakuzen shakuzen closed this as completed Jul 5, 2022
@shakuzen
Copy link
Member

shakuzen commented Jul 5, 2022

If you are not configuring a pause detector, you can exclude the LatencyUtils dependency.
If you are not configuring client-side percentiles, you can exclude the HdrHistogram dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug module: micrometer-core An issue that is related to our core module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants