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

Index patterns server - throw correct error on field caps 404 #95879

Merged
merged 7 commits into from
Apr 13, 2021

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Mar 30, 2021

Summary

Previously the server side field caps api call was throwing a generic error and was therefore not caught as expected. This PR fixes this and tests that relied on this incorrect error.

Also simplifies a small amount of map related index pattern code. Previously the field list was being loaded manually which wasn't necessary.

Note: better fixes could be made to the tests but the scripted field code is being deprecated anyway. The tests started failing because the API was correctly reporting errors on index patterns without backing indices.

Closes: #92947

@mattkime mattkime changed the title throw correct error on field caps 404 Index patterns server - throw correct error on field caps 404 Mar 31, 2021
@mattkime mattkime marked this pull request as ready for review April 13, 2021 04:12
@mattkime mattkime requested a review from a team as a code owner April 13, 2021 04:12
@elasticmachine
Copy link
Contributor

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

@mattkime mattkime added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed Feature:Data Views Data Views code and UI - index patterns before 8.0 labels Apr 13, 2021
@mattkime mattkime requested a review from kindsun April 13, 2021 13:54
@kindsun
Copy link
Contributor

kindsun commented Apr 13, 2021

First pass looks good overall. Maps telemetry works as expected without ugly log messages. Need to do a deeper dive, but any idea why the error message is coming through undefined in the log?

server    log   [09:46:42.145] [warning][data][data][indexPatterns][plugins] No matching indices found: kibana_sample_data_logs : undefined

To replicate:

  1. Load sample data
  2. Delete any index but retain the index pattern associated with it
  3. Load this endpoint to trigger the telemetry warning: http://localhost:5601/api/stats?extended=true

@mattkime
Copy link
Contributor Author

@aaronjcaldwell I resolved the : undefined issue.

@kindsun kindsun added the [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation label Apr 13, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

) {
throw new IndexPatternMissingIndices(pattern);
} else {
throw err;
Copy link
Member

Choose a reason for hiding this comment

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

When this is thrown, does the error get caught and logged to the server logs somewhere?

Or should there be a logger.error(err) before the throw?

Copy link
Contributor Author

@mattkime mattkime Apr 13, 2021

Choose a reason for hiding this comment

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

It gets caught and logged to the server logs - https://github.com/elastic/kibana/pull/95879/files#diff-a4e9a6425b24a341a47560c879ec9eb04e76748d29376a9c1390962d1da65c10R74 - its defined in the server abstraction layer.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

Copy link
Contributor

@kindsun kindsun left a comment

Choose a reason for hiding this comment

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

Thanks for simplifying the code on the Maps side and fixing the error handling on the data side. lgtm!

  • tested locally in chrome
  • code review, primarily Maps

@kindsun
Copy link
Contributor

kindsun commented Apr 13, 2021

@mattkime Is there any possibility these changes could be backported to 7.12.x?

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

LGTM

@mattkime mattkime merged commit d80c257 into elastic:master Apr 13, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 13, 2021
…c#95879)

* throw correct error on field caps 404 and update tests
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.12 Commit could not be cherrypicked due to conflicts
7.x

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 95879

mattkime added a commit to mattkime/kibana that referenced this pull request Apr 13, 2021
…c#95879)

* throw correct error on field caps 404 and update tests
# Conflicts:
#	src/plugins/data/server/index_patterns/index_patterns_service.ts
mattkime added a commit that referenced this pull request Apr 13, 2021
#97034)

* throw correct error on field caps 404 and update tests
# Conflicts:
#	src/plugins/data/server/index_patterns/index_patterns_service.ts
kibanamachine added a commit that referenced this pull request Apr 13, 2021
#97033)

* throw correct error on field caps 404 and update tests

Co-authored-by: Matthew Kime <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 14, 2021
…ax_primary_shard_size

* 'master' of github.com:elastic/kibana: (99 commits)
  added missing optional chain for bracket notation (elastic#96939)
  [Discover][DocViewer] Fix toggle columns from doc viewer table tab (elastic#95748)
  [TSVB] Fix per-request caching of index patterns (elastic#97043)
  [Datatable] Fix filter cell flakiness (elastic#96934)
  Unskip heatmap suite and fixes flakiness (elastic#96941)
  [Fleet] Improve performance of data stream API (elastic#97058)
  [ML] Data Frame Analytics: remove beta badge (elastic#96977)
  [App Search] Migrate expanded rows for meta engines table in Engines Overview (elastic#96251)
  Instances latency distribution chart tooltips and axis fixes (elastic#95577)
  [Monitoring] Using primary average shard size (elastic#96177)
  [Workplace Search] Hide Kibana chrome on 3rd party connector redirects (elastic#97028)
  ## [Security Solution] Fixes `Exit full screen` and `Copy to cliboard` styling issues (elastic#96676)
  Index pattern field editor - Add warning on name or type change (elastic#95528)
  [App Search] Add small engine breadcrumb utility helper (elastic#96917)
  Copy esArchiver commands from ./reassign.ts to fix tests (elastic#97012)
  [Security Solution][Detections] Updates MITRE Tactics, Techniques, and Subtechniques for 7.13 (elastic#97011)
  Index patterns server - throw correct error on field caps 404 (elastic#95879)
  Use `EuiThemeProvider` in lists plugin tests and stories (elastic#96129)
  [npm] upgrade caniuse database (elastic#97002)
  chore(NA): moving @kbn/apm-utils into bazel (elastic#96227)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/serialization/policy_serialization.test.ts
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.1 v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] Telemetry error when getFieldsForIndexPattern can't find index pattern
5 participants