Skip to content

feat(jmc-agent): add handler for retrieving available probe templates #1126

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

Merged

Conversation

Josh-Matsuoka
Copy link
Contributor

@Josh-Matsuoka Josh-Matsuoka commented Oct 18, 2022

Small PR to add a handler for retrieving currently available probe templates, this is needed for the Agent Plugin frontend. Also fixes an issue with the notification category for the delete handler and tweaks the way the TargetProbesGetHandler outputs the active probes for the frontend.

Keeping as Draft until I add tests for the new handler

Depends on cryostatio/cryostat-core#156

Josh-Matsuoka and others added 30 commits September 21, 2021 14:13
…a/container-jfr into agent-plugin-agent-handlers
andrewazores and others added 10 commits October 20, 2022 16:31
* build(base): update base image

* update mergify config
default values have been thoroughly tested over a long time and appear to be overly cautious. JDP "settling" doesn't seem to happen and is more likely to indicate a broken implementation than a natural effect.
…only (cryostatio#1132)

* build(frontend): do not run frontend tests on build

* block PRs manually updating web-client

* reflow
…-archives view (cryostatio#1136)

* added lost folder, refactored some special cases, fixed metadata interaction with all archives view and deletionFromPath

* fixed uts
mergify[bot]
mergify bot previously requested changes Oct 20, 2022
Copy link

@mergify mergify bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request blocked. web-client submodule updates are performed automatically by CI when that repository is updated. Please revert or drop all changes to the web-client submodule from this PR and perform any required frontend work by opening and merging a PR against cryostat-web.

@andrewazores
Copy link
Member

Nice, that removal of connection.connect() actually revealed a bug, since I don't think anywhere else in Cryostat actually uses connection.getHandle() like that.

SEVERE: HTTP 500: Cannot invoke "org.openjdk.jmc.rjmx.IConnectionHandle.getServiceOrDummy(java.lang.Class)" because "connectionHandle" is null
io.vertx.ext.web.handler.HttpException: Internal Server Error
Caused by: io.cryostat.net.web.http.api.v2.ApiException: Cannot invoke "org.openjdk.jmc.rjmx.IConnectionHandle.getServiceOrDummy(java.lang.Class)" because "connectionHandle" is null
	at io.cryostat.net.web.http.api.v2.AbstractV2RequestHandler.handle(AbstractV2RequestHandler.java:112)
	at io.cryostat.net.web.http.api.v2.AbstractV2RequestHandler.handle(AbstractV2RequestHandler.java:68)
	at io.vertx.ext.web.impl.BlockingHandlerDecorator.lambda$handle$0(BlockingHandlerDecorator.java:48)
	at io.vertx.core.impl.ContextImpl.lambda$null$0(ContextImpl.java:159)
	at io.vertx.core.impl.AbstractContext.dispatch(AbstractContext.java:100)
	at io.vertx.core.impl.ContextImpl.lambda$executeBlocking$1(ContextImpl.java:157)
	at io.vertx.core.impl.TaskQueue.run(TaskQueue.java:76)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.NullPointerException: Cannot invoke "org.openjdk.jmc.rjmx.IConnectionHandle.getServiceOrDummy(java.lang.Class)" because "connectionHandle" is null
	at io.cryostat.core.agent.AgentJMXHelper.<init>(AgentJMXHelper.java:68)
	at io.cryostat.net.web.http.api.v2.TargetProbesGetHandler.lambda$handle$0(TargetProbesGetHandler.java:113)
	at io.cryostat.net.TargetConnectionManager.executeConnectedTask(TargetConnectionManager.java:163)
	at io.cryostat.net.web.http.api.v2.TargetProbesGetHandler.handle(TargetProbesGetHandler.java:109)
	at io.cryostat.net.web.http.api.v2.AbstractV2RequestHandler.handle(AbstractV2RequestHandler.java:102)
	... 10 more

The problem is here, in -core's JFRConnection class:

    public synchronized IConnectionHandle getHandle() {
        return this.handle;
    }

    public synchronized IFlightRecorderService getService()
            throws ConnectionException, IOException, ServiceNotAvailableException {
        if (!isConnected()) {
            connect();
        }
        IFlightRecorderService service = serviceFactory.getServiceInstance(handle);
        if (service == null || !isConnected()) {
            throw new ConnectionException(
                    String.format(
                            "Could not connect to remote target %s",
                            this.connectionDescriptor.createJMXServiceURL().toString()));
        }
        return service;
    }

This could either be fixed in -core by checking for a connection before returning from getHandle(), or it could be fixed in Cryostat's TargetConnectionManager by calling connect() on the connection instance before invoking the connected task on it. I'm inclined to say that doing it in -core is the better fix.

@Josh-Matsuoka
Copy link
Contributor Author

Fixed the getHandle issue in the core PR, and made a quick adjustment here to accompany it

@github-actions
Copy link
Contributor

This PR/issue depends on:

@andrewazores andrewazores merged commit 07894f1 into cryostatio:main Oct 21, 2022
mergify bot pushed a commit that referenced this pull request Oct 21, 2022
…#1126)

* Adding agent plugin template management handlers

* Adding probe directory creation to the launch script

* Fixing missing lisence header, adding resourceActions

* Adding Tests, moving handlers to beta handlers, cleanup

* Adding agent handlers, adjusting run script

* Cleanup, adding tests

* Fixing HttpApiBetaModule

* Cleanup, enforcing file upload name for ProbeTemplateUploadHandler

* Rebasing after core changes, cleanup

* bumping core pom version

* Fixing run script and integration test setup

* fixing core version

* fixing spotbugs issues

* running spotless

* running spotless

* Fixing pom.xml and template module sanity checks

* Merging changes from upstream

* Add probe template fetching handler

* Remove entrypoint change for testing

* API Handler fixes/cleanup

* Running spotless

* Fixing spotbugs failure

* Adjusting notification structure

* Improve error handling for TargetProbesGet

* Moving probe mount location

* create probes directory if needed

* Removing unnecessary connect calls

* build(base): update base image (#1120)

* build(base): update base image

* update mergify config

* fix(archives): fix various archives and metadata related issues (#1122)

* init commit on some fixes

* changed some comments

* build(web-client): update submodule to a35c409

* build(web-client): update submodule to 2ac50cd

* build(web-client): update submodule to 5a52a48

* test(itest): reduce JDP wait times (#1127)

default values have been thoroughly tested over a long time and appear to be overly cautious. JDP "settling" doesn't seem to happen and is more likely to indicate a broken implementation than a natural effect.

* build(web-client): update submodule to e41e2c8

* build(pom): use different property for jasypt-hibernate5 (#1130)

* build(frontend): skip frontend tests, require frontend updated by CI only (#1132)

* build(frontend): do not run frontend tests on build

* block PRs manually updating web-client

* reflow

* fix(archives): fixed archive migration and metadata handling from all-archives view (#1136)

* added lost folder, refactored some special cases, fixed metadata interaction with all archives view and deletionFromPath

* fixed uts

* Accompanying adjustment for the getHandle fix in corE

* ignore probes directory

Co-authored-by: Andrew Azores <[email protected]>
Co-authored-by: Max Cao <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Elliott Baron <[email protected]>
Co-authored-by: Andrew Azores <[email protected]>
(cherry picked from commit 07894f1)
andrewazores added a commit that referenced this pull request Oct 21, 2022
… (backport #1126) (#1140)

* feat(jmc-agent): add handler for retrieving available probe templates (#1126)

* Adding agent plugin template management handlers

* Adding probe directory creation to the launch script

* Fixing missing lisence header, adding resourceActions

* Adding Tests, moving handlers to beta handlers, cleanup

* Adding agent handlers, adjusting run script

* Cleanup, adding tests

* Fixing HttpApiBetaModule

* Cleanup, enforcing file upload name for ProbeTemplateUploadHandler

* Rebasing after core changes, cleanup

* bumping core pom version

* Fixing run script and integration test setup

* fixing core version

* fixing spotbugs issues

* running spotless

* running spotless

* Fixing pom.xml and template module sanity checks

* Merging changes from upstream

* Add probe template fetching handler

* Remove entrypoint change for testing

* API Handler fixes/cleanup

* Running spotless

* Fixing spotbugs failure

* Adjusting notification structure

* Improve error handling for TargetProbesGet

* Moving probe mount location

* create probes directory if needed

* Removing unnecessary connect calls

* build(base): update base image (#1120)

* build(base): update base image

* update mergify config

* fix(archives): fix various archives and metadata related issues (#1122)

* init commit on some fixes

* changed some comments

* build(web-client): update submodule to a35c409

* build(web-client): update submodule to 2ac50cd

* build(web-client): update submodule to 5a52a48

* test(itest): reduce JDP wait times (#1127)

default values have been thoroughly tested over a long time and appear to be overly cautious. JDP "settling" doesn't seem to happen and is more likely to indicate a broken implementation than a natural effect.

* build(web-client): update submodule to e41e2c8

* build(pom): use different property for jasypt-hibernate5 (#1130)

* build(frontend): skip frontend tests, require frontend updated by CI only (#1132)

* build(frontend): do not run frontend tests on build

* block PRs manually updating web-client

* reflow

* fix(archives): fixed archive migration and metadata handling from all-archives view (#1136)

* added lost folder, refactored some special cases, fixed metadata interaction with all archives view and deletionFromPath

* fixed uts

* Accompanying adjustment for the getHandle fix in corE

* ignore probes directory

Co-authored-by: Andrew Azores <[email protected]>
Co-authored-by: Max Cao <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Elliott Baron <[email protected]>
Co-authored-by: Andrew Azores <[email protected]>
(cherry picked from commit 07894f1)

* bump -core version

Co-authored-by: Joshua Matsuoka <[email protected]>
Co-authored-by: Andrew Azores <[email protected]>
@andrewazores andrewazores mentioned this pull request Oct 25, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants