Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reporting/Dashboard] Investigate causes of POST URL becoming too large to automate report with Watcher #111777

Closed
tsullivan opened this issue Sep 9, 2021 · 9 comments
Labels
bug Fixes for quality problems that affect the customer experience (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:x-large Extra Large Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@tsullivan
Copy link
Member

To automate reports of a dashboard using Watcher, a user copies a POST URL for a saved dashboard, which is generated from the sharing data provided by the dashboard application.

Since Watcher does not support specifying a POST body for the Kibana report URL, all of the info needed to automate a report job in Watcher must be in the query string of the POST URL. Therefore all of the sharing data provided by the dashboard application ends up being in the query string of the Watcher POST URL.

The Dashboard app is sending the full state of the view in the sharing data, and when used in the query string of the Watcher POST URL, it creates URLs that are too long for many parts of the stack to tolerate. This problem seemed to increase after the introduction of "by value" visualizations.

For a POST URL to automate report of a dashboard, we want the embeddables to use reference as much as possible. For a dashboard that is saved in the library and has no unsaved local changes, the POST URL target to the dashboard should just use a reference to the saved object type and ID of the dashboard.

Kibana version: 7.13

Elasticsearch version:

Server OS version:

Browser version:

Browser OS version:

Original install method (e.g. download page, yum, from source, etc.):

Describe the bug:
A request URL that is too long can cause Watcher to fail to issue the request, or a proxy that handles the request to fail to pass the request on to the destination. These types of errors are coded HTTP 400: Bad Request.

Steps to reproduce:

  1. Create a new dashboard
  2. Add 3 or more Lens based visualizations plus a couple of standard visualizations
  3. Save the dashboard
  4. Copy the POST URL to automate a report of the dashboard
  5. The POST URL is too long

Expected behavior:
The POST URL should target the dashboard using a reference to the dashboard saved object ID

Screenshots (if relevant):

Errors in browser console (if relevant):

Provide logs and/or server output (if relevant):

Any additional context:
Terminology

  • "By value": using the target embeddables attributes directly
  • "By reference": using the target embeddable's saved object type and ID
@tsullivan tsullivan added the bug Fixes for quality problems that affect the customer experience label Sep 9, 2021
@botelastic botelastic bot added the needs-team Issues missing a team label label Sep 9, 2021
@tsullivan tsullivan added Feature:Dashboard Dashboard related features (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Team:AppServices Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Sep 9, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Sep 9, 2021
@ThomThomson
Copy link
Contributor

@jloleysens, I've been thinking about this, and have noticed that the reports using locators also use the entire dashboard state, as well as the saved object id even when there are no unsaved changes. Dashboard constantly tracks 'unsaved changes' by diffing its state against the last saved object, so I wonder if it would be good for us to work together to make the reporting URLs only put the unsaved state into the URL.

It's worth noting, though, that this will not solve the problem in every case, because if the panels are modified, or if the dashboard is unsaved, the full panel state including any by value visualizations will be included in the URL. In this case, if we only place the unsaved changes in the URL, we can detect query strings above a certain length and warn the user that the URL looks a little long, and they should save before generating the URL.

Does this seem like a good compromise for allowing reports to be generated against unsaved state, while also not necessarily producing extremely long URLs every time?

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:weeks and removed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Sep 10, 2021
@tsullivan
Copy link
Member Author

tsullivan commented Sep 10, 2021

@ThomThomson thank you for weighing in!

reports using locators also use the entire dashboard state, as well as the saved object id even when there are no unsaved changes

This is my understanding as well. It means that our V2 of PDF / PNG exports still have this issue with the POST URL being too large.

good for us to work together to make the reporting URLs only put the unsaved state into the URL.

I agree we should work together on this. I like your proposal as it would allow the user to create any kind of automated dashboard report they want: based on saved object reference, based partially on a saved object reference but includes the unsaved state by-value, or is entirely by-value without any references to a saved object.

There is a use case I'm concerned about that could lead to ambiguity:

  1. User creates a dashboard with default options and saves it
  2. User changes a config of the dashboard, such as turning off panel titles. They create an automated report based on these partially unsaved changes
  3. The user decides they actually do want panel titles in the report. But when they edit the saved dashboard, it already does show panel titles.

I'm also a little concerned about unknown bugs we might encounter if the dashboard saved object ends up with a state that conflicts with the unsaved state that is tracked in the report job. The solution in either case is that the user needs to regenerate a new POST URL of the dashboard report and start using that in the automation. IMO this could be an annoyance until we have scheduled reporting inside Kibana and can add more user-friendly options, such as the ability to edit a schedule in the UI.

@ThomThomson
Copy link
Contributor

Yes, both of these concerns are legitimate, but I wouldn't consider them bugs, just opportunities for user education. I can't think of any way to solve them from an engineering perspective, at least until the scheduled reporting UI is built, where we can have a state explorer or something that allows users to see which pieces of state are coded into their report URLs.

You are right, in a conflict scenario, the unsaved state always overwrites the state from the saved object, so when conflicts happen, they won't be obvious and could be annoying.

Maybe we extend the warning to show when generating the URL, even when panels are not present. It could say something like This URL will contain unsaved changes. Subsequent updates to the saved dashboard may not be visible in the report.

@jloleysens
Copy link
Contributor

jloleysens commented Sep 13, 2021

Related issue for "detecting" max URL length: #111303

unknown bugs we might encounter if the dashboard saved object ends up with a state that conflicts with the unsaved state that is tracked in the report job.

@tsullivan This is also a good point, although I expect locator state migration should have our backs covered here (the reporting redirect app calls migrate on state stored in the job before navigating).

Maybe we extend the warning to show when generating the URL, even when panels are not present. It could say something like This URL will contain unsaved changes. Subsequent updates to the saved dashboard may not be visible in the report.

@ThomThomson @tsullivan that's what I was thinking too. From a users perspective: the appropriate point to inform the user of unsaved state is on the dashboard, when they are copying a POST URL and in Reporting when viewing a report (on the list view).

If we wanted to give apps the ability to surface warnings about the reports in the reporting UI (like, "this dashboard is not saved" we could add warnings using data-render-warnings).

...reports using locators also use the entire dashboard state, as well as the saved object id even when there are no unsaved changes

For reporting, we are planning on updating UI to link to the place a report was generated with the state of the generated report (#111412) , this gives users a way to "travel back" to their unsaved state, edit again, and copy the new URL.

I was trying think about how we can formalise this flow a bit better to tell users: "Hey, you can teleport back to the time and place you generated the report". This gives users a way of "updating" the URL using the same UI they used to create it.

A potential issue here is this is an informal "fork" of the dashboard which creates the possibility that the dashboard saved object changes and because we can apply only partial state, our "snapshot" also changes. Maybe this is desirable, maybe not. Maybe this is already an "issue" but since we have full dashboard state I am guessing not? I'd be interested to get your thoughts on this point.

[UPDATE]
Specifically, whether we want to enable this flow and what other potential issues are. Perhaps having the "fork" is OK since a user can always create a dashboard saved object from it. Perhaps we want to support both "snapshot" and saved report locators. This would need some thinking through i.t.o. UX.

@tsullivan
Copy link
Member Author

tsullivan commented Sep 14, 2021

I think we have a solid plan here. The Dashboard sharing data will include the reference to the saved object, as well as unsaved state in the browser. That will reduce the length of the POST URL in many cases, and give us the opportunity to educate the user when it doesn't reduce the length enough.

It's not a perfect solution because it creates complexity about what affects the output of the report: the unsaved changes stored in the report job are permanent, but the referenced state is editable. We can detect when there is a hybrid state in the browser and add messaging in a few contexts to educate the user. Adding more documentation to our Reporting Guide will also be a helpful step to explain all of this complexity in more detail.

@tsullivan
Copy link
Member Author

tsullivan commented Sep 27, 2021

Closing for #112130 and #112979

@exalate-issue-sync exalate-issue-sync bot added the loe:x-large Extra Large Level of Effort label Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:x-large Extra Large Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

No branches or pull requests

4 participants