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] remove async execution from csv_from_savedobject #71031

Merged
merged 5 commits into from
Jul 9, 2020

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Jul 8, 2020

Summary

Follows #65213
Part of #64853
Closes #37471
Closes #62986

Some minor refactoring and hardening of the code that came up while working on Task Manager integration. These changes came from resolving TypeScript errors on over-complex and hard to test code.

This PR removes the async option of execution from csv_from_savedobject. When that feature was added, it was opportunistic based on the way Kibana worked, and was possible due to the lack of TypeScript in Reporting and Kibana overall. It had the hope of becoming the next functionality of Export CSV from Discover, and benefit the code through more code reuse.

The work on CSV exports and code re-use is happening in #67027

Checklist

Delete any items that are not applicable to this PR.

For maintainers

tsullivan added 2 commits July 7, 2020 17:04
This simplifies the csv_from_savedobject logic by removing the async
hook. This was added as premature optimization in the initial PR that
added the Download CSV button to the dashboards.
@tsullivan tsullivan requested a review from joelgriffith July 8, 2020 01:06
@tsullivan tsullivan added (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0 labels Jul 8, 2020
@tsullivan tsullivan changed the title Reporting/simplify csv ii [Reporting] remove async execution from csv_from_savedobject Jul 8, 2020
req
);
// FIXME: no scheduleTaskFn for immediate download
const jobDocPayload = await scheduleTaskFn(jobParams, req.headers, context, req);
Copy link
Member Author

@tsullivan tsullivan Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is what creates the complexity with getting Reporting to work with Task Manager. This CSV export handler uses the same methods other export types use for queuing, but that bind needs to be separated since this handler is responsible for "immediate" download of CSV without queuing.

By simplifying some TypeScript, this PR does enough to separate the bind to allow moving ahead with Task Manager work in Reporting.

@tsullivan tsullivan marked this pull request as ready for review July 8, 2020 18:38
@tsullivan tsullivan requested review from a team July 8, 2020 18:38
@mikecote mikecote self-requested a review July 8, 2020 19:05
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@tsullivan tsullivan merged commit a9f82a3 into elastic:master Jul 9, 2020
@tsullivan tsullivan deleted the reporting/simplify-csv-ii branch July 9, 2020 16:58
tsullivan added a commit to tsullivan/kibana that referenced this pull request Jul 9, 2020
…#71031)

* [Reporting] remove async execution from csv_from_savedobject

This simplifies the csv_from_savedobject logic by removing the async
hook. This was added as premature optimization in the initial PR that
added the Download CSV button to the dashboards.

* copy out export type ts changes

* remove routes

* fix i18n
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 13, 2020
tsullivan added a commit that referenced this pull request Jul 13, 2020
…#71281)

* [Reporting] remove async execution from csv_from_savedobject

This simplifies the csv_from_savedobject logic by removing the async
hook. This was added as premature optimization in the initial PR that
added the Download CSV button to the dashboards.

* copy out export type ts changes

* remove routes

* fix i18n

Co-authored-by: Elastic Machine <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 13, 2020
tsullivan added a commit that referenced this pull request Jan 11, 2023
## Summary

This restores an endpoint that was added in 7.3 in [this
PR](#34571), and was removed in
7.9 in [this PR](#71031). The
changes are re-done on top of 7.17, but still has a mostly-compatible
with the one that existed in 7.3-7.8. This serves 3rd parties that
relied on the earlier experimental code.

Supports:
* Saved searches with filters
* Saved searches with custom sorting
* Saved searches with or without selected columns
* Exports based on Index Patterns with or without a "time field"
* Requests can have an [optional POST
body](https://github.com/elastic/kibana/pull/148030/files#diff-0f565e26f3309c257fa919c5db227c3b7a78237015940c3d3677cbb1132a6701R27-R37)
with extra time range filters and/or specify a custom time zone.

LIMITATIONS:
* This endpoint is currently not supported in 8.x at this time.
* Saved Search objects created in older versions of Kibana may not work.
* Searching across hundreds of shards in the query could cause
Elasticsearch instability.
* Some minor bugs in the output of the CSV may exist, such as fields not
being formatted exactly as in the Discover table.
* This code may be forward-ported to `main` in a way that uses a
different API that is not compatible with this change.
* Does not allow "raw state" to be merged with the Search object, as in
the previous code. Otherwise, the API is compatible with the previous
code.
* This feature remains in "experimental" status, and is not ready to be
documented at this time.

## Testing
Since there is not a UI for this endpoint, there are a few options for
testing:
1. Run the functional test:
```sh
node scripts/functional_tests.js \
  --config x-pack/test/reporting_api_integration/reporting_and_security.config.ts \
  --grep 'CSV Generation from Saved Search ID'
```

2. Create a saved search in Kibana, and use a script to send a request
```sh
POST_URL="${HOST}/api/reporting/v1/generate/csv/saved-object/search:"$SAVED_SEARCH_ID

## Run transaction to generate a report, wait for execution completion, download the report, and send the
# report as an email attachment

# 1. Send a request to generate a report
DOWNLOAD_PATH=$(curl --silent -XPOST "$POST_URL" -H "kbn-xsrf: kibana-reporting" -H "${AUTH_HEADER}" | jq -e -r ".payload.path | values")
if [ -z "$DOWNLOAD_PATH" ]; then
  echo "Something went wrong! Could not send the request to generate a report!" 1>&2
  # TEST
  curl --silent -XPOST "$POST_URL" -H "kbn-xsrf: kibana-reporting" -H "${AUTH_HEADER}"
  exit 1
fi

# 2. Log the path used to download the report
DOWNLOAD_PATH=${HOST}$DOWNLOAD_PATH
echo Download path: $DOWNLOAD_PATH

# 3. Wait for report execution to finish
echo While the report is executing in the Kibana server, the reporting service will return a 503 status code response.
STATUS=''
while [[ -z $STATUS || $STATUS =~ .*503.* ]]
do
  echo Waiting 5 seconds...
  sleep 5
  STATUS=$(curl --silent --head "$DOWNLOAD_PATH" -H "${AUTH_HEADER}" | head -1)
  if [[ -z "$STATUS" || $STATUS =~ .*500.* ]]; then
    echo "Something went wrong! Could not request the report execution status!" 1>&2
    curl "$DOWNLOAD_PATH" -H "${AUTH_HEADER}" 1>&2
    exit 1
  fi
  echo $STATUS
done

# 4. Download final report and show the contents in the console
curl -v "$DOWNLOAD_PATH" -H "$AUTH_HEADER"
```

3. Test that the above script from (2) works in 7.8, and continues to
work after migrating to 7.17.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0
Projects
None yet
3 participants