feat(topology): topology view #891

tthvo · 2023-02-21T05:50:37Z

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: #678, #752 (topology)
Fixes: #666, #429, #867 (targetSelect)
Fixes: #594 (test connection)
Depends on https://github.com/cryostatio/cryostat/pull/1389

Description of the change:

This PR includes the new Topology to view discovered targets along with their group/realm. Remaining tasks:

Note: intentionally left out localizations and tests to keep the PR less clustered. Will include those changes as a follow-up PR.

Motivation for the change:

See #678

How to manually test:

Go to Topology Tab under sidecar.
Go to any view to see the latest top-left target selector. Test out search and favorite features.
Go Dashboard Settings in /settings to see the latest local target selector. Similarly, go/create-rule and /security (credential modal).
To save time deploying on real cluster, you can substitute discovery tree with this json object: https://gist.github.com/tthvo/72c7a0a6cd1eb36cc8616d4adf0211f7 or its already saved under @test/resources.

tthvo · 2023-02-21T05:56:07Z

Just in draft right now as filtering and tree-walk function needs closer look. This works only in local dev (exploring how it is in OCP now - nested groups are gonna be pretty clustered so need further restyling). Let me know how it looks or any functionalities that you want to add in :D

andrewazores · 2023-02-21T13:35:31Z

Wow, this looks amazing. My mind is pretty blown :-)

When I click on the link from the Topology view to add credentials for a target:

it would be cool if not only were I directed to the Security view but also if the credentials adding dialog could be drawn up immediately, too. Or maybe the modal can just be drawn up on top of the Topology view without directing to Security at all.

Also, maybe this is just the design guidelines for the PF Topology view, but I find that the connection test on the new custom target creation form is not very obvious. It took me a minute to realize that logo element was clickable. Maybe something like https://rxjs.dev/api/operators/throttleTime / https://rxjs.dev/api/operators/debounceTime on the text input fed into the testing function automatically would be helpful?

And if the test fails, the creation button should probably be disabled (until the connection URL input is changed) since we can now predict that the action is going to fail anyway.

tthvo · 2023-02-21T16:18:35Z

Also, maybe this is just the design guidelines for the PF Topology view, but I find that the connection test on the new custom target creation form is not very obvious. It took me a minute to realize that logo element was clickable. Maybe something like https://rxjs.dev/api/operators/throttleTime / https://rxjs.dev/api/operators/debounceTime on the text input fed into the testing function automatically would be helpful?

Ahhh actually I designed this layout to to replace the Test text button...Turned out not to be too obvious then. I will test out with the automatic testing here but will keep the clickable sample target node and maybe mention this in the top alert banner of the form + add a quickstart (if possible) + a blog post on this?

andrewazores · 2023-02-21T20:59:10Z

Sounds good, though I don't think a blog post is necessary just for this aspect. A post covering the topology view as a whole would be great. Ideally the test button should look button-y and be discoverable/obvious to the user just looking at the view, without having to go look at some other documentation to figure out how to operate it.

tthvo · 2023-02-21T21:26:20Z

Okayyy maybe a hint text right under the button would also help + mentioning in the top banner then :D

tthvo · 2023-02-21T22:46:47Z

it would be cool if not only were I directed to the Security view but also if the credentials adding dialog could be drawn up immediately, too. Or maybe the modal can just be drawn up on top of the Topology view without directing to Security at all

On this one, could u check the latest commit to see if that is what we want?

Also, maybe this is just the design guidelines for the PF Topology view, but I find that the connection test on the new custom target creation form is not very obvious. It took me a minute to realize that logo element was clickable. Maybe something like https://rxjs.dev/api/operators/throttleTime / https://rxjs.dev/api/operators/debounceTime on the text input fed into the testing function automatically would be helpful?

Maybe I don't have a clear idea but would there be problems:

What if the latest input test is throttled (the user stops typing) and the validation state is now shown by the previous one?
What if one input test request is slow for some reason and comes after the newer test request. Then, its state will override the newer one? Probably can be solved in switchMap.
How is this automatic testing affects the backend load? Seems like we can DoS ourselves?

Maybe adding helper text to make the button (sample target node) functionality stands out makes the most sense?

andrewazores · 2023-02-22T17:25:41Z

What if the latest input test is throttled (the user stops typing) and the validation state is now shown by the previous one?

