[Reporting] Fix Generating Reports with long `jobParams` RISON #45603

tsullivan · 2019-09-13T02:04:44Z

Summary

Closes #20037

This solves the problem with URLs having long query string being too long to use as a Reportingt POST URL. This uses the same jobParams data as before, but includes it as POST payload data rather than the query string. This fixes the issues because there are fewer limits in network environments around long strings in POST payload versus long strings as a URL query string.

This fix changes a central area of code that all the export types use. In other words, it fixes the bug for:

CSV generation from Discover
PNG/PDF generation from Dashboard
PNG/PDF generation from Visualize (although there were no known problems before)

The generation API now works either way, but the UI's new behavior is the more optimal flow of including jobParams as a field of POST payload data.

Release note: Fixed a bug with Reporting where it would fail to create a job with a large state in the parameters.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

Let's make sure the Canvas menu has the same code. There may have been a pre-existing bug there, but the use case would probably fail anyway due to other timeout limits and large http payloads sent to Elasticsearch
This was checked for cross-browser compatibility, including a check against IE11
~~[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~[ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

elasticmachine · 2019-09-13T02:04:46Z

Pinging @elastic/kibana-stack-services

elasticmachine · 2019-09-13T03:28:31Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-13T20:22:41Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-13T20:44:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-14T00:30:18Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

nreese

One question is whether we want to update the docs. Getting the post url places job params in the query string. Do we need a section here, or in troubleshooting, explaining how jobParams can be put in the body if the URL is too long?

lgtm with green CI
code review, tested in chrome

tsullivan · 2019-09-17T19:29:29Z

One question is whether we want to update the docs

Thanks Nathan! Yes updating the docs is an open and legitimate checkbox for this PR.

elasticmachine · 2019-09-17T21:22:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-17T23:22:20Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-17T23:56:47Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-19T21:11:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

tsullivan · 2019-09-19T22:28:48Z

I think it would be fine to write the documentation in a followup PR, that way I can focus on running the docs server against the changes to make sure formatting looks good.

In case it's someone other than me that has to write the documentation, this is my draft:

https://www.elastic.co/guide/en/kibana/current/automating-report-generation.html#_using_watcher

Using Watcher

<Add a note that if the URL query string gets too long, they may not be able to use Watcher. See "Using a Script" below>

https://www.elastic.co/guide/en/kibana/current/automating-report-generation.html#_using_a_script

Using a Script

To automatically generate reports from a script, youÕll make a request to the POST URL and include a POST payload body containing the job parameters for the report.

The request method must be POST and it must include a kbn-version header for Kibana to allow the request.

The following example queues CSV report generation using the POST URL with cURL:
http://0.0.0.0:5601
kbn-version: 8.0.0
POST /api/reporting/generate/csv
{
  "jobParams": "<string of rison>"
}

TODO: how to get the rison

elasticmachine · 2019-09-19T23:10:08Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

tsullivan · 2019-09-20T06:46:36Z

As a future enhancement, we can add "Copy as cUrl" to the Share > PDF menu.

elasticmachine · 2019-09-20T08:40:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

joelgriffith · 2019-09-20T21:53:21Z

x-pack/legacy/plugins/reporting/server/routes/generate_from_jobparams.ts

@@ -25,13 +26,45 @@ export function registerGenerate(
  server.route({
    path: `${BASE_GENERATE}/{exportType}`,
    method: 'POST',
-    config: getRouteConfig(request => request.params.exportType),
+    config: {
+      ...getRouteConfig(request => request.params.exportType),


Nice I like this method better

I have an unrelated small fix I can make to validate the exportType param is registered in the exportTypesRegistry as an ID of one of the types

joelgriffith · 2019-09-20T21:54:30Z

x-pack/test/reporting/api/generate/fixtures.ts

@@ -158,3 +158,27 @@ export const CSV_RESULT_NANOS = `date,message,"_id"
 "2015-01-01T12:10:30.123456789Z","Hello 2",
 "2015-01-01T12:10:30","Hello 1",
 `;
+


Maybe we could use something like the backtick operator with dedent? I've seen that lib used for our dockerfiles and elsewhere. Just a bit more pleasant to work on.

export const JOB_PARAMS_IN_QUERYSTRING = dedent`(conflictedTypesFields:!(),fields:!(%27@timestamp%27,clientip,extens ion),indexPatternId:%27logstash-*%27,metaFields:!(_source,_id,_type,_index,_scor e),searchRequest:(body:(_source:(excludes:!(),includes:!(%27@timestamp%27,client ip,extension)),docvalue_fields:!(),query:(bool:(filter:!((match_all:()),(range:( %27@timestamp%27:(gte:%272015-09-20T10:19:40.307Z%27,lt:%272015-09-20T10:26:56.2 21Z%27))),(range:(%27@timestamp%27:(format:strict_date_optional_time,gte:%272004 -09-17T21:19:34.213Z%27,lte:%272019-09-17T21:19:34.213Z%27)))),must:!(),must_not :!(),should:!())),script_fields:(),sort:!((%27@timestamp%27:(order:desc,unmapped _type:boolean))),stored_fields:!(%27@timestamp%27,clientip,extension),version:!t ),index:%27logstash-*%27),title:%27A%20Saved%20Search%20With%20a%20DATE%20FILTER %27,type:search)`;

I like it! It turns out dedent still retains the line breaks, so I added in a short little function to act as the template for the string.

I also realized there does not need to be 2 consts here, because the query string one is just the same rison, just URI-encoded.

joelgriffith · 2019-09-20T21:55:39Z

x-pack/test/reporting/api/generate/csv_job_params.ts

+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+


joelgriffith

Looks good! Small comment about using dedent vs string-concatenation for those larger rison URLs

elasticmachine · 2019-09-21T08:06:05Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

tsullivan · 2019-09-23T16:39:25Z

@elasticmachine merge upstream

elasticmachine · 2019-09-23T18:21:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cqliu1

Canvas changes look good 👍

…ic#45603) * Rename route file for consistency * Generate via jobParams allows post payload * jobParams as post body * [Reporting] Skip failing test * fix types * canvas pdf generation to use post payload * api test * tests * fix a typescript * fix fn test * fix tests * fix tests * run esArchiver fewer times * return rison that came in invalid

… (#46404) * Rename route file for consistency * Generate via jobParams allows post payload * jobParams as post body * [Reporting] Skip failing test * fix types * canvas pdf generation to use post payload * api test * tests * fix a typescript * fix fn test * fix tests * fix tests * run esArchiver fewer times * return rison that came in invalid

tsullivan added 3 commits September 12, 2019 18:34

Rename route file for consistency

7791970

Generate via jobParams allows post payload

0640cab

jobParams as post body

8dd3241

tsullivan added release_note:fix (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead v8.0.0 Team:Stack Services v7.5.0 labels Sep 13, 2019

tsullivan requested review from nreese, kobelb and joelgriffith September 13, 2019 02:04

tsullivan added 5 commits September 13, 2019 10:28

[Reporting] Skip failing test

df344d0

Merge branch 'master' into reporting/job-params-as-payload

2cbfe96

fix types

68db0d4

Merge branch 'master' into reporting/job-params-as-payload

fc7fbe5

canvas pdf generation to use post payload

1b7b3d8

tsullivan requested a review from a team as a code owner September 13, 2019 19:09

tsullivan added 2 commits September 13, 2019 14:19

api test

411178e

tests

7039ee2

nreese approved these changes Sep 17, 2019

View reviewed changes

tsullivan changed the title ~~Reporting/job params as payload~~ [Reporting] Fix Generating Reports with long jobParams RISON Sep 17, 2019

tsullivan added 2 commits September 17, 2019 12:33

Merge branch 'master' into reporting/job-params-as-payload

a6d2df2

fix a typescript

d3fba77

tsullivan added 2 commits September 19, 2019 11:50

Merge branch 'master' into reporting/job-params-as-payload

451e80a

fix tests

9319e87

run esArchiver fewer times

1703ccb

Merge branch 'master' into reporting/job-params-as-payload

074982f

tsullivan mentioned this pull request Sep 20, 2019

[Canvas] Allow Print to PDF page selection #39722

Closed

2 tasks

joelgriffith reviewed Sep 20, 2019

View reviewed changes

joelgriffith approved these changes Sep 20, 2019

View reviewed changes

tsullivan added 2 commits September 20, 2019 15:49

Merge branch 'master' into reporting/job-params-as-payload

547c59d

return rison that came in invalid

5e6735b

Merge branch 'master' into reporting/job-params-as-payload

0419325

cqliu1 approved these changes Sep 23, 2019

View reviewed changes

tsullivan merged commit 5fbcfe6 into elastic:master Sep 23, 2019

tsullivan deleted the reporting/job-params-as-payload branch September 23, 2019 23:18

tsullivan mentioned this pull request Sep 23, 2019

[7.x] [Reporting] Fix Generating Reports with long jobParams RISON (#45603) #46404

Merged

tsullivan mentioned this pull request Jan 9, 2020

Reporting error Request-URI Too Long #27728

Closed

tsullivan mentioned this pull request Jul 23, 2021

[Reporting] Create reports with full state required to generate the report #101048

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reporting] Fix Generating Reports with long `jobParams` RISON #45603

[Reporting] Fix Generating Reports with long `jobParams` RISON #45603

tsullivan commented Sep 13, 2019 •

edited

Loading

elasticmachine commented Sep 13, 2019

elasticmachine commented Sep 13, 2019

elasticmachine commented Sep 13, 2019

elasticmachine commented Sep 13, 2019

elasticmachine commented Sep 14, 2019

nreese left a comment •

edited

Loading

tsullivan commented Sep 17, 2019

elasticmachine commented Sep 17, 2019

elasticmachine commented Sep 17, 2019

elasticmachine commented Sep 17, 2019

elasticmachine commented Sep 19, 2019

tsullivan commented Sep 19, 2019 •

edited

Loading

Using Watcher

Using a Script

elasticmachine commented Sep 19, 2019

tsullivan commented Sep 20, 2019

elasticmachine commented Sep 20, 2019

joelgriffith Sep 20, 2019

tsullivan Sep 21, 2019

joelgriffith Sep 20, 2019

joelgriffith Sep 20, 2019 •

edited

Loading

tsullivan Sep 21, 2019 •

edited

Loading

joelgriffith Sep 20, 2019

joelgriffith left a comment

elasticmachine commented Sep 21, 2019

tsullivan commented Sep 23, 2019

elasticmachine commented Sep 23, 2019

cqliu1 left a comment

[Reporting] Fix Generating Reports with long jobParams RISON #45603

[Reporting] Fix Generating Reports with long jobParams RISON #45603

Conversation

tsullivan commented Sep 13, 2019 • edited Loading

Summary

Checklist

elasticmachine commented Sep 13, 2019

elasticmachine commented Sep 13, 2019

💔 Build Failed

elasticmachine commented Sep 13, 2019

💚 Build Succeeded

elasticmachine commented Sep 13, 2019

💔 Build Failed

elasticmachine commented Sep 14, 2019

💔 Build Failed

nreese left a comment • edited Loading

Choose a reason for hiding this comment

tsullivan commented Sep 17, 2019

elasticmachine commented Sep 17, 2019

💔 Build Failed

elasticmachine commented Sep 17, 2019

💔 Build Failed

elasticmachine commented Sep 17, 2019

💔 Build Failed

elasticmachine commented Sep 19, 2019

💔 Build Failed

tsullivan commented Sep 19, 2019 • edited Loading

Using Watcher

Using a Script

elasticmachine commented Sep 19, 2019

💔 Build Failed

tsullivan commented Sep 20, 2019

elasticmachine commented Sep 20, 2019

💚 Build Succeeded

joelgriffith Sep 20, 2019

Choose a reason for hiding this comment

tsullivan Sep 21, 2019

Choose a reason for hiding this comment

joelgriffith Sep 20, 2019

Choose a reason for hiding this comment

joelgriffith Sep 20, 2019 • edited Loading

Choose a reason for hiding this comment

tsullivan Sep 21, 2019 • edited Loading

Choose a reason for hiding this comment

joelgriffith Sep 20, 2019

Choose a reason for hiding this comment

joelgriffith left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 21, 2019

💚 Build Succeeded

tsullivan commented Sep 23, 2019

elasticmachine commented Sep 23, 2019

💚 Build Succeeded

cqliu1 left a comment

Choose a reason for hiding this comment

[Reporting] Fix Generating Reports with long `jobParams` RISON #45603

[Reporting] Fix Generating Reports with long `jobParams` RISON #45603

tsullivan commented Sep 13, 2019 •

edited

Loading

nreese left a comment •

edited

Loading

tsullivan commented Sep 19, 2019 •

edited

Loading

joelgriffith Sep 20, 2019 •

edited

Loading

tsullivan Sep 21, 2019 •

edited

Loading