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(dashboard): add JVM Details card #896

Merged
merged 18 commits into from
Mar 15, 2023

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Feb 23, 2023

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 the last commit: git commit --amend --signoff

Fixes: #894
Fixes: #895
Depends on #891

Description of the change:

This adds a simple dashboard card that simply displays basic information about the selected target JVM such as its connection URL, alias, and labels/annotations.

Motivation for the change:

This provides users with a nicer presentation of the data currently displayed on the <TargetSelect/> component expansion, which the user might need to know in order to define matchExpression credentials, automated rules, or to filter/sort through the All Archives table. A follow-up PR will add further MBean-based information to this card as well.

How to manually test:

  1. Run CRYOSTAT_IMAGE=quay.io... sh smoketest.sh...
  2. Select a target JVM
  3. Add the JVM Details dashboard card
  4. Check that the details are updated with different target selections and that the details match the target

image

@andrewazores andrewazores added the feat New feature or request label Feb 23, 2023
@mergify mergify bot added the safe-to-test label Feb 23, 2023
@andrewazores andrewazores force-pushed the jvm-details-dashboard-card branch from 9f15d13 to e1df0ff Compare February 23, 2023 20:17
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-e1df0ff252fe64859541d7ae1ab536c38e424cdc sh smoketest.sh

Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

Looks great to me!

Just wondering because I have a similar in #891 that looks like this with a bit styling, labels/annotations grouping, handling for empty fields and warnings. Do u want to apply these here or maybe after #891 (probably refactoring out to some common component)?

Screenshot from 2023-02-23 15-53-05

Screenshot from 2023-02-23 15-53-51

@andrewazores
Copy link
Member Author

I'll wait for yours and pull it out to a shared component.

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-2a0ffe2ea6d2b766438ae7a291b23cf13d6c62b2 sh smoketest.sh

@andrewazores andrewazores force-pushed the jvm-details-dashboard-card branch from 2a0ffe2 to 54244ec Compare February 23, 2023 23:30
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-54244ecd0974f22ed74da390dac204a20cab3e13 sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-557da7e904f36eb0206bd528cbb46fe7616acd07 sh smoketest.sh

@github-actions
Copy link

This PR/issue depends on:

@andrewazores andrewazores force-pushed the jvm-details-dashboard-card branch from 557da7e to 206126e Compare March 13, 2023 23:29
@andrewazores
Copy link
Member Author

image

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-206126ee9edbfcfbac50f4119781f1816cef2ef4 sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-84284830f2d4e1539ced04cf258a63260282f231 sh smoketest.sh

@andrewazores andrewazores force-pushed the jvm-details-dashboard-card branch 2 times, most recently from 108c8fc to 5c14705 Compare March 14, 2023 00:05
@tthvo
Copy link
Member

tthvo commented Mar 14, 2023

No features for #895 right?

@andrewazores
Copy link
Member Author

I could (re)add that into the new shared component.

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-5c14705e2ea24831484732a4332bd97165a4da00 sh smoketest.sh

@tthvo
Copy link
Member

tthvo commented Mar 14, 2023

I could (re)add that into the new shared component.

Sure! I think I have a const here to define them. I guess we can extract into a common function to return this array.

const _transformedData = React.useMemo(() => {

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-a0b533955ba89720584c3ceb5318bfda5e8f827d sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-3d18ba03b81785ce63493fd51036c53e4ac1a7af sh smoketest.sh

tthvo
tthvo previously approved these changes Mar 15, 2023
Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@andrewazores
Copy link
Member Author

Bah, I need to rebase to sign commits.

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-896-70399a8f86812ead927f4249a77645d9d6ddee7e sh smoketest.sh

@andrewazores andrewazores merged commit 864a896 into cryostatio:main Mar 15, 2023
@andrewazores andrewazores deleted the jvm-details-dashboard-card branch March 15, 2023 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
No open projects
Status: Done
3 participants