The idea is just to prevent too many tests from being performed on the input while it's changing, so once the input has settled for some time there would be a "final" observable emission that would go through the pipeline and end up as a connection test call.

What if one input test request is slow for some reason and comes after the newer test request. Then, its state will override the newer one? Probably can be solved in switchMap.

Possible yes, but as you say I think there's a way to solve this with the right kind of observable chaining, and by applying an appropriately slow/long debounce on the input changes.

How is this automatic testing affects the backend load? Seems like we can DoS ourselves?

Well... I hope not. This isn't a very expensive request and if it's debounced so that each individual web-client instance is only performing one request every second or so, DoSing should be unlikely.

andrewazores · 2023-02-22T17:34:22Z

On this one, could u check the latest commit to see if that is what we want?

This is probably even better than what I was saying, I say stick with what you added here. The helper text for the test button is also probably enough, so no need to worry too much about the automatic testing thing.

tthvo · 2023-02-22T22:19:04Z

@andrewazores Just wanna double check on this one. I added the resource tab for each target. Currently, it only shows the total of certain resource. Anything you think we should add in here? Each link should lead the users the corresponding view.

andrewazores · 2023-02-22T23:58:36Z

Maybe the number of matching rules and credentials?

The links from that table to the views appear to work nicely, but navigating directly to those views from the main left side nav menu seems to be broken now.

tthvo · 2023-02-23T00:08:00Z

Opps i forgot to check null XD

tthvo · 2023-02-23T03:43:35Z

Rebasing causes this CACHE_CHECKSUM_MISMATCH:

@babel/template@npm:7.18.10: The remote archive doesn't match the expected checksum

Not sure what happens to this package checksum (maybe backports? updates?). So I have to clean cache and use YARN_CHECKSUM_BEHAVIOUR=update to fix (seems to fix and no visible problems at the moment)...

andrewazores · 2023-03-10T20:51:34Z

This is an existing "vulnerability" from before this PR anyway, but I don't think it poses any real risk since again this is exactly equivalent to the built-in JS console in the browser.

github-actions · 2023-03-10T20:58:58Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-891-dc6ee80f3804bdb86900c9ee7c6a9bdfab12a156 sh smoketest.sh

maxcao13 · 2023-03-10T21:05:10Z

Gotcha, makes sense to me then. Thanks for the explanation!

maxcao13 · 2023-03-10T21:26:42Z

One last thing:

I think these Details fields should be selectable like text?

tthvo · 2023-03-10T21:35:02Z

I think these Details fields should be selectable like text?

Unfortunately, the TreeView component seems to wrap that view in a button which causes it not to be selectable...

Warning: validateDOMNesting(...): <button> cannot appear as a descendant of <button>.

This comes up whenrendering the action dropdown in the detail view.

src/app/Shared/MatchExpressionEvaluator.tsx

github-actions · 2023-03-10T22:23:11Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-891-7f187ac2e75dfef2791c8ae3c307d6fefcd0a599 sh smoketest.sh

github-actions · 2023-03-10T22:34:23Z

Test image available:

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

andrewazores · 2023-03-10T22:36:06Z

Typing in the filter box doesn't seem to do anything?

tthvo · 2023-03-10T23:02:39Z

A little weird that the compareTo function is given different arguments if select Option is not in a group. Fixed now thanks!

github-actions · 2023-03-10T23:07:56Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-891-aebdddcb11328c943ef9aecd67cee7e22ee70663 sh smoketest.sh

github-actions · 2023-03-10T23:17:16Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-891-c804149a57d9f060106e878445687129e7a2863f sh smoketest.sh

maxcao13

No other comments, looks great to me! Awesome job on this.

tthvo added the feat New feature or request label Feb 21, 2023

github-actions bot added the needs-triage Needs thorough attention from code reviewers label Feb 21, 2023

mergify bot added the safe-to-test label Feb 21, 2023

tthvo requested review from maxcao13 and andrewazores February 21, 2023 05:55

tthvo removed the safe-to-test label Feb 21, 2023

mergify bot added the safe-to-test label Feb 21, 2023

tthvo force-pushed the topology branch from e3098cb to 65f4cbd Compare February 21, 2023 22:01

tthvo force-pushed the topology branch from 2b7017d to 96ab6c0 Compare February 22, 2023 19:02

tthvo force-pushed the topology branch from f2ee529 to 9341d89 Compare February 23, 2023 03:25

andrewazores mentioned this pull request Feb 23, 2023

