We have metrics support for Tomcat and micrometer has support for Jetty so we should configure that as well for consistency.
Comment From: wilkinsona
I looked at this briefly when I was adding support for Tomcat's metrics. It's not completely straightforward as it relies on Jetty being configured with a StatisticsHandler
which it isn't by default. I wondered if the Jetty metrics auto-configuration should customise Jetty with a StatisticsHandler
but didn't manage to convince myself it was the right thing to do.
Comment From: wilkinsona
Given the potential complications with StatisticsHandler
, we decided that this can wait until 2.1.
Comment From: evenh
In my mind this sounds pretty useful. If I were to have a look at this, where would I start (never contributed to the boot codebase before)?
Regarding adding a StatisticsHandler
customization to Jetty, one option would be to add a property for explicitly disabling it?
Comment From: snicoll
@evenh we've decided to postpone this to 2.1 (it's not a matter of a lack of time working on it). Thanks anyway. You can look at the contributing page to get started. We're also on Gitter if you want to chat.
Comment From: davidkarlsen
Should this be picked up again? https://github.com/micrometer-metrics/micrometer/blob/master/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/TimedHandler.java
Comment From: wilkinsona
Thanks, @davidkarlsen. Unfortunately, TimedHandler
makes things more complicated rather than less. We now use Jetty's StatisticsHandler
when graceful shutdown is enabled and we would not want to use both StatisticsHandler
and TimedHandler
when Micrometer's on the classpath as well.
@jkschneider @shakuzen What was the motivation for creating TimedHandler
rather than using Jetty's StatisticsHandler
?
Comment From: jkschneider
@wilkinsona Spring's http.server.requests
> TimedHandler
> StatisticsHandler
. Because you have good framework-level instrumentation of API endpoints, adding TimedHandler
or StatisticsHandler
would not be useful.
TimedHandler
is like StatisticsHandler
, but much higher resolution. Rather than relying on StatisticsHandler
's pre-aggregation of request throughput, etc. which are lossy it uses Micrometer timers.
TimedHandler
was designed for projects that are using the Jetty API more directly, like Javalin to get a request-level metrics experience similar to Spring Boot's WebMvc and WebFlux metrics.
For Spring Boot, I think just stick with what you've already got for http.server.requests
and maybe autoconfigure the JettyServerThreadPoolMetrics
, JettyConnectionMetrics
, and JettySslHandshakeMetrics
binders? As LongTaskTimer
continues to evolve into a more general purpose measure of saturation, the pool and connection metrics will just keep getting more and more useful.
Comment From: nyilmaz
@wilkinsona JettyServerThreadPoolMetrics
is now supported by JettyMetricsAutoConfiguration
. Would *Metrics
classes which @jkschneider mentioned, be considered for further auto configuration?
Comment From: wilkinsona
@nyilmaz Yes, we can consider them. I think, although can't remember for certain, that's why I left this issue open when https://github.com/spring-projects/spring-boot/issues/14591 was implemented.
Comment From: nyilmaz
oh nice, any plans or target release candidates for this issue?
Comment From: wilkinsona
The issue's assigned to the general backlog milestone which means we don't have any concrete plans for it at the moment.
Comment From: wilkinsona
We now bind connection and SSL handshake metrics in addition to the existing thread pool metrics binding so I think we're done here thanks to @bono007.
Comment From: nyilmaz
@wilkinsona @snicoll is there any chance to include TimedHandler
to the autoconfiguration process?
Comment From: wilkinsona
As per Jon's comment above, that's not something we're considering at the moment.