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

Add metrics support for ThreadPoolTaskExecutor and ThreadPoolTaskScheduler #23818

Closed
aheritier opened this issue Oct 22, 2020 · 14 comments
Closed
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@aheritier
Copy link
Contributor

aheritier commented Oct 22, 2020

Micrometer allows to monitor the executor states using ExecutorServiceMetrics

ref: https://github.com/micrometer-metrics/micrometer/blob/master/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jvm/ExecutorServiceMetrics.java

It allows to monitor the usage of executors with metrics like:

    "async.executor",
    "async.executor.active",
    "async.executor.completed",
    "async.executor.idle",
    "async.executor.pool.core",
    "async.executor.pool.max",
    "async.executor.pool.size",
    "async.executor.queue.remaining",
    "async.executor.queued",

To achieve that we have to wrap our executors with something like this:

    /**
     * <p>A default task executor.</p>
     * <p>The {@link Executor} instance to be used when processing async
     * method invocations.</p>
     */
    @Bean(DEFAULT_TASK_EXECUTOR_BEAN_NAME)
    @Override
    public Executor getAsyncExecutor() {
        // Using a thread-pooling TaskExecutor implementation,
        // in particular for executing a large number of short-lived tasks.
        final ThreadPoolTaskExecutor executor = new TaskExecutorBuilder()
            .corePoolSize(100) // With unlimited queue
            .allowCoreThreadTimeOut(true)
            .threadNamePrefix("task-")
            .build();
        executor.initialize();
        return ExecutorServiceMetrics.monitor(
            meterRegistry,
            executor.getThreadPoolExecutor(),
            "AsyncExecutor",
            "async",
            tags);
    }

