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

chore(jmc-core): update to JMC version 9.0.0 #362

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

aptmac
Copy link
Member

@aptmac aptmac commented Mar 25, 2024

Fixes #365

This PR goes against issues #22 and #173, in which it would be nice to not have to embed JMC sources into cryostat-core.

As of JMC 9.0.0, the requisite classes for cryostat-core to run have been moved into the JMC core libraries. This includes changes in existing packages, and the newly moved flightrecorder.configuration and rjmx.common packages.

At the moment, JMC 9 core libraries are available for download on the Adoptium repositories, and that's what this PR makes use of. If it'd be preferable to wait and use Maven Central supplied artifacts instead, I can just draft this PR and update it when they're released.

@aptmac aptmac requested a review from a team as a code owner March 25, 2024 18:35
@andrewazores
Copy link
Member

image

Love it.

@andrewazores
Copy link
Member

At the moment, JMC 9 core libraries are available for download on the Adoptium repositories, and that's what this PR makes use of. If it'd be preferable to wait and use Maven Central supplied artifacts instead, I can just draft this PR and update it when they're released.

#337 - I would prefer to wait a while for Maven Central publication, just to keep things a little tidier.

@andrewazores
Copy link
Member

Looks like it must be on Central now, since Dependabot picked it up over here first: cryostatio/jfr-datasource#284

@aptmac
Copy link
Member Author

aptmac commented Apr 19, 2024

Looks like it must be on Central now, since Dependabot picked it up over here first: cryostatio/jfr-datasource#284

Excellent, yeah I can see the updated versions at: https://central.sonatype.com/artifact/org.openjdk.jmc/common/versions

It doesn't look like flightrecorder.configuration or rjmx.common have made it yet though, so I'll wait for them to show up and then re-visit this PR.

@aptmac
Copy link
Member Author

aptmac commented Apr 24, 2024

Looks like it must be on Central now, since Dependabot picked it up over here first: cryostatio/jfr-datasource#284

Excellent, yeah I can see the updated versions at: https://central.sonatype.com/artifact/org.openjdk.jmc/common/versions

It doesn't look like flightrecorder.configuration or rjmx.common have made it yet though, so I'll wait for them to show up and then re-visit this PR.

I've opened https://bugs.openjdk.org/browse/JMC-8217 to track the inclusion of the missing core artifacts into Maven Central. Oracle opted to simply update the existing artifacts for 9.0.0 without adding the new ones.

When asked about it, the timeline went from 9.1.0 (2025-01-29), to a 9.0.X release (which might be on the horizon to fix a PID formatting issue), to a "we'll try to add them soon" in response to the above jira ticket.

@aptmac
Copy link
Member Author

aptmac commented Apr 30, 2024

I've rebased here, locally it's looking good with mvn verify across cryostat-reports, cryostat, and cryostat3.

What do you think is the best way to go about reviewing this work? The cryostat3 CI step isn't going to be too happy because core and reports will need the changes first. If you want to try it locally, I've prepared my later branches to use 2.31.0-SNAPSHOT so that you could locally build and test across cryostat-core -> cryostat-reports -> cryostat3, and then we could circle back once that's been verified.

See:

@andrewazores
Copy link
Member

The cryostat3 CI step isn't going to be too happy because core and reports will need the changes first.

Yep, that's fine. That step is there to help catch accidental breakages, but when they're expected then I'm fine with ignoring it and validating things manually.

@andrewazores
Copy link
Member

@aptmac
Copy link
Member Author

aptmac commented Apr 30, 2024

https://github.com/aptmac/cryostat3/tree/jmc9

404 here

Ah I still had the repo set to private, it should be visible now.

@andrewazores
Copy link
Member

Conflicting files
src/main/java/io/cryostat/core/templates/LocalStorageTemplateService.java

This file has been removed, so it's a trivial merge/rebase fix.

@andrewazores
Copy link
Member

Manual reviews are all looking good. The Agent also consumes this dependency but doesn't actually use any of the JMC pieces, so it shouldn't need any additional work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. safe-to-test
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] EventTypeMetadataV2 embedded source fork is outdated
3 participants