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] Fix ability to export CSV on searched data with frozen indices #109976

Merged

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Aug 24, 2021

Summary

Fixes #109167

CSV Export should search frozen indices if the Advanced Setting search:includeFrozen is true

Checklist

Delete any items that are not applicable to this PR.

@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@tsullivan tsullivan marked this pull request as draft August 24, 2021 23:18
@tsullivan tsullivan force-pushed the reporting/fix-csvsearchsource-frozen-idxes branch 2 times, most recently from f46fdcf to 63c1aeb Compare August 24, 2021 23:49
@tsullivan tsullivan force-pushed the reporting/fix-csvsearchsource-frozen-idxes branch from ba0dc89 to e55ba57 Compare August 24, 2021 23:55
@tsullivan tsullivan marked this pull request as ready for review August 24, 2021 23:58
@tsullivan tsullivan force-pushed the reporting/fix-csvsearchsource-frozen-idxes branch from 4dd6c86 to bb2585b Compare August 25, 2021 05:01
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

This looks great @tsullivan , thanks for adding the functional test 👏🏻

@tsullivan tsullivan enabled auto-merge (squash) August 25, 2021 19:28
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
reporting 67.4KB 67.6KB +191.0B

History

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

@tsullivan tsullivan merged commit 06c6168 into elastic:master Aug 25, 2021
tsullivan added a commit to tsullivan/kibana that referenced this pull request Aug 26, 2021
…dices (elastic#109976)

* use include frozen setting in csv export

* add api integration test

* add fixes

* Update x-pack/test/reporting_api_integration/reporting_and_security/search_frozen_indices.ts

* test polish

* update per feedback
tsullivan added a commit to tsullivan/kibana that referenced this pull request Aug 26, 2021
…dices (elastic#109976)

* use include frozen setting in csv export

* add api integration test

* add fixes

* Update x-pack/test/reporting_api_integration/reporting_and_security/search_frozen_indices.ts

* test polish

* update per feedback
@tsullivan tsullivan deleted the reporting/fix-csvsearchsource-frozen-idxes branch August 26, 2021 00:16
tsullivan added a commit that referenced this pull request Aug 26, 2021
…dices (#109976) (#110162)

* use include frozen setting in csv export

* add api integration test

* add fixes

* Update x-pack/test/reporting_api_integration/reporting_and_security/search_frozen_indices.ts

* test polish

* update per feedback
tsullivan added a commit that referenced this pull request Aug 26, 2021
…dices (#109976) (#110163)

* use include frozen setting in csv export

* add api integration test

* add fixes

* Update x-pack/test/reporting_api_integration/reporting_and_security/search_frozen_indices.ts

* test polish

* update per feedback
@@ -45,6 +45,7 @@ export const KBN_SCREENSHOT_HEADER_BLOCK_LIST = [

export const KBN_SCREENSHOT_HEADER_BLOCK_LIST_STARTS_WITH_PATTERN = ['proxy-'];

export const UI_SETTINGS_SEARCH_INCLUDE_FROZEN = 'search:includeFrozen';
Copy link
Member

Choose a reason for hiding this comment

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

cc @lukasolson shouldn't this setting be respected inside search service ? (and as such we should not need to set it here ?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed: #110679

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like this setting is only respected with the ESE_SEARCH_STRATEGY. Unfortunately, CSV export can not use that search strategy because it doesn't support scroll queries: #110679 (comment)

@sophiec20 sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024
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:fix v7.15.0 v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reporting: CSV export does not work on frozen indices.
6 participants