Micrometer: Support ScheduledExecutorService per-execution monitoring

[bsideup]

Hi!

The latest version of Micrometer (1.1.0) supports per-execution ExecutorService monitoring via TimedExecutorService.

In Project Reactor, we use ScheduledExecutorService (extended version of ExecutorService).
Would be nice if Micrometer will add it to ExecutorServiceMetrics, so that we can transparently reuse the infrastructure provided by Micrometer.

See https://github.com/reactor/reactor-core/pull/1281

[jkschneider]

Sorry, I'm not clear on what you are looking for.

[bsideup]

Hi @jkschneider,

We're looking for a built-in instrumentation for JVM's ScheduledExecutorService in addition to currently implemented ExecutorService, so that we can return it from here:
https://github.com/reactor/reactor-core/blob/4f9e176f9e49ad3311894433e99d7b04cb33ca68/reactor-core/src/main/java/reactor/core/scheduler/SchedulerMetricDecorator.java#L82-L85

[jkschneider]

Since ScheduledExecutorService is an ExecutorService, what needs to change?

[fragonib]

My understanding is that timed submitted tasks within ExecutorService implies a micrometer strategy that uses a wrapping executor with TimedExecutorService.

Therefore I guess that just using current ExecutorService instrumentation leave some ScheduledExecutorService mehods that submit tasks without being wrapped with Timer. Methods ScheduledExecutorService.schedule, scheduleAtFixedRate and scheduleWithFixedDelay doesn't submit a task wrapped with Timer.

Also an ExecutorServiceMetrics.monitor specialized for ScheduledExecutorService would be nice.

[fragonib]

Is there any plan to add time meter support for particular methods provided by ScheduledExecutorService? Is this included yet in your roadmap?

[shakuzen]

@fragonib this issue is currently assigned to the 1.2.0 milestone. We don't have a target release date for that yet, but you can check for those in the milestones tab: https://github.com/micrometer-metrics/micrometer/milestones

[slovdahl]

Is someone working on this already? If not, I would be willing to make a contribution.

[slovdahl]

I have a WIP branch on https://github.com/slovdahl/micrometer/tree/scheduled-executor-service-timings, I'll submit a PR tomorrow unless there are any objections.

[bsideup]

Hi @slovdahl,

it would be really nice if you submit it! I'm not a maintainer of Micrometer, but a consumer, and we would benefit from having it in Micrometer 👍

@shakuzen WDYT?

[mweirauch]

When working on this, there should be - not necessarily on this issue - a consolidation of metric and tag naming between Executor, ExecutorService and SchedulerExecturService metrics as mentioned in related tickets.

[slovdahl]

I tried rebasing the branch on upstream master, but there were some conflicts. I'm lacking the time to investigate further at the moment, but I'll try to pick it up sometime in the future.

If someone else has the time, feel free to take my WIP branch and submit a pull request.

[shakuzen]

Hi @slovdahl,

it would be really nice if you submit it! I'm not a maintainer of Micrometer, but a consumer, and we would benefit from having it in Micrometer 👍

@shakuzen WDYT?

A pull request would likely help move this along. I'll try to get to it in time for the 1.3 release regardless, but there's a lot to do for this release and time is limited.

[shakuzen]

@bsideup let us know if what was merged works for your purposes.

[bsideup]

@shakuzen thanks a lot! I will try to integrate it into reactor-core and report back if anything 👍