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

Project relies on jmc classes that aren't part of jmc-core #22

Open
jiekang opened this issue Jan 3, 2020 · 9 comments
Open

Project relies on jmc classes that aren't part of jmc-core #22

jiekang opened this issue Jan 3, 2020 · 9 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@jiekang
Copy link
Contributor

jiekang commented Jan 3, 2020

The project currently uses classes/packages from jmc that aren't part of jmc-core, the api bundle for jmc. These should be requested in jmc upstream to be moved into jmc-core (if appropriate).

This is a decent time to make the request as JMC is now on 8.0.0-SNAPSHOT, which suggests api changes like this could be made.

@jiekang jiekang self-assigned this Jan 3, 2020
@jiekang
Copy link
Contributor Author

jiekang commented Jan 3, 2020

@andrewazores Do you happen to have a list handy of the exact fully qualified class names this project uses that are in jmc, not jmc-core?

@andrewazores
Copy link
Member

I don't. The only list of classes I had [0] looked at were the ones in container-jfr which were from JMC, but not whether those came from jmc or jmc-core.

[0] cryostatio/cryostat-legacy#59 (comment)

@andrewazores
Copy link
Member

And that's a list of the classes that at that time were still not abstracted over and moved into this -core project, so the full set is larger than that since it needs to be unioned with the set of classes used in this codebase. The same or similar ag/grep/etc. is handy enough for getting that class list.

@jiekang
Copy link
Contributor Author

jiekang commented Jan 3, 2020

Ah so I should actually look at both container-jfr and container-jfr-core. Okay thanks!

@andrewazores
Copy link
Member

Yea, not everything has been moved into -core yet. Ideally all of the jmc API access is abstracted over or reimplemented by -core and the main container-jfr codebase contains zero references to any JMC classes, but that isn't the case right now.

@jiekang
Copy link
Contributor Author

jiekang commented Jan 3, 2020

List of imports not in jmc-core:

import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID;
import org.openjdk.jmc.flightrecorder.configuration.events.IEventTypeID;
import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder;

import org.openjdk.jmc.jdp.client.JDPClient;

import org.openjdk.jmc.rjmx.ConnectionDescriptorBuilder;
import org.openjdk.jmc.rjmx.ConnectionToolkit;
import org.openjdk.jmc.rjmx.IConnectionDescriptor;
import org.openjdk.jmc.rjmx.IConnectionHandle;
import org.openjdk.jmc.rjmx.IConnectionListener;
import org.openjdk.jmc.rjmx.internal.DefaultConnectionHandle;
import org.openjdk.jmc.rjmx.internal.RJMXConnection;
import org.openjdk.jmc.rjmx.internal.ServerDescriptor;
import org.openjdk.jmc.rjmx.internal.WrappedConnectionException;
import org.openjdk.jmc.rjmx.services.internal.CommercialFeaturesServiceFactory;

import org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException;
import org.openjdk.jmc.rjmx.services.jfr.IEventTypeInfo;
import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService;
import org.openjdk.jmc.rjmx.services.jfr.internal.FlightRecorderServiceFactory;
import org.openjdk.jmc.rjmx.services.jfr.internal.FlightRecorderServiceV2;
import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor;
import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor.RecordingState;

import org.openjdk.jmc.ui.common.security.ActionNotGrantedException;
import org.openjdk.jmc.ui.common.security.FailedToSaveException;
import org.openjdk.jmc.ui.common.security.ISecurityManager;
import org.openjdk.jmc.ui.common.security.SecurityException;

@jiekang
Copy link
Contributor Author

jiekang commented Jan 3, 2020

List of imports in jmc-core:

import org.openjdk.jmc.common.unit.IConstrainedMap;
import org.openjdk.jmc.common.unit.IConstraint;
import org.openjdk.jmc.common.unit.IDescribedMap;
import org.openjdk.jmc.common.unit.IMutableConstrainedMap;
import org.openjdk.jmc.common.unit.IOptionDescriptor;
import org.openjdk.jmc.common.unit.IQuantity;
import org.openjdk.jmc.common.unit.QuantityConversionException;
import org.openjdk.jmc.common.unit.UnitLookup;

import org.openjdk.jmc.flightrecorder.CouldNotLoadRecordingException;
import org.openjdk.jmc.flightrecorder.JfrLoaderToolkit;

@andrewazores andrewazores added bug Something isn't working help wanted Extra attention is needed labels Aug 4, 2020
@ebaron
Copy link
Member

ebaron commented Dec 22, 2020

#56 has a temporary fix for this, by maintaining an in-tree copy of JMC application classes that we use. A more permanent fix would be refactoring JMC upstream to move this functionality to core.

@aptmac
Copy link
Member

aptmac commented Dec 11, 2023

Last week the PRs for JMC-7308 (flightrecorder.configuration) [0] and JMC-7069 (rjmx.common) [1] were merged, which moves the classes listed in the comments above into the JMC core libraries.

These changes will be a part of the JMC 9 release, and should be available through maven central in the new year. JMC 9 is currently set to have a source release on January 24, 2024 [2].

I'll revisit these once the maven central release happens, but here are some branches that I prepared to make sure the cryostat repos build and pass all their unit + itests.

(2024-01-23: these branches are up-to-date and use the Adoptium JMC core snapshot jars)
cryostat-core: https://github.com/aptmac/cryostat-core/tree/core-9.0.0-SNAPSHOT
cryostat-reports: https://github.com/aptmac/cryostat-reports/tree/core-9.0.0-SNAPSHOT
cryostat: https://github.com/aptmac/cryostat/tree/core-9.0.0-SNAPSHOT
cryostat3: https://github.com/aptmac/cryostat3/tree/core-9.0.0-SNAPSHOT

These branches can also go towards: #173

[0] https://bugs.openjdk.org/browse/JMC-7308
[1] https://bugs.openjdk.org/browse/JMC-7069
[2] https://wiki.openjdk.org/display/jmc/Releases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants