search

elastic / kibana

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

Apply pinned filters in dashboard reports #166069

Open vadimkibana opened 1 year ago

vadimkibana commented 1 year ago

Currently, pinned filters are not applied when dashboard PDF reports are generated, we would like to apply them in reports.

See more details in SDH: https://github.com/elastic/sdh-kibana/issues/4061

elasticmachine commented 1 year ago

Pinging @elastic/kibana-presentation (Team:Presentation)

elasticmachine commented 1 year ago

Pinging @elastic/appex-sharedux (Team:SharedUX)

tsullivan commented 12 months ago

@ThomThomson and I have found that global filters are not completely applied in PNG/PDF report execution.

Root cause

The time range and query are applied, and are part of global filters. We have found there is some slightly hacky code in src/plugins/dashboard/public/dashboard_app/top_nav/share/show_share_modal.tsx which adds these filters into the dashboard locator params, when the params are generated for sharing data.

  1. The query is read from global state in one place, and applied to the locator params in another:

  2. The time range is read from global state and applied to the locator params here:

Those parts of the code intermingle global state with application state, which is not the intention of locator params, thus we are labeling this code as hacky.

Other pinned filters don't have the hacky application and are therefore not getting entered into the locator params that are stored for the report job.

Proposals

  1. We could apply global filters into the locator params using the hacky application that intermingles global state with application state. This would not adversely affect reporting. However, there could be other consumers of the dashboard locatorParams that are adversely affected if we do this (currently, saved short URLs would not be affected, but would be once legacy code is cleaned up).
  2. We could update dashboard_app/top_nav/share/show_share_modal.tsx to supply globalParams separately from locatorParams. This would be ideal since it stops us from intermingling global state from application state, and removes the hackiness from this part of the code. This solution will be more time consuming since the Reporting and Screenshotting plugins need to be updated to become aware of a new field for globalParameters. We can try to minimize the complication by unifying the global and app params within the Reporting code that registers the share menu option - that would hopefully eliminate the need to update serverside Reporting code.
vadimkibana commented 12 months ago

I'm not sure I like the two options, I'm thinking maybe there is a third option. Also, the Reporting should not be aware of this feature at all, it should only take a screenshot of URL it is given.

That said, the option I have in mind is as follows:

  1. Dashboard would have a URL dedicated for reporting and that URL would receive the global filters in URL param, say &globalFilters=.... It could be the existing URL, or it could be a new clean URL dedicated for taking report screenshots, for example: /app/dashboard/123/report?...&globalFilters=...
  2. Then it can pass those filters to the right data plugin service, which will apply them (maybe set them as _g, up to the data plugin, the places where they store them is their internal implementation detail).
  3. Then the dashboard would render. Or redirect to the regular dashboard URL: /app/dashboard/123
tsullivan commented 12 months ago

@vadimkibana please correct me if I am mistaken. I believe if we use a URL with full params for a report job configuration, that configuration would not be migratable, and could break if the configuration is re-used for automation across version upgrades.

I believe we have these negative effects currently when creating a short URL from a dashboard. Internally, this stores a locator object that uses the LEGACY_SHORT_URL_LOCATOR, which is basically just a full URL.

Example: "locatorJSON": "{\"id\":\"LEGACY_SHORT_URL_LOCATOR\",\"version\":\"8.10.0\",\"state\":{\"url\":\"/app/dashboards#/view/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_g=(refreshInterval:(pause:!f,value:900000),time:(from:now-7d%2Fd,to:now))&_a=()\"}}",

Today, PNG/PDF report job configurations use the DASHBOARD_APP_LOCATOR.

Example: locatorParams:(id:DASHBOARD_APP_LOCATOR,params:(dashboardId:edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b,preserveSavedFilters:!t,timeRange:(from:now-7d/d,to:now),useHash:!f,viewMode:view)),

One advantage this gives us is the dashboard doesn't need to be saved before creating a report, which was a huge demand from our users. Hopefully whatever solution we move towards will not break that functionality.

ThomThomson commented 10 months ago

Commenting here that we haven't been able to prioritize this issue yet, but it is still worth fixing - and it's still on our radar.

tsullivan commented 7 months ago

removing my assignment