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

[SearchProfiler] Fix handling of bad profile data and update tab behaviour #55806

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Jan 24, 2020

Summary

There is a less known feature of SearchProfiler that enables users to provide a profile response JSON object in the text editor. When they click "Profile" it detects the presence of profile data, short circuits the network requests and goes straight to rendering a profile tree.

It is not clear how many users actually use this functionality, given that it is not really advertised in the UI, but currently pasting a bad profile (malformed) into the SearchProfiler text editor and clicking profile crashes the app. Steps:

  1. Open SearchProfiler
  2. Paste { "profile": { "shards": [{}] } } into the text editor
  3. Click "Profile"
  4. See blank UI where SearchProfiler was.

This contribution adds a try..catch to prevent the UI from hard crashing.

This contribution also updates auto-setting of tabs under different conditions. If the user is viewing an aggregation, the UI shouldn't switch back to query if they submit another aggregation.

E.g.

{
    "aggs" : {
        "avg_grade" : { "avg" : { "field" : "grade" } }
    }
}

This bug should exist on previous Kibana versions too.

Release note

Fixed crash on bad profile data in SearchProfiler.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

Also fix tab changing upon subsequent requests
@jloleysens jloleysens changed the title [ConsoleFix searchprofiler's ability to handle badly formed profile data [SearchProfiler] Fix handling of bad profile data and update tab behaviour Jan 24, 2020
@jloleysens jloleysens added Feature:Search Profiler Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.6.0 v7.7.0 v8.0.0 labels Jan 24, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Code LGTM. Tested locally; confirmed fix and verified tab behavior. I think as an improvement to this (not blocking), it would be nice to render a callout message or something indicating the user provided bad profile data, rather than just an empty screen.

@jloleysens jloleysens merged commit b8f7748 into elastic:master Jan 24, 2020
@jloleysens jloleysens deleted the fix/searchprofiler/handle-bad-profile-data branch January 24, 2020 14:37
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 24, 2020
…viour (elastic#55806)

* Fix searchprofiler's ability to handle badly formed profile data
Also fix tab changing upon subsequent requests

* Fix comment typo
jloleysens added a commit that referenced this pull request Jan 24, 2020
…viour (#55806) (#55844)

* Fix searchprofiler's ability to handle badly formed profile data
Also fix tab changing upon subsequent requests

* Fix comment typo
@cjcenizal
Copy link
Contributor

TIL 💡

jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 27, 2020
…viour (elastic#55806)

* Fix searchprofiler's ability to handle badly formed profile data
Also fix tab changing upon subsequent requests

* Fix comment typo

# Conflicts:
#	x-pack/legacy/plugins/searchprofiler/public/np_ready/application/utils/has_aggregations.ts
#	x-pack/legacy/plugins/searchprofiler/public/np_ready/application/utils/has_searches.ts
#	x-pack/plugins/searchprofiler/public/application/components/profile_tree/__tests__/profile_tree.test.tsx
#	x-pack/plugins/searchprofiler/public/application/containers/main/main.tsx
jloleysens added a commit that referenced this pull request Jan 27, 2020
…viour (#55806) (#55971)

* Fix searchprofiler's ability to handle badly formed profile data
Also fix tab changing upon subsequent requests

* Fix comment typo

# Conflicts:
#	x-pack/legacy/plugins/searchprofiler/public/np_ready/application/utils/has_aggregations.ts
#	x-pack/legacy/plugins/searchprofiler/public/np_ready/application/utils/has_searches.ts
#	x-pack/plugins/searchprofiler/public/application/components/profile_tree/__tests__/profile_tree.test.tsx
#	x-pack/plugins/searchprofiler/public/application/containers/main/main.tsx

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@cjcenizal cjcenizal added this to Done in Elasticsearch UI Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Search Profiler release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.6.0 v7.7.0 v8.0.0
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants