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.

Sign up for GitHub

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

Jump to bottom

[Monitoring] Handle no mappings found for sort and collapse fields #77099

Merged
merged 6 commits into from
Sep 14, 2020
Merged

[Monitoring] Handle no mappings found for sort and collapse fields #77099

merged 6 commits into from
Sep 14, 2020

Conversation

chrisronline
Copy link
Contributor

Resolves #77066

In #76015, we modified a good number of queries to work off of metricbeat-* indices, based on work done in beats, and thought we had it working perfectly for all scenarios (all data in .monitoring-*, all data in metricbeat-*, some data in .monitoring-* and some in metricbeat-*) but we didn't account for older metricbeat-* data and how that would affect our changes.

With older (and current) versions of Metricbeat, it will create mappings that do not contain any of the new aliases included in elastic/beats#19747 which actually presents an issue in a couple of ways that affect us:

  1. Attempting to sort on fields that do not exist in the mappings will throw a shard exception from ES
  2. Attempting to field collapse on fields that do not exist in the mappings will also throw a shard exception from ES

This exception will always happen but because the allow_partial_search_results parameter for the search api in ES is true by default, we only need to worry about this when the indices do not actually exist.

The good news is this only presents itself when we attempt to query against a stack products monitoring index that does not exist but it does affect the cluster overview page, as well as some listing pages too (as evident in #77066).

This PR fixes the first way elegantly by leveraging the unmapped_type feature - this change is OK because there is no actual data to sort so we just need the error to be suppressed.

However, the second way doesn't appear to have a similar elegant solution so we went down a "hack path" for this where we try/catch our API calls and look for a specific kind of exception. If it's present, we return an empty set instead of throwing the exception. Looking at the git history of the collapse error message, it seems it's been unchanged since field collapsing was first introduced so this check should work for all supported versions of ES.

@chrisronline chrisronline requested review from igoristic and a team September 9, 2020 18:37
@chrisronline chrisronline self-assigned this Sep 9, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

igoristic
igoristic approved these changes Sep 10, 2020
View reviewed changes
Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

Awesome work! Thanks for the fix🏅

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

1 similar comment
@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@chrisronline chrisronline added release_note:skip Skip the PR/issue when compiling release notes and removed v7.9.2 release_note:fix labels Sep 11, 2020
@chrisronline
Copy link
Contributor Author

@igoristic I changed a bit more code here so requested another review

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
monitoring 619 +2 617

page load bundle size

id value diff baseline
monitoring 302.6KB +4.8KB 297.8KB

distributable file count

id value diff baseline
default 45524 +2 45522

History

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

igoristic
igoristic approved these changes Sep 14, 2020
View reviewed changes
Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@chrisronline chrisronline merged commit 2055f5e into elastic:master Sep 14, 2020
@chrisronline chrisronline deleted the monitoring/sort_collapse branch September 14, 2020 18:29
@chrisronline chrisronline mentioned this pull request Sep 14, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 15, 2020
@gmmorris
Merge branch 'master' into alerting-exempt-rbac
696a7cb 
* master: (25 commits)
  [Security Solution] Add unit tests for Network search strategy (elastic#77416)
  [Alerting] Improves performance of the authorization filter in AlertsClient.find by skipping KQL parsing (elastic#77040)
  [Ingest Manager] Add route for package installation by upload (elastic#77044)
  [APM-UI][E2E] filter PRs from the uptime GH team (elastic#77359)
  [APM] Remove useLocation and some minor route improvements (elastic#76343)
  [Enterprise Search] Update enterpriseSearchRequestHandler to manage range of errors + add handleAPIErrors helper (elastic#77258)
  [SECURITY_SOLUTION] Task/hostname policy response ux updates (elastic#76444)
  Move remaining uses of serviceName away from urlParams (elastic#77248)
  [Lens] Move configuration popover to flyout (elastic#76046)
  [Ingest Manager] Manually build Fleet kuery with Node arguments (elastic#76589)
  skip flaky suite (elastic#59975)
  Neutral-naming in reporting plugin (elastic#77371)
  [Enterprise Search] Add UserIcon styles (elastic#77385)
  [RUM Dashboard] Added loading state to visitor breakdown pie charts (elastic#77201)
  [Ingest Manager] Fix polling for new agent action (elastic#77339)
  Remote cluster - Functional UI test to change the superuser to a test_user with limited role (elastic#77212)
  Stacked headers and navigational search (elastic#72331)
  [ML] DF Analytics creation wizard: Fixing field loading race condition (elastic#77326)
  [Monitoring] Handle no mappings found for sort and collapse fields (elastic#77099)
  Add Lens to Recently Accessed (elastic#77249)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 15, 2020
@gmmorris
Merge branch 'master' into actions/webhook-remove-header
17a325d 
* master: (293 commits)
  Fix tsvb filter ration for table (elastic#77272)
  [Security Solution] Add unit tests for Network search strategy (elastic#77416)
  [Alerting] Improves performance of the authorization filter in AlertsClient.find by skipping KQL parsing (elastic#77040)
  [Ingest Manager] Add route for package installation by upload (elastic#77044)
  [APM-UI][E2E] filter PRs from the uptime GH team (elastic#77359)
  [APM] Remove useLocation and some minor route improvements (elastic#76343)
  [Enterprise Search] Update enterpriseSearchRequestHandler to manage range of errors + add handleAPIErrors helper (elastic#77258)
  [SECURITY_SOLUTION] Task/hostname policy response ux updates (elastic#76444)
  Move remaining uses of serviceName away from urlParams (elastic#77248)
  [Lens] Move configuration popover to flyout (elastic#76046)
  [Ingest Manager] Manually build Fleet kuery with Node arguments (elastic#76589)
  skip flaky suite (elastic#59975)
  Neutral-naming in reporting plugin (elastic#77371)
  [Enterprise Search] Add UserIcon styles (elastic#77385)
  [RUM Dashboard] Added loading state to visitor breakdown pie charts (elastic#77201)
  [Ingest Manager] Fix polling for new agent action (elastic#77339)
  Remote cluster - Functional UI test to change the superuser to a test_user with limited role (elastic#77212)
  Stacked headers and navigational search (elastic#72331)
  [ML] DF Analytics creation wizard: Fixing field loading race condition (elastic#77326)
  [Monitoring] Handle no mappings found for sort and collapse fields (elastic#77099)
  ...
chrisronline added a commit that referenced this pull request Sep 15, 2020
@chrisronline @elasticmachine
[Monitoring] Handle no mappings found for sort and collapse fields (#…
b71eb50 
…77099) (#77386)

* Handle no mappings found for sort and collapse fields

* Add comment

* Fix sort usage

* Ensure we query off MB for new api calls as well

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@chrisronline
Copy link
Contributor Author

Backport:

7.x: b71eb50

This was referenced Dec 9, 2020
Closed
Merged
Closed
@igoristic igoristic mentioned this pull request Dec 30, 2020
Open
@chrisronline chrisronline mentioned this pull request Mar 1, 2021
Closed
1 task
@phillipb phillipb mentioned this pull request Apr 29, 2021
Open
1 task
@estermv estermv mentioned this pull request Jul 12, 2021
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igoristic igoristic igoristic approved these changes

Assignees

@chrisronline chrisronline

Labels
release_note:skip Skip the PR/issue when compiling release notes review Team:Monitoring Stack Monitoring team v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Monitoring page broken on master, No mapping found for [timestamp] in order to sort on
4 participants
@chrisronline @elasticmachine @kibanamachine @igoristic