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 JettyClientMetrics compatible with Jetty12 #4609

Closed
Lybby26 opened this issue Jan 22, 2024 · 14 comments · Fixed by #4756
Closed

Make JettyClientMetrics compatible with Jetty12 #4609

Lybby26 opened this issue Jan 22, 2024 · 14 comments · Fixed by #4756
Labels
enhancement A general enhancement
Milestone

Comments

@Lybby26
Copy link

Lybby26 commented Jan 22, 2024

Please describe the feature request.
Micrometer JettyClientMetrics not working with Jetty 12, getting NoClassDefFoundError exception, since Jetty 12 renamed packages,
https://github.com/micrometer-metrics/micrometer/blob/1.12.x/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyClientMetrics.java

See imports below, which makes reference to old packages found in Jetty11
import org.eclipse.jetty.client.api.ContentProvider;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Result;

Rationale
Springboot 3.2.x was released adding Jetty12 support, but latest micrometer-core version 1.12.2 is not supporting them.

Additional context
This behavior is similar to tickets:
#4261
qos-ch/logback#719
akkinoc/logback-access-spring-boot-starter#374

@shakuzen
Copy link
Member

Thanks for bringing this to our attention. We have been trying to keep things compatible across as wide a range as is feasible. There was an unsuccessful attempt to request a micrometer module in the Jetty project, which would avoid these compatibility issues as the proposed jetty-micrometer module could be versioned with Jetty (and so the instrumentation would always compile against the version of Jetty matching its version). I also see that Jetty 10 and 11 are soon to be out of community support per jetty/jetty.project#10485. I suppose that's an opportunity to deprecate what we have in micrometer-core and micrometer-jetty11. We would need to introduce a new module micrometer-jetty12 that contains the instrumentation we support. We also have #4261 regarding Jetty 12 support.

I'll bring it up with the team and see about scheduling this work.

@shakuzen shakuzen added enhancement A general enhancement waiting for team An issue we need members of the team to review and removed waiting-for-triage labels Jan 23, 2024
@shakuzen shakuzen added this to the 1.x milestone Jan 23, 2024
@ben77reilly
Copy link

Thank you @Lybby26 and @shakuzen for bringing this update related to Micrometer compatibility issue with Jetty 12.
So the community can expect some news about Jetty 12 support for next Micrometer version? Is the work already in progress so perhaps we can expect something coming up soon?

I really appreciate your help on this.

@shakuzen
Copy link
Member

The goal is to include it in our next minor release but work has not started on it yet. We'll update here when there's any news.

@Lybby26
Copy link
Author

Lybby26 commented Jan 26, 2024

thank you @shakuzen

@rulas
Copy link

rulas commented Feb 6, 2024

Hello Micrometer team, it's been two weeks since the last update, just checking in. Are there any positive developments on when this will be resolved? Appreciate any update on this. Thanks!

@pradeeppadhi
Copy link

Appreciate if there is any update on this ticket?

@shakuzen
Copy link
Member

I appreciate many people are waiting on this and want updates. Let me set expectations. Even if something were merged right now, the soonest it is planned to be available in a GA release of Micrometer is in May with the 1.13.0 release. So whether we merge something now or in the next couple of months, it isn't going to be in a stable version sooner. When there is an update, we will share it here.

I can understand that delay is frustrating since Jetty 12 was released in August 2023. Given the current setup of the instrumentation being maintained here and Micrometer feature releases being every 6 months (May and November), these kinds of delays are expected. We could do a better job of planning work to support these upgrades if we know ahead of time something is coming. In the case of Jetty 12, it wasn't on our radar in time to plan the work for it to minimize the gap. This kind of gap in support for Jetty+Micrometer users is the kind of thing that would be avoided if the instrumentation were a module in the Jetty project because it would be versioned and released along each release of Jetty. In that case, Micrometer instrumentation would be available and tested against the latest version of Jetty from the day of the Jetty release.

With those expectations and background set, it looks like we have a pull request for some initial Jetty 12 support from one of the Jetty maintainers: see #4753. We will review that in due course, and if/when something is merged that resolves things for JettyClientMetrics (what this issue is about), we will update things here.

@shakuzen shakuzen modified the milestones: 1.x, 1.13.x Feb 16, 2024
@shakuzen shakuzen removed the waiting for team An issue we need members of the team to review label Feb 16, 2024
@shakuzen shakuzen linked a pull request Feb 19, 2024 that will close this issue
@rulas
Copy link

rulas commented Feb 20, 2024

Hi @shakuzen, thanks the detailed update on this.

Do you have plans to release beta versions of this patch? if already available, when can this be downloaded from?

Raul

@shakuzen
Copy link
Member

You can follow the pull request #4756 the intends to resolve this issue. Once that is merged, snapshots will be automatically published. For instructions on consuming snapshots, see this section of the repository README.

shakuzen added a commit that referenced this issue Feb 28, 2024
Add JettyClientMetrics and related classes to a micrometer-jetty12 module and migrate Jetty API accordingly.

Resolves gh-4609
@shakuzen
Copy link
Member

The pull requests for Jetty 12 server and client instrumentation have been merged now and the new module micrometer-jetty12 is available in 1.13.0-SNAPSHOT in the snapshot repository - see this section of the documentation. They will be included in the upcoming 1.13.0-M2 milestone pre-release. Please try out the instrumentation and give us any feedback you have so we can make sure we get this right in time for the 1.13.0 GA release in May.

@shakuzen shakuzen modified the milestones: 1.13.x, 1.13.0-M2 Feb 28, 2024
@Lybby26
Copy link
Author

Lybby26 commented Mar 15, 2024

@shakuzen thanks for your reply, I just took 1.13.0-SNAPSHOT from maven { url "https://repo.spring.io/snapshot" }
implementation 'io.micrometer:micrometer-core:1.13.0-SNAPSHOT'
but still I see the reference to old packages and I'm getting same exception, could you please advice If I'm missing anything?

import org.eclipse.jetty.client.api.ContentProvider;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.client.api.Request.Listener;

thanks,

@shakuzen
Copy link
Member

@Lybby26 The instrumentation in micrometer-core that is incompatible with Jetty 12 has been marked deprecated and the deprecation message should point you to the new module micrometer-jetty12 that has the version of the instrumentation that is compatible.

@Lybby26
Copy link
Author

Lybby26 commented Mar 22, 2024

Hi @shakuzen I took snapshot 'io.micrometer:micrometer-jetty12:1.13.0-SNAPSHOT' and also I tested with io.micrometer:micrometer-jetty12:1.13.0-M2, and compilation issues solved.
Still not able to deploy and run to complete test due other dependencies issue in my application, but at least micrometer compilation issues were solved with this fix.

@shakuzen
Copy link
Member

Thanks for at least checking the compilation with the new artifact. Let us know if you find any issues when you are able to test more.

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