(I am creating the tags from MetricsProperties#getTags)

It might be great if an AutoConfiguration could do it magically (it's why we love Spring-Boot). Not sure how it could be implemented in a Spring-Boot way properly.

We can find such request in various places:

cc @snicoll

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 22, 2020
@snicoll snicoll changed the title Automatically wrap Executors using ExecutorServiceMetrics Add metrics support for thread pool async executors Oct 23, 2020
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 26, 2020
@snicoll snicoll added this to the 2.5.x milestone Oct 26, 2020
@snicoll
Copy link
Member

snicoll commented Oct 26, 2020

Thanks for the suggestion. I think it would be worthwhile to have a MeterBinder for TaskExecutor and TaskScheduler as those have a different hierarchy structure. I've triaged this for our next feature release.

@aheritier
Copy link
Contributor Author

I agree @snicoll it makes sense to me. thanks a lot

@aheritier
Copy link
Contributor Author

My approach seems to be wrong @snicoll
I just discovered that I am loosing the default metrics from spring boot

I also notice that it added a lot of warnings is not eligible for getting processed by all BeanPostProcessors

2020-10-29 19:41:53.110  INFO 71216 --- [  restartedMain] trationDelegate$BeanPostProcessorChecker : Bean 'org.springframework.boot.actuate.autoconfigure.metrics.export.statsd.StatsdMetricsExportAutoConfiguration' of type [org.springframework.boot.actuate.autoconfigure.metrics.export.statsd.StatsdMetricsExportAutoConfiguration] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-10-29 19:41:53.117  INFO 71216 --- [  restartedMain] trationDelegate$BeanPostProcessorChecker : Bean 'management.metrics.export.statsd-org.springframework.boot.actuate.autoconfigure.metrics.export.statsd.StatsdProperties' of type [org.springframework.boot.actuate.autoconfigure.metrics.export.statsd.StatsdProperties] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-10-29 19:41:53.117  INFO 71216 --- [  restartedMain] trationDelegate$BeanPostProcessorChecker : Bean 'statsdConfig' of type [org.springframework.boot.actuate.autoconfigure.metrics.export.statsd.StatsdPropertiesConfigAdapter] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-10-29 19:41:53.118  INFO 71216 --- [  restartedMain] trationDelegate$BeanPostProcessorChecker : Bean 'org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration' of type [org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-10-29 19:41:53.118  INFO 71216 --- [  restartedMain] trationDelegate$BeanPostProcessorChecker : Bean 'micrometerClock' of type [io.micrometer.core.instrument.Clock$1] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-10-29 19:41:53.120  INFO 71216 --- [  restartedMain] trationDelegate$BeanPostProcessorChecker : Bean 'statsdMeterRegistry' of type [io.micrometer.statsd.StatsdMeterRegistry] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-10-29 19:41:53.124  INFO 71216 --- [  restartedMain] trationDelegate$BeanPostProcessorChecker : Bean 'management.metrics-org.springframework.boot.actuate.autoconfigure.metrics.MetricsProperties' of type [org.springframework.boot.actuate.autoconfigure.metrics.MetricsProperties] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)

Something like this is helping

// https://stackoverflow.com/questions/57607445/spring-actuator-jvm-metrics-not-showing-when-globalmethodsecurity-is-enabled
// Do not loose default metrics
@Bean
InitializingBean forcePostProcessor(BeanPostProcessor meterRegistryPostProcessor) {
    return () -> meterRegistryPostProcessor.postProcessAfterInitialization(meterRegistry, "");
}

I still have the post processing errors but the the default metrics are back

@snicoll
Copy link
Member

snicoll commented Oct 29, 2020

@aheritier yes, we're not going to implement that feature this way. Spring Boot rather injects the component (i.e. thread pool) later on and register metrics on it, which avoids causing unnecessary early initialisations due to the dependency on the MeterRegistry. If you have questions about registering custom metrics, please follow-up on StackOverflow or come chat with the community on Gitter.

@aheritier
Copy link
Contributor Author

It's good I better understand now. Because I also implements AsyncConfigurer
Your approach using a MeterBinder similar to what is done for Cache and others will be the best approach
Thanks

@philwebb philwebb modified the milestones: 2.5.x, 2.6.x Apr 14, 2021
@snicoll snicoll self-assigned this Apr 21, 2021
@snicoll snicoll changed the title Add metrics support for thread pool async executors Add metrics support for ThreadPoolTaskExecutor and ThreadPoolTaskScheduler Apr 21, 2021
snicoll added a commit to snicoll/spring-boot that referenced this issue Apr 21, 2021
@snicoll
Copy link
Member

snicoll commented Apr 21, 2021

I've added a proposal in 910e14f. Unfortunately, it is too late for 2.5.x but I'll do my best to get that in for 2.6.0-M1. It handles both task executors and task schedulers using Micrometer's ExecutorServiceMetrics.

@aheritier
Copy link
Contributor Author

Awesome. Thanks a lot @snicoll

@aheritier
Copy link
Contributor Author

@snicoll I imported your code in my 2.5.x codebase and it's working fine.
I just had to be sure that my Beans were defined as ThreadPoolTaskExecutor and not simple Executor
Well done!

@snicoll
Copy link
Member

snicoll commented Jun 11, 2021

Thanks for testing Arnaud. Your feedback on the bean type is interesting, I might improve the auto-configuration to take that into account transparently.

@aheritier
Copy link
Contributor Author

yes not sure if you can make it transparent. For sure I was a bit confused because it wasn't working at the beginning with my Executors then I read the code and the doc to understand that using ThreadPoolTaskExecutor was the solution

@snicoll snicoll modified the milestones: 2.6.x, 2.6.0-M1 Jun 14, 2021
@csdbianhua
Copy link

csdbianhua commented Jun 23, 2021

@snicoll I have a question after reading 910e14f. I need to use the TimedExecutorService returned by ExecutorServiceMetrics#monitor to wrap the tasks I submitted, then I can get task execution time (executor.execution) and idle time (executor.idle). If you just iterate over ThreadPoolTaskExecutor and invoke ExecutorServiceMetrics#monitor, won't these metrics be lost?
Am I missing something?

@snicoll
Copy link
Member

snicoll commented Jun 23, 2021

Those two are unrelated, I think. You can give 2.6.0-SNAPSHOT a try. If that doesn't work, feel free to create a new issue with a small sample that allows us to reproduce the problem in the form of a sample project and we can take a look.

@artemik
Copy link

artemik commented Oct 4, 2024

@snicoll Hi, what's eventually the status of it? Is there a documentation on how to properly setup monitoring of task executors?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants