Expose ForkJoinPool parallelism and pool size metrics

[genuss]

Hello, contributors,

I recently switched to using virtual threads, so became a big user of ForkJoinPool. When I looked at metrics, exposed by micrometer, I saw that parallelism isn't exposed. Is there a reason for this? Well, if not, I believe this could be a nice addition to already exposed ones.

I hesitated to add ForkJoinPool::getPoolSize, as I'm not sure this one is important, but I could add it too. What do you think about it?

[jonatan-ivanov]

I saw that parallelism isn't exposed. Is there a reason for this?

I think no one asked for it so far. What is your use-case behind parallelism? Are you setting it explicitly? By default, it uses the value of Runtime.getRuntime().availableProcessors() (cpu count) which Micrometer already reports but if you set it explicitly, it can have other values.

I think based on the use-case, the pool size can also be useful since its value will be the same as parallelism if enough tasks are submitted but if not, it can be lower, please feel free to add a Gauge for that too.

I also wanted to ask you to write some tests but then I realized there are no tests for ForkJoinPool in ExecutorServiceMetricsTest. :(
Let me know if you want to add some, otherwise I will take a look at them.

/cc @shakuzen

[genuss]

I think no one asked for it so far. What is your use-case behind parallelism? Are you setting it explicitly? By default, it uses the value of Runtime.getRuntime().availableProcessors() (cpu count) which Micrometer already reports but if you set it explicitly, it can have other values.

Well, I don't do it now, but I might in future as in container environment JVM's heuristics don't always work correct. Anyway, it's possible to create a new ForkJoinPool apart from the common one. In this case availableProcessors won't work too.

I also wanted to ask you to write some tests but then I realized there are no tests for ForkJoinPool in ExecutorServiceMetricsTest. :(

I added a simple one. Please tell me if you need more.

[genuss]

The failed test is this one:

JvmGcMetricsTest > gcTimingIsCorrectForPauseCycleCollectors() FAILED
    org.opentest4j.AssertionFailedError: 
    expected: 10L
     but was: 9L
        at app//io.micrometer.core.instrument.binder.jvm.JvmGcMetricsTest.lambda$checkPhaseCount$3(JvmGcMetricsTest.java:236)

6 tests completed, 1 failed

> Task :micrometer-core:zgcGenerationalTest FAILED

Doesn't look like a related failure, does it?

[jonatan-ivanov]

I might in future as in container environment JVM's heuristics don't always work correct

Yepp, that might be a valid scenario, though in that case upgrading Java might be a simpler task than modifying every place you use a pool. I think there could also be a valid (but very rare) use-case even if ergonomics sets the cpu size and the pool sizes correctly. In containerized environments, you can modify cgroup limits while the process is running. This can mean that the cpu count can increase or decrease over time and your pools might not. I haven't tried if ForkJoinPool does or not but even if it does, bugs can arise so knowing the parallelism level and the current pool size seems like a good idea to me.

I added a simple one. Please tell me if you need more.

Thank you! I also forgot to say thank you for opening the PR in the first place so double thank you! :)
I think we should add more but that should not effect this PR, I will add those tests separately. I think I would add some basic verifications for the values in the test of this PR but I can do those too during merging it.

Doesn't look like a related failure, does it?

No, that test is flaky which we should fix, I re-triggered the failed job and now it's green.

[genuss]

Thank you, folks!

[shakuzen]

Thank you for the pull request and the quick action on reviews.