feat(dashboard): add JVM Details card #896

Merged

7 tasks

tthvo force-pushed the topology branch from 5b4586b to b080b9d Compare February 23, 2023 22:02

tthvo added needs-triage Needs thorough attention from code reviewers and removed needs-triage Needs thorough attention from code reviewers labels Feb 27, 2023

tthvo force-pushed the topology branch from af84352 to dc6ee80 Compare March 10, 2023 20:51

andrewazores reviewed Mar 10, 2023

View reviewed changes

src/app/Shared/MatchExpressionEvaluator.tsx Outdated Show resolved Hide resolved

fix(topology): selectable area in list view

7f187ac

chore(topology): refactor common match expre eval

896ad08

fix(topology): fix filter typeahead

aebdddc

fix(topology): selectable text area should have styled cursor

c804149

maxcao13 approved these changes Mar 11, 2023

View reviewed changes

andrewazores approved these changes Mar 11, 2023

View reviewed changes

andrewazores merged commit bfd9f86 into cryostatio:main Mar 11, 2023

tthvo deleted the topology branch March 11, 2023 00:15

tthvo mentioned this pull request Mar 13, 2023

fix(topology): custom target submission should also send credentials #907

Merged

7 tasks

This was referenced Mar 21, 2023

fix(topology): add missing callbacks to empty state actions #917

Merged

feat(topology): expanded row for resource details #921

Merged

This was referenced Mar 30, 2023

fix(topology): mini quicksearch should check active feature level #930

Merged

fix(topology): scrollable detail sidebar #935

Merged

tthvo mentioned this pull request Apr 16, 2023

fix(topology): topology view should auto-refresh if enabled #962

Merged

7 tasks

tthvo mentioned this pull request Apr 25, 2023

fix(topology): listview target details need fixed height #980

Merged

7 tasks

This was referenced May 2, 2023

fix(topology): catalog btn tooltip should not show if not needed #1009

Merged

fix(topology): correct toolbar mode icon #1019

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(topology): topology view #891

feat(topology): topology view #891

tthvo commented Feb 21, 2023 •

edited

Loading

tthvo commented Feb 21, 2023 •

edited

Loading

andrewazores commented Feb 21, 2023

tthvo commented Feb 21, 2023 •

edited

Loading

andrewazores commented Feb 21, 2023

tthvo commented Feb 21, 2023

tthvo commented Feb 21, 2023 •

edited

Loading

andrewazores commented Feb 22, 2023

andrewazores commented Feb 22, 2023

tthvo commented Feb 22, 2023

andrewazores commented Feb 22, 2023

tthvo commented Feb 23, 2023

tthvo commented Feb 23, 2023

andrewazores commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

maxcao13 commented Mar 10, 2023

maxcao13 commented Mar 10, 2023

tthvo commented Mar 10, 2023 •

edited

Loading

github-actions bot commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

andrewazores commented Mar 10, 2023

tthvo commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

maxcao13 left a comment

feat(topology): topology view #891

feat(topology): topology view #891

Conversation

tthvo commented Feb 21, 2023 • edited Loading

Welcome to Cryostat! 👋

Before contributing, make sure you have:

Description of the change:

Motivation for the change:

How to manually test:

tthvo commented Feb 21, 2023 • edited Loading

andrewazores commented Feb 21, 2023

tthvo commented Feb 21, 2023 • edited Loading

andrewazores commented Feb 21, 2023

tthvo commented Feb 21, 2023

tthvo commented Feb 21, 2023 • edited Loading

andrewazores commented Feb 22, 2023

andrewazores commented Feb 22, 2023

tthvo commented Feb 22, 2023

andrewazores commented Feb 22, 2023

tthvo commented Feb 23, 2023

tthvo commented Feb 23, 2023

andrewazores commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

maxcao13 commented Mar 10, 2023

maxcao13 commented Mar 10, 2023

tthvo commented Mar 10, 2023 • edited Loading

github-actions bot commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

andrewazores commented Mar 10, 2023

tthvo commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

maxcao13 left a comment

Choose a reason for hiding this comment

tthvo commented Feb 21, 2023 •

edited

Loading

tthvo commented Feb 21, 2023 •

edited

Loading

tthvo commented Feb 21, 2023 •

edited

Loading

tthvo commented Feb 21, 2023 •

edited

Loading

tthvo commented Mar 10, 2023 •

edited

Loading