[BUGFIX] Dashboard: remove unmountOnExit flag of the row collapse#80
[BUGFIX] Dashboard: remove unmountOnExit flag of the row collapse#80
Conversation
|
Yeah, as told in private. The unMount has been probably added for perf reasons (https://mui.com/material-ui/react-accordion/#performance). To remove some nodes from DOM, if panel group has a lot of panels in it . That need to be tested too. |
Yes it makes sense and I will provide a test. Thank you for this great suggestion. |
Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com>
18a9f66 to
a3e9ef7
Compare
|
@Gladorme To tests I used the following Go DaCpackage main
import (
"flag"
"fmt"
"github.com/perses/perses/go-sdk"
"github.com/perses/perses/go-sdk/dashboard"
"github.com/perses/perses/go-sdk/panel"
panelgroup "github.com/perses/perses/go-sdk/panel-group"
promDs "github.com/perses/plugins/prometheus/sdk/go/datasource"
"github.com/perses/plugins/prometheus/sdk/go/query"
timeSeriesPanel "github.com/perses/plugins/timeserieschart/sdk/go"
)
func main() {
flag.Parse()
exec := sdk.NewExec()
groupOptions := []panelgroup.Option{
panelgroup.PanelsPerLine(2),
}
for i := 1; i <= 100; i++ {
panelTitle := fmt.Sprintf("simple up query #%d", i)
groupOptions = append(groupOptions, panelgroup.AddPanel(panelTitle,
timeSeriesPanel.Chart(),
panel.Title(panelTitle),
panel.AddQuery(
query.PromQL("up"),
),
))
}
builder, buildErr := dashboard.New("TESTDASHBAORD",
dashboard.ProjectName("TESTPROJECT"),
dashboard.AddPanelGroup("100 panels",
groupOptions...,
),
dashboard.AddDatasource("promLocal",
promDs.Prometheus(promDs.HTTPProxy("http://localhost:9090")),
),
)
exec.BuildDashboard(builder, buildErr)
} |
|
After, testing with 10 panel groups of 100 panels, I am thinking My conclusion, if dashboard has:
No real data to confirm except "fluidity" of tooltip and no idea what is really a "lot of panel" (here I tried with 100 panels, maybe something nobody will do). Not expert of UI profiling and I have issue to use React Devtools on Perses :( Tweaked a little bit your DaC: package main
import (
"flag"
"fmt"
"github.com/perses/perses/go-sdk"
"github.com/perses/perses/go-sdk/dashboard"
"github.com/perses/perses/go-sdk/datasource"
"github.com/perses/perses/go-sdk/panel"
panelgroup "github.com/perses/perses/go-sdk/panel-group"
promDs "github.com/perses/plugins/prometheus/sdk/go/datasource"
"github.com/perses/plugins/prometheus/sdk/go/query"
timeSeriesPanel "github.com/perses/plugins/timeserieschart/sdk/go"
)
func main() {
flag.Parse()
exec := sdk.NewExec()
groupOptions := []panelgroup.Option{
panelgroup.PanelsPerLine(2),
}
panels := 100
for i := 1; i <= panels; i++ {
panelTitle := fmt.Sprintf("simple up query #%d", i)
groupOptions = append(groupOptions, panelgroup.AddPanel(panelTitle,
timeSeriesPanel.Chart(),
panel.Title(panelTitle),
panel.AddQuery(
query.PromQL("up"),
),
))
}
builder, buildErr := dashboard.New(fmt.Sprintf("%d-panels", panels),
dashboard.ProjectName("q"),
dashboard.AddPanelGroup("Group 1",
groupOptions...,
),
dashboard.AddPanelGroup("Group 2",
groupOptions...,
),
dashboard.AddPanelGroup("Group 3",
groupOptions...,
),
dashboard.AddPanelGroup("Group 4",
groupOptions...,
),
dashboard.AddPanelGroup("Group 5",
groupOptions...,
),
dashboard.AddPanelGroup("Group 6",
groupOptions...,
),
dashboard.AddPanelGroup("Group 7",
groupOptions...,
),
dashboard.AddPanelGroup("Group 8",
groupOptions...,
),
dashboard.AddPanelGroup("Group 9",
groupOptions...,
),
dashboard.AddPanelGroup("Group 10",
groupOptions...,
),
dashboard.AddDatasource("promLocal",
promDs.Prometheus(promDs.HTTPProxy("https://prometheus.demo.prometheus.io")),
datasource.Default(true),
),
)
exec.BuildDashboard(builder, buildErr)
} |
relates to perses/perses#3448
Description
This small change makes the heavy dashboards open/close collapses 10X faster!
I drilled down and found out that when row -> collapse -> unmountOnExit is set to true and the user closes and opens the collapse again, (VERY LIKELY) two things happen. (Based on the browser profiling metrics)
Considering the volume of data downloaded for a huge dashboard like nodexporter, retrieving the data from the cache itself is a huge time consuming task! This task alone takes more than 2000 ms
Confusion 💀 Open Scary Question
Why have we set unmountOnExit to true?! Here, the only benefit of collapse is to hide unnecessary info. Why should it destroy the the only and immediate child?! What is the advantage?
Tested corner cases
Test instruction 🧪
You can find the details of the test instruction here
perses/perses#3448
Checklist
[<catalog_entry>] <commit message>naming convention using one of thefollowing
catalog_entryvalues:FEATURE,ENHANCEMENT,BUGFIX,BREAKINGCHANGE,DOC,IGNORE.UI Changes
See e2e docs for more details. Common issues include: