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

[ML] Fix unnecessary trigger of wildcard field type search for ML plugin routes. #84605

Merged
merged 5 commits into from
Dec 1, 2020

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Dec 1, 2020

Summary

Fixes #84537.

Passing in an empty string '' to useResolver() would trigger a wild card search across all indices and fields, potentially causing a timeout and the page would fail to load. The following pages were affected:

  • Single Metric Viewer
  • Data frame analytics models list
  • Data frame analytics jobs list
  • Data frame analytics exploration page
  • File Data Visualizer (Data visualizer - Import data from a log file)

This PR fixes it by passing undefined instead of '' to useResolver to avoid calling _fields_for_wildcard with an empty pattern. Jest tests were added to cover the two parameter scenarios empty string/undefined.

Checklist

For maintainers

@walterra walterra added bug Fixes for quality problems that affect the customer experience regression :ml Feature:Anomaly Detection ML anomaly detection v8.0.0 Feature:File and Index Data Viz ML file and index data visualizer Feature:Data Frame Analytics ML data frame analytics features v7.11.0 v7.10.1 labels Dec 1, 2020
@walterra walterra self-assigned this Dec 1, 2020

const { combinedQuery } = createSearchItems(config, indexPattern!, savedSearch);
// note, currently we're using our own kibana context that requires a current index pattern to be set
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding a comment to clarify that for pages that don't require an index pattern, undefined should be passed for the index pattern and saved search IDs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comments here: 2a916a9

@walterra walterra marked this pull request as ready for review December 1, 2020 15:10
@walterra walterra requested a review from a team as a code owner December 1, 2020 15:10
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra walterra requested a review from qn895 December 1, 2020 15:14
@walterra walterra changed the title [ML] Don't pass empty strings to useResolver. [ML] Avoid passing empty strings to useResolver(). Dec 1, 2020
@walterra walterra changed the title [ML] Avoid passing empty strings to useResolver(). [ML] Fix unnecessary trigger of wildcard field type search for ML plugin routes Dec 1, 2020
@walterra walterra changed the title [ML] Fix unnecessary trigger of wildcard field type search for ML plugin routes [ML] Fix unnecessary trigger of wildcard field type search for ML plugin routes. Dec 1, 2020
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@qn895
Copy link
Member

qn895 commented Dec 1, 2020

Tested and LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 5.2MB 5.2MB +388.0B

History

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

@walterra walterra merged commit 5889e36 into elastic:master Dec 1, 2020
@walterra walterra deleted the ml-fix-use-resolver branch December 1, 2020 17:06
walterra added a commit to walterra/kibana that referenced this pull request Dec 1, 2020
…gin routes. (elastic#84605)

Passing in an empty string '' to useResolver() would trigger a wild card search across all indices and fields, potentially causing a timeout and the page would fail to load. The following pages were affected: Single Metric Viewer, Data frame analytics models list, Data frame analytics jobs list, Data frame analytics exploration page, File Data Visualizer (Data visualizer - Import data from a log file). This PR fixes it by passing undefined instead of '' to useResolver to avoid calling _fields_for_wildcard with an empty pattern. Jest tests were added to cover the two parameter scenarios empty string/undefined.
walterra added a commit to walterra/kibana that referenced this pull request Dec 1, 2020
…gin routes. (elastic#84605)

Passing in an empty string '' to useResolver() would trigger a wild card search across all indices and fields, potentially causing a timeout and the page would fail to load. The following pages were affected: Single Metric Viewer, Data frame analytics models list, Data frame analytics jobs list, Data frame analytics exploration page, File Data Visualizer (Data visualizer - Import data from a log file). This PR fixes it by passing undefined instead of '' to useResolver to avoid calling _fields_for_wildcard with an empty pattern. Jest tests were added to cover the two parameter scenarios empty string/undefined.

# Conflicts:
#	x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_job_exploration.tsx
#	x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_map.tsx
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 1, 2020
* master: (63 commits)
  Revert the Revert of "[Alerting] renames Resolved action group to Recovered (elastic#84123)"  (elastic#84662)
  declare kbn/monaco dependency on kbn/i18n explicitly (elastic#84660)
  Remove unscripted fields from sample data index-pattern saved objects (elastic#84659)
  [ML] Fix unnecessary trigger of wildcard field type search for ML plugin routes. (elastic#84605)
  Update create.asciidoc (elastic#84046)
  [Security Solution][Detections] Fix labels and issue with mandatory fields (elastic#84525)
  Fix flaky test suite (elastic#84602)
  [Security Solution] [Detections] Create a 'partial failure' status for rules (elastic#84293)
  Revert "[Alerting] renames Resolved action group to Recovered (elastic#84123)"
  Update code-comments describing babel plugins (elastic#84622)
  [Security Solution] [Cases] Cypress for case connector selector options (elastic#80745)
  [Discover] Unskip doc table tests (elastic#84564)
  [Lens] (Accessibility) Improve landmarks in Lens (elastic#84511)
  [Lens] (Accessibility) Focus mistakenly stops on righthand form (elastic#84519)
  Return early when parallel install process detected (elastic#84190)
  [Security Solution][Detections] Support arrays in event fields for Severity/Risk overrides (elastic#83723)
  [Security Solution][Detections] Fix grammatical error in validation message for threshold field in "Create new rule" -> "Define rule" (elastic#84490)
  [Fleet] Update agent details page  (elastic#84434)
  adding documentation of use of NODE_EXTRA_CA_CERTS env var (elastic#84578)
  [Search] Integrate "Send to background" UI with session service (elastic#83073)
  ...
walterra added a commit that referenced this pull request Dec 1, 2020
…gin routes. (#84605) (#84680)

Passing in an empty string '' to useResolver() would trigger a wild card search across all indices and fields, potentially causing a timeout and the page would fail to load. The following pages were affected: Single Metric Viewer, Data frame analytics models list, Data frame analytics jobs list, Data frame analytics exploration page, File Data Visualizer (Data visualizer - Import data from a log file). This PR fixes it by passing undefined instead of '' to useResolver to avoid calling _fields_for_wildcard with an empty pattern. Jest tests were added to cover the two parameter scenarios empty string/undefined.
walterra added a commit that referenced this pull request Dec 1, 2020
…gin routes. (#84605) (#84682)

Passing in an empty string '' to useResolver() would trigger a wild card search across all indices and fields, potentially causing a timeout and the page would fail to load. The following pages were affected: Single Metric Viewer, Data frame analytics models list, Data frame analytics jobs list, Data frame analytics exploration page, File Data Visualizer (Data visualizer - Import data from a log file). This PR fixes it by passing undefined instead of '' to useResolver to avoid calling _fields_for_wildcard with an empty pattern. Jest tests were added to cover the two parameter scenarios empty string/undefined.
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 Feature:Anomaly Detection ML anomaly detection Feature:Data Frame Analytics ML data frame analytics features Feature:File and Index Data Viz ML file and index data visualizer :ml regression release_note:fix v7.10.1 v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Route resolver fails to load _fields_for_wildcard
5 participants