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

Make Jetty server metrics work with Jetty 12 #4261

Closed
mvestola opened this issue Oct 22, 2023 · 4 comments · Fixed by #4753
Closed

Make Jetty server metrics work with Jetty 12 #4261

mvestola opened this issue Oct 22, 2023 · 4 comments · Fixed by #4753
Labels
enhancement A general enhancement
Milestone

Comments

@mvestola
Copy link

Description

Micrometer Jetty metrics do not work with Jetty 12. Micrometer references Jetty classes which are no longer there in Jetty 12.

Both TimedHandler and JettyConnectionMetrics has issues with Jetty 12.

TimedHandler is used to get request metrics (like how many 200 OK status requests) and JettyConnectionMetrics is used to get connection metrics.

Out of the box, JettyConnectionMetrics prints following to logs when using Jetty 12:

INFO o.e.j.i.AbstractConnection Failure while notifying listener imcibj.JettyConnectionMetrics@7786a3c6{STARTED} java.lang.NoClassDefFoundError: org/eclipse/jetty/server/HttpConnection

JettyConnectionMetrics references class org.eclipse.jetty.server.HttpConnection which is no longer there in Jetty 12 but moved to org.eclipse.jetty.server.internal.HttpConnection (might not be a good idea to use that at all since package name is "internal")

TimedHandler references these classes:

import org.eclipse.jetty.server.AsyncContextEvent;
import org.eclipse.jetty.server.HttpChannelState;
import org.eclipse.jetty.server.handler.HandlerWrapper;

These are moved to org.eclipse.jetty.ee9.nested.* in Jetty 12. Not sure if it is good to use these nested classes at all since then would depend on these org.eclipse.jetty.eeX packages.

So both TimedHandler and JettyConnectionMetrics would need some changes to make them work with Jetty 12.

Rationale

Jetty 12 has been released July 20, 2023 and it is the latest stable release of Jetty: https://projects.eclipse.org/projects/rt.jetty/releases/12.0 . There has been quite many changes in Jetty 12 compared to earlier versions. One of the biggest change is probably "The Servlet-Api has been removed from the internals of Jetty allowing for non-servlet-based application creation."

These changes in Jetty 12 seems to make micrometer jetty metrics not to work at all when using Jetty 12.

Package structure has changed in Jetty 12 a bit: https://webtide.com/new-jetty-12-maven-coordinates/

There has been some plans that Jetty 10 and 11 will probably reach end of life in couple of years, possibly in 2025, so would be good time to start preparing micrometer to also work with Jetty 12.

Additional context

Other libraries seem to also have some issues since Jetty 12 is quite new, e.g. logback has some issues with Jetty 12 due to package/class structure change: qos-ch/logback#719

@shakuzen shakuzen added enhancement A general enhancement and removed waiting-for-triage labels Oct 23, 2023
@shakuzen shakuzen added this to the 1.x milestone Oct 23, 2023
@gnilron
Copy link

gnilron commented Nov 2, 2023

Any news on this one?
Would be nice if this could get prioritized and be part of the upcoming SpringBoot 3.2 release as using SpringBoot-3.2-RC1 currently leads to repeating logs about NoClassDefFound.

@rdolejsi
Copy link

rdolejsi commented Nov 8, 2023

+1 on this one. I can confirm this is still not working with SpringBoot 3.2.0-RC2 BOM (having micrometer-core:1.12.0-RC1).

The issue we are seeing is exactly the one cited in the description (NoClassDefFoundError org/eclipse/jetty/server/HttpConnection). It is caused by Jetty 12 moving this class into package org/eclipse/jetty/server/internal.

Not sure if we can do anything to bump the attention on this issue and get it fixed in time for SpringBoot 3.2.0 official release this month?

@shakuzen
Copy link
Member

shakuzen commented Nov 9, 2023

I've opened #4324 specifically for JettyConnectionMetrics. I have some changes that should work to fix that. I've separated it from this issue so we can track the remaining things that need to be updated to support Jetty 12 here.

@joakime
Copy link
Contributor

joakime commented Feb 14, 2024

Opened PR #4753 to get this started for Jetty 12 server core

@shakuzen shakuzen modified the milestones: 1.x, 1.13.x Feb 16, 2024
@shakuzen shakuzen changed the title Make Jetty metrics work with Jetty 12 Make Jetty server metrics work with Jetty 12 Feb 16, 2024
@shakuzen shakuzen modified the milestones: 1.13.x, 1.13.0-M2 Feb 28, 2024
shakuzen pushed a commit that referenced this issue Feb 28, 2024
Ports the existing Jetty server instrumentation to support Jetty 12. This decouples the instrumentation from Servlet and instead depends only on Jetty core classes. Due to backward incompatibility with previous major versions of Jetty, this instrumentation needs to be in a new module micrometer-jetty12.

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

Successfully merging a pull request may close this issue.

5 participants