search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.63k stars 8.22k forks source link

[Drilldowns] Dashboard broken back button if embeddable update input during initial rendering #61596

Closed Dosant closed 4 years ago

Dosant commented 4 years ago

While working on dashboard-to-dashboard drilldown a handful of issues were fixed to improve back button experience. https://github.com/elastic/kibana/issues/61230. But this particular one stands out.

Reproduce:

  1. Load logs sample data
  2. Open [Logs] Web Traffic dashboard
  3. Press back button

Expected behaviour: navigate back to listing page Current behaviour (after all fixes from https://github.com/elastic/kibana/issues/61230 in master): nothing

If to look into history stack: 

#/dashboard/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_a=(description:'Analyze%20mock%20web%20traffic%20log%20data%20for%20Elastic!'s%20website',filters:!(),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),panels:!((embeddableConfig:(vis:(colors:('Avg.%20Bytes':%236ED0E0,'Unique%20Visitors':%230A437C),legendOpen:!f)),gridData:(h:13,i:'2',w:21,x:27,y:11),id:e1d0f010-9ee7-11e7-8711-e7a007dcef99,panelIndex:'2',type:visualization,version:'8.0.0'),(embeddableConfig:(isLayerTOCOpen:!f),gridData:(h:18,i:'4',w:24,x:0,y:49),id:de71f4f0-1902-11e9-919b-ffe5949a18d2,panelIndex:'4',type:map,version:'8.0.0'),(embeddableConfig:(vis:(defaultColors:('0%20-%2022':'rgb(247,251,255)','22%20-%2044':'rgb(208,225,242)','44%20-%2066':'rgb(148,196,223)','66%20-%2088':'rgb(74,152,201)','88%20-%20110':'rgb(23,100,171)'),legendOpen:!f)),gridData:(h:13,i:'7',w:24,x:0,y:36),id:'935afa20-e0cd-11e7-9d07-1398ccfcefa3',panelIndex:'7',type:visualization,version:'8.0.0'),(embeddableConfig:(mapCenter:!(36.8092847020594,-96.94335937500001),vis:(params:(sort:(columnIndex:!n,direction:!n)))),gridData:(h:12,i:'9',w:21,x:27,y:24),id:'4eb6e500-e1c7-11e7-b6d5-4dc382ef7f5b',panelIndex:'9',type:visualization,version:'8.0.0'),(embeddableConfig:(title:'',vis:(colors:('0%20-%20500':%23BF1B00,'1000%20-%201500':%237EB26D,'500%20-%201000':%23F2C96D),defaultColors:('0%20-%20500':'rgb(165,0,38)','1000%20-%201500':'rgb(0,104,55)','500%20-%201000':'rgb(255,255,190)'),legendOpen:!f)),gridData:(h:11,i:'11',w:9,x:10,y:0),id:'69a34b00-9ee8-11e7-8711-e7a007dcef99',panelIndex:'11',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:12,i:'13',w:27,x:0,y:24),id:'42b997f0-0c26-11e8-b0ec-3bb475f6b6ff',panelIndex:'13',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:31,i:'14',w:24,x:24,y:36),id:'7cbd2350-2223-11e8-b802-5bcf64c2cfb4',panelIndex:'14',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:13,i:'15',w:27,x:0,y:11),id:'314c6f60-2224-11e8-b802-5bcf64c2cfb4',panelIndex:'15',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'16',w:15,x:19,y:0),id:'24a3e970-4257-11e8-b3aa-73fdaf54bfc9',panelIndex:'16',type:visualization,version:'8.0.0'),(embeddableConfig:(vis:(legendOpen:!f)),gridData:(h:11,i:'17',w:14,x:34,y:0),id:'14e2e710-4258-11e8-b3aa-73fdaf54bfc9',panelIndex:'17',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'18',w:10,x:0,y:0),id:'47f2c680-a6e3-11e8-94b4-c30c0228351b',panelIndex:'18',type:visualization,version:'8.0.0')),query:(language:kuery,query:''),timeRestore:!t,title:'%5BLogs%5D%20Web%20Traffic',viewMode:view)&_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-7d,to:now))
#/dashboard/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_a=(description:'Analyze%20mock%20web%20traffic%20log%20data%20for%20Elastic!'s%20website',filters:!(),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),panels:!((embeddableConfig:(vis:(colors:('Avg.%20Bytes':%236ED0E0,'Unique%20Visitors':%230A437C),legendOpen:!f)),gridData:(h:13,i:'2',w:21,x:27,y:11),id:e1d0f010-9ee7-11e7-8711-e7a007dcef99,panelIndex:'2',type:visualization,version:'8.0.0'),(embeddableConfig:(isLayerTOCOpen:!f,mapCenter:(lat:42.16337,lon:-88.92107,zoom:3.64)),gridData:(h:18,i:'4',w:24,x:0,y:49),id:de71f4f0-1902-11e9-919b-ffe5949a18d2,panelIndex:'4',type:map,version:'8.0.0'),(embeddableConfig:(vis:(defaultColors:('0%20-%2022':'rgb(247,251,255)','22%20-%2044':'rgb(208,225,242)','44%20-%2066':'rgb(148,196,223)','66%20-%2088':'rgb(74,152,201)','88%20-%20110':'rgb(23,100,171)'),legendOpen:!f)),gridData:(h:13,i:'7',w:24,x:0,y:36),id:'935afa20-e0cd-11e7-9d07-1398ccfcefa3',panelIndex:'7',type:visualization,version:'8.0.0'),(embeddableConfig:(mapCenter:!(36.8092847020594,-96.94335937500001),vis:(params:(sort:(columnIndex:!n,direction:!n)))),gridData:(h:12,i:'9',w:21,x:27,y:24),id:'4eb6e500-e1c7-11e7-b6d5-4dc382ef7f5b',panelIndex:'9',type:visualization,version:'8.0.0'),(embeddableConfig:(title:'',vis:(colors:('0%20-%20500':%23BF1B00,'1000%20-%201500':%237EB26D,'500%20-%201000':%23F2C96D),defaultColors:('0%20-%20500':'rgb(165,0,38)','1000%20-%201500':'rgb(0,104,55)','500%20-%201000':'rgb(255,255,190)'),legendOpen:!f)),gridData:(h:11,i:'11',w:9,x:10,y:0),id:'69a34b00-9ee8-11e7-8711-e7a007dcef99',panelIndex:'11',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:12,i:'13',w:27,x:0,y:24),id:'42b997f0-0c26-11e8-b0ec-3bb475f6b6ff',panelIndex:'13',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:31,i:'14',w:24,x:24,y:36),id:'7cbd2350-2223-11e8-b802-5bcf64c2cfb4',panelIndex:'14',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:13,i:'15',w:27,x:0,y:11),id:'314c6f60-2224-11e8-b802-5bcf64c2cfb4',panelIndex:'15',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'16',w:15,x:19,y:0),id:'24a3e970-4257-11e8-b3aa-73fdaf54bfc9',panelIndex:'16',type:visualization,version:'8.0.0'),(embeddableConfig:(vis:(legendOpen:!f)),gridData:(h:11,i:'17',w:14,x:34,y:0),id:'14e2e710-4258-11e8-b3aa-73fdaf54bfc9',panelIndex:'17',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'18',w:10,x:0,y:0),id:'47f2c680-a6e3-11e8-94b4-c30c0228351b',panelIndex:'18',type:visualization,version:'8.0.0')),query:(language:kuery,query:''),timeRestore:!t,title:'%5BLogs%5D%20Web%20Traffic',viewMode:view)&_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-7d,to:now))
#/dashboard/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_a=(description:'Analyze%20mock%20web%20traffic%20log%20data%20for%20Elastic!'s%20website',filters:!(),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),panels:!((embeddableConfig:(vis:(colors:('Avg.%20Bytes':%236ED0E0,'Unique%20Visitors':%230A437C),legendOpen:!f)),gridData:(h:13,i:'2',w:21,x:27,y:11),id:e1d0f010-9ee7-11e7-8711-e7a007dcef99,panelIndex:'2',type:visualization,version:'8.0.0'),(embeddableConfig:(hiddenLayers:!(),isLayerTOCOpen:!f,mapCenter:(lat:42.16337,lon:-88.92107,zoom:3.64),openTOCDetails:!()),gridData:(h:18,i:'4',w:24,x:0,y:49),id:de71f4f0-1902-11e9-919b-ffe5949a18d2,panelIndex:'4',type:map,version:'8.0.0'),(embeddableConfig:(vis:(defaultColors:('0%20-%2022':'rgb(247,251,255)','22%20-%2044':'rgb(208,225,242)','44%20-%2066':'rgb(148,196,223)','66%20-%2088':'rgb(74,152,201)','88%20-%20110':'rgb(23,100,171)'),legendOpen:!f)),gridData:(h:13,i:'7',w:24,x:0,y:36),id:'935afa20-e0cd-11e7-9d07-1398ccfcefa3',panelIndex:'7',type:visualization,version:'8.0.0'),(embeddableConfig:(mapCenter:!(36.8092847020594,-96.94335937500001),vis:(params:(sort:(columnIndex:!n,direction:!n)))),gridData:(h:12,i:'9',w:21,x:27,y:24),id:'4eb6e500-e1c7-11e7-b6d5-4dc382ef7f5b',panelIndex:'9',type:visualization,version:'8.0.0'),(embeddableConfig:(title:'',vis:(colors:('0%20-%20500':%23BF1B00,'1000%20-%201500':%237EB26D,'500%20-%201000':%23F2C96D),defaultColors:('0%20-%20500':'rgb(165,0,38)','1000%20-%201500':'rgb(0,104,55)','500%20-%201000':'rgb(255,255,190)'),legendOpen:!f)),gridData:(h:11,i:'11',w:9,x:10,y:0),id:'69a34b00-9ee8-11e7-8711-e7a007dcef99',panelIndex:'11',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:12,i:'13',w:27,x:0,y:24),id:'42b997f0-0c26-11e8-b0ec-3bb475f6b6ff',panelIndex:'13',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:31,i:'14',w:24,x:24,y:36),id:'7cbd2350-2223-11e8-b802-5bcf64c2cfb4',panelIndex:'14',type:visualization,version:'8.0.0'),(embeddableConfig:(),gridData:(h:13,i:'15',w:27,x:0,y:11),id:'314c6f60-2224-11e8-b802-5bcf64c2cfb4',panelIndex:'15',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'16',w:15,x:19,y:0),id:'24a3e970-4257-11e8-b3aa-73fdaf54bfc9',panelIndex:'16',type:visualization,version:'8.0.0'),(embeddableConfig:(vis:(legendOpen:!f)),gridData:(h:11,i:'17',w:14,x:34,y:0),id:'14e2e710-4258-11e8-b3aa-73fdaf54bfc9',panelIndex:'17',type:visualization,version:'8.0.0'),(embeddableConfig:(title:''),gridData:(h:11,i:'18',w:10,x:0,y:0),id:'47f2c680-a6e3-11e8-94b4-c30c0228351b',panelIndex:'18',type:visualization,version:'8.0.0')),query:(language:kuery,query:''),timeRestore:!t,title:'%5BLogs%5D%20Web%20Traffic',viewMode:view)&_g=(filters:!(),refreshInterval:(pause:!f,value:900000),time:(from:now-7d,to:now))

Redundant changes are triggered by maps embeddable:

(embeddableConfig:(isLayerTOCOpen:!f)
(embeddableConfig:(isLayerTOCOpen:!f,mapCenter:(lat:42.16337,lon:-88.92107,zoom:3.64))
(embeddableConfig:(hiddenLayers:!(),isLayerTOCOpen:!f,mapCenter:(lat:42.16337,lon:-88.92107,zoom:3.64)

I assume what is happening, is when map embeddable from older kibana version is opened, maps are doing state migrations on a fly and update input during initial rendering.

https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/maps/public/embeddable/map_embeddable.tsx#L190

I am not sure, if there are other cases like this except for maps. Didn't find in dashboards from sample data

Please Note:

  1. If to Save Dashboard now and return to it - history button works correctly, because we saved last state which map embeddable expects!

  2. This maps behaviour causing other issue: as it updates input multiple time, it will retrigger redundant rerenders for other panels

This is not only migration edge case

  1. Create a new map
  2. Go to dashboard, create new dashboard
  3. Add this map do dashboard
  4. Press back button

The same problem is happening, app adds new fields to embeddable state during it's initial rendering.

Possible solutions:

  1. Fix in maps. ? Is this possible? ~2. Come up with some magic "all settled" on parent embeddable? start syncing state in dashboard when it emitted?~ - won't work because issue is actually happening not only during initial rendering ~3. Magic setTimeout in dashboard before push state into url?~ - won't work because issue is actually happening not only during initial rendering
  2. Consider not storing all the state in the URL?

cc @stacey-gammon @ppisljar @streamich

p.s.: maps are ignoring most of state changes which are coming from embeddable input, so history button almost not working for user actions done in maps. but this seems like separate bug

elasticmachine commented 4 years ago

Pinging @elastic/kibana-app-arch (Team:AppArch)

ppisljar commented 4 years ago

another possible solution would be moving the logic into migration script or (preferably not as we need to move that one to migration script as well) move it into visualizations/legacy/vis_update_state

Dosant commented 4 years ago

@ppisljar, just realised that this is not only initial migration issue. updated the description:

Create a new map Go to dashboard, create new dashboard Add this map do dashboard Press back button

And, basically, the same thing is happening when opening older dashboard for the first time

Dosant commented 4 years ago

Agreed to try to solve this by removing panels state from the url in view mode