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

feat(attributes): add methods to extract all readable MBeanAttributes from various mxbeans #174

Merged
merged 14 commits into from
Feb 22, 2023

Conversation

maxcao13
Copy link
Member

@maxcao13 maxcao13 commented Feb 10, 2023

Signed-off-by: Max Cao macao@redhat.com

Related https://github.com/cryostatio/cryostat/issues/1360

There is a bunch of commented out methods that I tried to use before I realized that every JVM may have different attributes for these beans depending on their version so it doesn't make sense to have a field for each of them. I keep the comments just for now for convenience but I will remove them when this is OK'd.

How to test:
See https://github.com/cryostatio/cryostat/pull/1361

@andrewazores
Copy link
Member

Querying the memory field seems to not work currently:

query {
    targetNodes(filter: { name: "service:jmx:rmi:///jndi/rmi://cryostat:9093/jmxrmi" }) {
        jmxMetrics {
            runtime {
                inputArguments
            }
            thread {
                allThreadIds
                currentThreadCpuTime
                currentThreadUserTime
                threadCount
                peakThreadCount
            }
            os {
                arch
            }
            memory {
                heapMemoryUsage {
                    used
                }
                nonHeapMemoryUsage {
                    used
                }
            }
        }
    }
}
https --verify=no -v :8181/api/v2.2/graphql query=@query.graphql 'Authorization: Basic dXNlcjpwYXNzCg'
POST /api/v2.2/graphql HTTP/1.1
Accept: application/json, */*;q=0.5
Accept-Encoding: gzip, deflate
Authorization: Basic dXNlcjpwYXNzCg
Connection: keep-alive
Content-Length: 689
Content-Type: application/json
Host: localhost:8181
User-Agent: HTTPie/3.2.1

{
    "query": "query {\n    targetNodes(filter: { name: \"service:jmx:rmi:///jndi/rmi://cryostat:9093/jmxrmi\" }) {\n        jmxMetrics {\n            runtime {\n                inputArguments\n            }\n            thread {\n                allThreadIds\n                currentThreadCpuTime\n                currentThreadUserTime\n                threadCount\n                peakThreadCount\n            }\n            os {\n                arch\n            }\n            memory {\n                heapMemoryUsage {\n                    used\n                }\n                nonHeapMemoryUsage {\n                    used\n                }\n            }\n        }\n    }\n}\n"
}


HTTP/1.1 200 OK
content-encoding: gzip
content-length: 535
content-type: application/json

{
    "data": {
        "targetNodes": [
            {
                "jmxMetrics": {
                    "memory": {
                        "heapMemoryUsage": null,
                        "nonHeapMemoryUsage": null
                    },
                    "os": {
                        "arch": "amd64"
                    },
                    "runtime": {
                        "inputArguments": [
                            "-XX:+CrashOnOutOfMemoryError",
                            "-Dcom.sun.management.jmxremote.autodiscovery=true",
                            "-Dcom.sun.management.jmxremote.port=9093",
                            "-Dcom.sun.management.jmxremote.rmi.port=9093",
                            "-Dcom.sun.management.jmxremote.ssl.need.client.auth=false",
                            "-Dcom.sun.management.jmxremote.ssl=false",
                            "-Dcom.sun.management.jmxremote.registry.ssl=false",
                            "-Dcom.sun.management.jmxremote.authenticate=false"
                        ]
                    },
                    "thread": {
                        "allThreadIds": [
                            2,
                            3,
                            4,
                            23,
                            24,
                            26,
                            27,
                            28,
                            30,
                            31,
                            32,
                            33,
                            34,
                            35,
                            36,
                            37,
                            41,
                            42,
                            46,
                            47
                        ],
                        "currentThreadCpuTime": 19268024,
                        "currentThreadUserTime": 10000000,
                        "peakThreadCount": 20,
                        "threadCount": 20
                    }
                }
            }
        ]
    },
    "errors": [
        {
            "extensions": {
                "classification": "DataFetchingException"
            },
            "locations": [
                {
                    "column": 17,
                    "line": 18
                }
            ],
            "message": "Exception while fetching data (/targetNodes[0]/jmxMetrics/memory/heapMemoryUsage) : java.lang.reflect.InvocationTargetException",
            "path": [
                "targetNodes",
                0,
                "jmxMetrics",
                "memory",
                "heapMemoryUsage"
            ]
        },
        {
            "extensions": {
                "classification": "DataFetchingException"
            },
            "locations": [
                {
                    "column": 17,
                    "line": 21
                }
            ],
            "message": "Exception while fetching data (/targetNodes[0]/jmxMetrics/memory/nonHeapMemoryUsage) : java.lang.reflect.InvocationTargetException",
            "path": [
                "targetNodes",
                0,
                "jmxMetrics",
                "memory",
                "nonHeapMemoryUsage"
            ]
        }
    ]
}

@andrewazores
Copy link
Member

Feb 10, 2023 11:24:33 PM graphql.execution.SimpleDataFetcherExceptionHandler logException
WARNING: Exception while fetching data (/targetNodes[0]/jmxMetrics/memory/heapMemoryUsagePercent) : java.lang.reflect.InvocationTargetException
graphql.GraphQLException: java.lang.reflect.InvocationTargetException
	at graphql.schema.PropertyFetchingImpl.invokeMethod(PropertyFetchingImpl.java:251)
	at graphql.schema.PropertyFetchingImpl.getPropertyViaGetterUsingPrefix(PropertyFetchingImpl.java:142)
	at graphql.schema.PropertyFetchingImpl.getPropertyViaGetterMethod(PropertyFetchingImpl.java:135)
	at graphql.schema.PropertyFetchingImpl.getPropertyValue(PropertyFetchingImpl.java:93)
	at graphql.schema.PropertyDataFetcherHelper.getPropertyValue(PropertyDataFetcherHelper.java:19)
	at graphql.schema.PropertyDataFetcher.get(PropertyDataFetcher.java:120)
	at graphql.execution.ExecutionStrategy.fetchField(ExecutionStrategy.java:282)
	at graphql.execution.ExecutionStrategy.resolveFieldWithInfo(ExecutionStrategy.java:211)
	at graphql.execution.AsyncExecutionStrategy.execute(AsyncExecutionStrategy.java:59)
	at graphql.execution.ExecutionStrategy.completeValueForObject(ExecutionStrategy.java:670)
	at graphql.execution.ExecutionStrategy.completeValue(ExecutionStrategy.java:457)
	at graphql.execution.ExecutionStrategy.completeField(ExecutionStrategy.java:407)
	at graphql.execution.ExecutionStrategy.lambda$resolveFieldWithInfo$1(ExecutionStrategy.java:213)
	at java.base/java.util.concurrent.CompletableFuture.uniApplyNow(CompletableFuture.java:684)
	at java.base/java.util.concurrent.CompletableFuture.uniApplyStage(CompletableFuture.java:662)
	at java.base/java.util.concurrent.CompletableFuture.thenApply(CompletableFuture.java:2168)
	at graphql.execution.ExecutionStrategy.resolveFieldWithInfo(ExecutionStrategy.java:212)
	at graphql.execution.AsyncExecutionStrategy.execute(AsyncExecutionStrategy.java:59)
	at graphql.execution.ExecutionStrategy.completeValueForObject(ExecutionStrategy.java:670)
	at graphql.execution.ExecutionStrategy.completeValue(ExecutionStrategy.java:457)
	at graphql.execution.ExecutionStrategy.completeField(ExecutionStrategy.java:407)
	at graphql.execution.ExecutionStrategy.lambda$resolveFieldWithInfo$1(ExecutionStrategy.java:213)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1773)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at graphql.schema.PropertyFetchingImpl.invokeMethod(PropertyFetchingImpl.java:248)
	... 25 more
Caused by: java.lang.ClassCastException: class java.lang.Double cannot be cast to class java.lang.Long (java.lang.Double and java.lang.Long are in module java.base of loader 'bootstrap')
	at io.cryostat.core.net.MemoryDetails.getHeapMemoryUsagePercent(MemoryDetails.java:72)
	... 30 more

@maxcao13
Copy link
Member Author

Oh, I know what I did, I just forgot that I was changed something accidently in the parseCompositeData method.

private Object parseObject(Object obj) {
if (obj instanceof CompositeData) {
CompositeData cd = (CompositeData) obj;
if (cd.getCompositeType().getTypeName().equals("java.lang.management.MemoryUsage"))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe rather than the string value this could be MemoryUsage.class.getName()

Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

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

Do you think it would be worthwhile to refactor the JVM ID function:

public synchronized String getJvmId() throws IDException, IOException {

to call into getMBeanMetrics()?

@maxcao13
Copy link
Member Author

maxcao13 commented Feb 21, 2023

Do you think it would be worthwhile to refactor the JVM ID function:

public synchronized String getJvmId() throws IDException, IOException {

to call into getMBeanMetrics()?

Hmm... it would be convenient to put all these metric data things into one function but there's one thing I'm wondering about.

To get the jvmID then using graphql, the server (-core) side also has to calculate all the mbean attributes before the graphql fetcher filters the output so there's a bit more overhead from that. Unless that can be changed so that the -core side only retrieves certain mbean attributes specified by the graphql query. That would be nice but I'm not sure how to do that. Which do you think is best?

@andrewazores
Copy link
Member

This needs some profiling work to determine for sure but I suspect that the large bulk of the overhead comes from actually establishing the JMX connection. Once it's opened the various MBean operations should be relatively negligible, so picking and choosing which to do vs just doing them all and filtering out the response data to serialize out doesn't seem like it should be too bad. This is an operation that should only be occurring once every few seconds at most per target JVM while the user is looking at its monitoring dashboard, so it doesn't seem like the overall performance overhead should be substantial.

@maxcao13
Copy link
Member Author

Does something like this make sense?

andrewazores
andrewazores previously approved these changes Feb 22, 2023
@andrewazores andrewazores force-pushed the jvm-jmx-bean-attributes branch from 1dd4cbf to 97245b6 Compare February 22, 2023 16:47
@andrewazores andrewazores force-pushed the jvm-jmx-bean-attributes branch 2 times, most recently from 555bd19 to 6983d3f Compare February 22, 2023 17:14
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
@andrewazores andrewazores force-pushed the jvm-jmx-bean-attributes branch from 6983d3f to 7d815a7 Compare February 22, 2023 17:26
@andrewazores andrewazores merged commit 31234ee into cryostatio:main Feb 22, 2023
@maxcao13 maxcao13 deleted the jvm-jmx-bean-attributes branch February 22, 2023 19:01
Josh-Matsuoka pushed a commit to Josh-Matsuoka/cryostat-core that referenced this pull request Apr 14, 2023
… from various mxbeans (cryostatio#174)

* add details methods

Signed-off-by: Max Cao <macao@redhat.com>

* collectors import

* use explicit attributes

Signed-off-by: Max Cao <macao@redhat.com>

* remove overrides

Signed-off-by: Max Cao <macao@redhat.com>

* refactor to remove details from query field

Signed-off-by: Max Cao <macao@redhat.com>

* refactor

* some cleanup

Signed-off-by: Max Cao <macao@redhat.com>

* default values for gson

Signed-off-by: Max Cao <macao@redhat.com>

* refactor to remove extra 'attributes' field

Signed-off-by: Max Cao <macao@redhat.com>

* remove unnecessary function

Signed-off-by: Max Cao <macao@redhat.com>

* remove custom type

Signed-off-by: Max Cao <macao@redhat.com>

* change hardcoded classname to use actual class name

Signed-off-by: Max Cao <macao@redhat.com>

* add jvmId to mbeanMetrics

Signed-off-by: Max Cao <macao@redhat.com>

* writeLong

Signed-off-by: Max Cao <macao@redhat.com>

---------

Signed-off-by: Max Cao <macao@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants