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(mbeans): adjust MBeanMetrics GraphQL query for updated schema #1219

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Feb 29, 2024

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Depends on cryostatio/cryostat#307
Based on #1218

Description of the change:

Adjusts GraphQL queries for the updated schema in 3.0 where the mbeanMetrics subquery is nested under the Target object, not under the TargetNode (the DiscoveryNode which the Target belongs to). Adjusts the client's data model to always expect labels and annotations on Targets returned from the API, whether GraphQL or plain HTTP, and for both labels and annotations to use the new "key-value list" format.

Motivation for the change:

Restores the web UI's ability to retrieve MBean Metrics for the Dashboard and Topology, as seen in the screenshot below.

How to manually test:

  1. Run CRYOSTAT_IMAGE=quay.io... sh smoketest.sh...
  2. ...

image

@andrewazores andrewazores added the chore Refactor, rename, cleanup, etc. label Feb 29, 2024
@andrewazores andrewazores force-pushed the cryostat3-graphql-mbeanmetrics branch 2 times, most recently from 6c02250 to d748cee Compare February 29, 2024 18:26
Copy link

@andrewazores
Copy link
Member Author

/build_test

@andrewazores andrewazores requested review from aali309 and mwangggg March 1, 2024 15:38
Copy link

github-actions bot commented Mar 1, 2024

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1219-a37abbbbc1d1c339103ae1fdfc8426e115808ca1 bash smoketest.bash # then open http://localhost:8080

@andrewazores
Copy link
Member Author

Not sure why the diff here is including #1216 as part of the commit set. That's already merged into main which this is based on top of, but I guess something about the unmerged upstream cryostat3-graphql branch is confusing GitHub's UI.

@andrewazores andrewazores force-pushed the cryostat3-graphql-mbeanmetrics branch 2 times, most recently from d048ee3 to 0722347 Compare March 1, 2024 15:47
Copy link
Contributor

@aali309 aali309 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@andrewazores andrewazores merged commit a2a8815 into cryostatio:cryostat3-graphql Mar 1, 2024
8 checks passed
@andrewazores andrewazores deleted the cryostat3-graphql-mbeanmetrics branch March 1, 2024 21:24
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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants