Library instrumentation for java.util.concurrent (metrics)

[ZawadaZSE]

Is your feature request related to a problem? Please describe.
We're migrating from opentracing and noticed that manual instrumentation for Executors is missing.

Describe the solution you'd like
I'd like to know if you plan to implement similar solution to: https://github.com/opentracing-contrib/java-concurrent that would allow manual instrumentation of Executors.

Describe alternatives you've considered
I know that it's supported with javaagent instrumentation but we'd rather not use it.

Additional context
Add any other context or screenshots about the feature request here.

[mateuszrzeszutek]

Hey @ZawadaZSE ,
Do you need to create spans for every submitted task? Or is it just about context propagation across threads? If it's the latter, there already are facilities to help you with that: you can use the Context.taskWrapping() method to decorate an ExecutorService/Executor with current context propagation.

[reactivejson]

Hey @mateuszrzeszutek
We are using Context.taskWrapping() method to decorate an ExecutorService.
But we are getting errors since we can't use io.opentelemetry.context.CurrentContextExecutorService with Micrometer ExecutorServiceMetrics:
i.m.c.i.b.jvm.ExecutorServiceMetrics - Failed to bind as io.opentelemetry.context.CurrentContextExecutorService is unsupported.

[mateuszrzeszutek]

Hey @reactivejson ,
Can you try using micrometer first, and then wrapping with Context.taskWrapping()?
E.g.

Context.taskWrapping(
    ExecutorServiceMetrics.monitor(meterRegistry, executorService, "myExecutor"))

[reactivejson]

Thanks @mateuszrzeszutek for your reply, I will check your suggestion.
One more issue we have is that we cant cast io.opentelemetry.context.CurrentContextExecutorService to ThreadPoolExecutor:
java.lang.ClassCastException: class io.opentelemetry.context.CurrentContextExecutorService cannot be cast to class java.util.concurrent.ThreadPoolExecutor (io.opentelemetry.context.CurrentContextExecutorService is in unnamed module of loader 'app'; java.util.concurrent.ThreadPoolExecutor is in module java.base of loader 'bootstrap')

Do you know how to approach it?

[reactivejson]

Hey @reactivejson , Can you try using micrometer first, and then wrapping with Context.taskWrapping()? E.g.

Context.taskWrapping(
    ExecutorServiceMetrics.monitor(meterRegistry, executorService, "myExecutor"))

We are beanifying it in Spring boot so we don't have much control over it, and it's used by Prometheus, so we can't use ExecutorServiceMetrics.monitor(meterRegistry, executorService, "myExecutor"))

Our code:
@Bean ExecutorServiceMetrics example(ExecutorService operationNotificationsExecutorService) { return new ExecutorServiceMetrics(operationNotificationsExecutorService, "operation-notifications", emptyList()); }

[mateuszrzeszutek]

One more issue we have is that we cant cast io.opentelemetry.context.CurrentContextExecutorService to ThreadPoolExecutor:
java.lang.ClassCastException: class io.opentelemetry.context.CurrentContextExecutorService cannot be cast to class java.util.concurrent.ThreadPoolExecutor (io.opentelemetry.context.CurrentContextExecutorService is in unnamed module of loader 'app'; java.util.concurrent.ThreadPoolExecutor is in module java.base of loader 'bootstrap')

That is expected, because CurrentContextExecutorService is not a ThreadPoolExecutor. In general, I'd recommend using the ExecutorService interface instead of ThreadPoolExecutor where it's possible.

[ZawadaZSE]

Hey,

@mateuszrzeszutek problem is that this is internal implementation of micrometer.

We're using ExecutorService interface everywhere in out code. We also use bean post processing to wrap all executor services in CurrentContextExecutorService so that we would not loose context anywhere.

I've checked how it was done in opentracing implementation and they had extended ThreadPoolExecutorService (io.opentracing.contrib.spring.cloud.async.instrument.TracedThreadPoolTaskExecutor).

Can something similar be done for opentelemetry? I feel that whole community could benefit of such change.

[mateuszrzeszutek]

@ZawadaZSE @reactivejson
Are you using spring-actuator to register the executor service metrics? Or are you doing it manually?
For OpenTelemetry, are you using the OpenTelemetry Spring Starter? Spring Cloud Sleuth? Or again, is it a manual setup?

[ZawadaZSE]

@mateuszrzeszutek
Yes, we're using spring-actuator and we're manually configuring ExecutorServiceMetrics beans like this:

    @Bean
    ExecutorServiceMetrics someExecutorServiceMetrics(ExecutorService someExecutorService) {
        return new ExecutorServiceMetrics(someExecutorService, "some-executor", emptyList());
    }

As for open telemetry, we're using spring starter but that one does not provide auto configuration for executors.
We wrote our own BeanPostProcessor which wraps all ExecutorService type beans in Context.taskWrapping(executorService)

[mateuszrzeszutek]

You can make use of the ExecutorServiceMetrics.monitor() function, which also returns an ExecutorService, and redefine the someExecutorService bean the following way:

@Bean 
ExecutorService someExecutorService(MeterRegistry meterRegistry) {
  ExecutorService executor = new ThreadPoolExecutor(...);
  return ExecutorServiceMetrics.monitor(meterRegistry, executor, "some-executor");
}

That way your BeanPostProcessor will wrap the already metricized executor.

I agree with you that the OpenTelemetry Spring starter should offer context propagation by default, and that it should play well with the Micrometer metrics installed by the Spring Actuator (that only measure Spring task executors), but I don't think that we can avoid wrapping Executors/ExecutorServices -- in the first place, the OpenTracing lib you mentioned does exactly that, it wraps standard Java executors, and has separate extending wrapper classes for spring types.