feat(dashboard): implement embedded chart cards #856
Conversation
|
Test image available: |
|
Test image available: |
|
Test image available: |
andrewazores
force-pushed
the
grafana-card
branch
from
8a4531a to
6da8c1c
Compare
|
Test image available: |
|
Test image available: |
|
Test image available: |
andrewazores
force-pushed
the
grafana-card
branch
from
37ddb69 to
38e038b
Compare
|
Test image available: |
andrewazores
force-pushed
the
grafana-card
branch
from
279d1b5 to
56d211a
Compare
|
Test image available: |
|
Test image available: |
andrewazores
force-pushed
the
grafana-card
branch
from
08a15b4 to
0110a65
Compare
|
Test image available: |
|
Test image available: |
|
Test image available: |
|
Test image available: |
|
Test image available: |
|
Test image available: |
|
Test image available: |
andrewazores
force-pushed
the
grafana-card
branch
from
c870469 to
8bd1348
Compare
|
Test image available: |
andrewazores
force-pushed
the
grafana-card
branch
from
763078c to
ce4dc78
Compare
|
Test image available: |
There was a problem hiding this comment.
I am wondering how we are going to handle multiple accesses from different web console instances (2 tabs or 2 people looking at cryostat). For example, one person is looking at 9093 and another person enters dashboard and choose another target 9091. This will override datasource data.
This will make the first person dashboard outdated and rendering empty data at some point?
Correct. This is just a limitation of the stateful jfr-datasource model we have right now. In the feature documentation we will need to call this out and explain it to the user. Maybe for this reason it also makes sense to downgrade the feature level to BETA. In the future when we have a smarter jfr-datasource that can handle multiple source recordings and integration with Grafana panels (or custom charts) that know how to perform queries across different sources, then this problem should basically just disappear as a consequence. |
|
Oh cool make sense! Do you plan to fix the drag and resize issue here too? |
Yes, I'll do that next after I finish up with trying to genericize and refactor out the |
|
Test image available: |
|
Test image available: |
| "CHART_CARD": { | ||
| "BUTTONS": { | ||
| "CREATE": { | ||
| "LABEL": "Create" |
There was a problem hiding this comment.
I notice these aria-labels have a different scheme from some other aria labels in the file
e.g.
"DateTimePicker": {
"ARIA_LABELS": {
"DISPLAY_SELECTED_DATETIME": "Displayed selected datetime",
"TABS": "Select a date or time tab"
},
"SELECTED_DATETIME": "Selected DateTime"
},
"MeridiemPicker": {
"ARIA_LABELS": {
"LISTBOX": "Select AM or PM"
}
},Maybe either change this or we should change the other ones to keep consistency?
There was a problem hiding this comment.
We probably need to document key format somewhere too
There was a problem hiding this comment.
You mean just change the way that the key is split up? Or change the actual localized text?
There was a problem hiding this comment.
I mean the way the key is split up. Maybe we should have
"DateTimePicker": {
"TABS": {
"LABEL": "Select a date or time tab"
}in the same format in this PR that you have done here. If so, then that's okay to change in a separate refactor PR.
Just keeping a consistent key format.
There was a problem hiding this comment.
Sure, I don't really mind either way. I think there probably are/will be instances where the aria-label matches some text that is visually rendered, so specifying ARIA_LABELS as part of the key maybe doesn't make the most sense.
Not for this PR, I am thinking of something like a banner to show on web console when in BETA mode like this:
If the description is too long, we can just link to upstream docs or dev posts. |
It is indeed slightly annoying, but only because the re-ordering functionality is tied in with the order that the card elements are being attached to the DOM. It doesn't have to actually be this way since Patternfly grid uses the grid-order for its ordering which is just a css attribute, however I implemented the drag drop keeping the JS list order in mind which it is just a bit more intuitive. There will just need to a bit of refactoring with how the drag and drop works, but its probably not horrible. If you guys think this is important, I will look into a patch. |
Sounds good. That's more along the lines of how the old OSIO project did feature flagging IIRC, which is what inspired the feature flags here too. |
File an issue for it and we'll keep it as a lower priority item to tackle if there is time for this release. |
|
Will merge as soon as unblocked on cryostatio/cryostat-grafana-dashboard#20 . |
|
This PR/issue depends on:
|
Welcome to Cryostat! 👋
Before contributing, make sure you have:
mainbranch[chore, ci, docs, feat, fix, test]git commit --amend --signoffFixes: #847
Description of the change:
This adds a dashboard card implementation that can embed various Grafana metrics charts.
Motivation for the change:
Users do not need to use the
View in Grafana...action on a specific recording to view metrics. Instead, they can simply configure a dashboard with the metrics that interest them, and select a target application. The metrics layout is as the user wishes, the source recording is started as needed, and the data is reloaded on a schedule to provide near-realtime metrics display.