cryostat-legacy icon indicating copy to clipboard operation
cryostat-legacy copied to clipboard

feat(graphql): add aggregate information for active recordings in the GraphQL API

Open maxcao13 opened this issue 2 years ago • 7 comments

Fixes #1030

maxcao13 avatar Sep 14 '22 22:09 maxcao13

I think this will depend on a -web PR to be created as well: https://github.com/cryostatio/cryostat-web/pull/473#issuecomment-1223383346

andrewazores avatar Sep 15 '22 14:09 andrewazores

Related: https://github.com/cryostatio/cryostat/pull/1077

@maxcao13 can you resume work on this PR so we can be sure it gets into this release alongside the linked one above? Otherwise the API structure becomes asymmetric between active and archived.

andrewazores avatar Sep 23 '22 13:09 andrewazores

@maxcao13 can you resume work on this PR so we can be sure it gets into this release alongside the linked one above? Otherwise the API structure becomes asymmetric between active and archived.

Sure, no problem.

maxcao13 avatar Sep 23 '22 15:09 maxcao13

The PR doesn't included the new 'size' aggregate info, do you want me to include it here?

maxcao13 avatar Sep 23 '22 17:09 maxcao13

Also, a query like this:

$ http :8181/api/v2.2/graphql Authorization:"Basic $(echo user:pass | base64)" query="query { targetNodes { name recordings { active { aggregate { count } } }
 } }"

results in

{
    "data": null,
    "errors": [
        {
            "extensions": {
                "classification": "DataFetchingException"
            },
            "locations": [
                {
                    "column": 28,
                    "line": 1
                }
            ],
            "message": "Exception while fetching data (/targetNodes[4]/recordings) : Failed to retrieve RMIServer stub: javax.naming.CommunicationException [Root exception is java.rmi.ConnectIOException: non-JRMP server at remote endpoint]",
            "path": [
                "targetNodes",
                4,
                "recordings"
            ]
        }
    ]
}

if a target is unreachable (in my case 9095 with no SSL cert), but this is fixed in the archive endpoint PR #1047, by the try catch in the ActiveRecordingFetcher.

This is a query on reachable targets using smoketest (for me who doesn't know how to validate the SSL for 9095).

$ http :8181/api/v2.2/graphql Authorization:"Basic $(echo user:pass | base64)" query="query { environmentNodes(filter: { name: \"quarkus-test\" }) { name nodeType descendantTargets { recordings { active { data { name state metadata { labels } } } archived { data { name metadata { labels } } } } } } }"

maxcao13 avatar Sep 23 '22 18:09 maxcao13

The PR doesn't included the new 'size' aggregate info, do you want me to include it here?

I'm not sure if we can easily do that right now - the JDK Recording class has an accessor for this, but the JMC IRecordingDescriptor does not. The information is available over JMX with an accessor like the Recording one, but to use this we would need to update -core. This could be something to propose for JMC to add upstream, too.

In theory this would be a nice addition but in practice I don't think we should go for it, not right now.

andrewazores avatar Sep 24 '22 00:09 andrewazores

(for me who doesn't know how to validate the SSL for 9095).

https://github.com/cryostatio/cryostatio.github.io/pull/70

+

https://github.com/andrewazores/vertx-fib-demo/blob/master/src/main/extras/app/resources/vertx-fib-demo.cer

andrewazores avatar Sep 24 '22 00:09 andrewazores