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

Create Painless Lab app #57538

Merged
merged 30 commits into from
Mar 23, 2020
Merged

Create Painless Lab app #57538

merged 30 commits into from
Mar 23, 2020

Conversation

cjcenizal
Copy link
Contributor

@cjcenizal cjcenizal commented Feb 13, 2020 •
edited
Loading

Pertains to #59500

See original PR #54578 for history, including alternative approaches in the UI (e.g. a "Submit" button for sending the request instead of live-updating).

Release note

The Painless Lab dev tool helps you experiment with and learn about the Painless language.

image

Use cases

Currently, Kibana allows users to test out scripted fields against their cluster data from within Management > Index Patterns > Scripted Fields, but there are many other ways Painless can be used, which aren't addressed by that particular tool.

Ingested data

In this situation, the user has ingested data they want to simulate a script against. However even this use case extends beyond the way the Index Pattern Scripted Fields UI is used. That use case is limited to the Field context but there are numerous other contexts which expect ingested data (reindexing, watcher conditions/transforms, and so on).

The ideal solution might be to update each UI with better Painless support within that particular context, the same way the Scripted Fields UI does. But in the meantime, generic support for these testing scripts within these contexts in the Painless Lab could be a more efficient way to provide this functionality to users.

Pre-ingestion data

Tal mentioned that there's a strong ingestion use case for Painless, specifically the ingest node Script processor which has access to the Ingest processor context. In this case, the user has data they're going to ingest but the data isn't yet available in the cluster. So the Scripted Fields UX isn't applicable here.

Developer tool

Stu helped me understand the role this tool can play for our own development of the Painless language and API. For example, by being able to see how errors are surfaced in the UI, they were able to improve the way syntax errors are indicated in the error response (elastic/elasticsearch#51069). Jack also mentioned that this UI will support their development of an API to support autocomplete.

Prep for other apps, e.g. Watcher

As mentioned by @jeffvestal in #54578 (comment):

Painless was used in different areas with es but one that caused me the most frequent pain was writing a lot of "advanced" watcher jobs that used painless in the conditions and formatting output. I went through a lot of frustration writing and debugging scripts.

What I generally did to debug watcher issues was set the query to something I knew would trigger the watch, then simulate the output of watcher, looking through the error output, and repeating.

Other use cases of painless around querying data, for example, would just be debugged in the dev tools console, tweaking a script, sending the request, checking the error output, repeating.

Having a syntax editor where I could have queried existing data would have been a huge help.

To-do

Requirements for shipping

Optional enhancements

  • Explore allowing the user to opt out of live updating and use a "Submit" button instead, per Refine Painless Playground UI/UX kertal/kibana#5 (comment)
  • Move help into the main Kibana header bar help menu (see screenshot below)
  • Fix the odd visual layout issues with the code editors (large gutters, overlapping lines).
  • Prepopulate index-selection field with a list of available indices. At a later stage we can enhance this to be a flyout that lets you preview the mappings of each index to help you select the one you want.
  • The parameters field should be a component that lets you add key-value pairs via a list editor. There can be a button that lets you switch back and forth between the KV list mode and JSON editing mode.

Kibana header bar help menu

image

Resources

@cjcenizal cjcenizal added Feature:Dev Tools enhancement New value added to drive a business result painless painless Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more labels Feb 13, 2020
@cjcenizal cjcenizal requested a review from a team as a code owner February 13, 2020 09:02
@elasticmachine
Copy link
Contributor

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

@cjcenizal cjcenizal mentioned this pull request Feb 13, 2020
Merged
27 tasks
@cjcenizal cjcenizal added release_note:enhancement v7.7.0 v8.0.0 and removed enhancement New value added to drive a business result labels Feb 13, 2020
@cjcenizal
Improve smiley face proportions.
74cd1c6 
- Add def keyword to Painless spec.
- Temporarily fix broken highlighting.
- Add small padding to main controls.
@jloleysens
Copy link
Contributor

From the API docs it looks like the UI is missing a "query" field for score context.

On that, it looks like the contexts can also be dynamically created beyond known ones. We should consider supporting this in future iterations.

@cjcenizal cjcenizal changed the title Create Painless Playground app Create Painless Lab app Feb 21, 2020
@jloleysens
[Painless Lab] Minor Fixes (#58135)
35de7e0 
* Code restructure, improve types, add plugin id, introduced hook

Moved the code execution hook to a custom hook outside of main,
also chaining off promise to avoid lower level handling of
sequencing.

* Re-instated formatting code

To improve DX the execution error response from the painless API
was massaged to a more reader friendly state, only giving non-repeating
information.

Currently it is hard to determine the line and character information from
the painless endpoint. If the user wishes to see this raw information it
will be available in the API response flyout.

* Remove leading new line in default script

* Remove registration of feature flag

* Fix types

* Restore previous auto-submit request behaviour

* Remove use of null and remove old comment

Stick with "undefined" as the designation for something not existing.
@cjcenizal
Copy link
Contributor Author

@elasticmachine merge upstream

@mattkime mattkime mentioned this pull request Mar 5, 2020
Closed
@cjcenizal
Copy link
Contributor Author

@elasticmachine merge upstream

elasticmachine and others added 2 commits March 14, 2020 09:20
@elasticmachine
Merge branch 'master' into app/painless
07abc99
@cjcenizal
[Painless Lab] Fix float -> integer coercion bug (#60201)
3de6244 
* Clarify data and persistence flow. Fix floating point precision bug.
* Send a string to API and ES client instead of an object.
@cjcenizal
Copy link
Contributor Author

@elasticmachine merge upstream

@stu-elastic stu-elastic mentioned this pull request Mar 20, 2020
Closed
@jdconrad
Copy link

Just want to highlight that this whole project is awesome! I'm looking forward to working with @stu-elastic and the UI team to continue to add new features and make more improvements.

After testing this I do not have much to add in the way of constructive feedback that I think should go into this specific PR outside of what @stu-elastic has already highlighted. The one thing I keep thinking is it seems a bit clunky to have the output of the error message both take up a significant portion of the usable space and be presented as part of a screen on the right side. I'm hopeful that this can be migrated to syntax error highlighting in a similar way that other IDE's such as Intellij/Eclipse do it with the underlined red syntax showing the error and pop up to say what the error is.

jdconrad
jdconrad approved these changes Mar 20, 2020
View reviewed changes
Copy link

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

From my perspective this is ready to go as a beta feature. Thanks to all that put in the work to make this happen.

@stu-elastic
Copy link
Contributor

  1. (Future) The null in the response in this error is a bit weird
    painless-null

@stu-elastic
Copy link
Contributor

6 (Future) Regex literals are not highlighted

painless-regex

stu-elastic
stu-elastic approved these changes Mar 20, 2020
View reviewed changes
Copy link
Contributor

@stu-elastic stu-elastic left a comment

Choose a reason for hiding this comment

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

No show stoppers.

This is amazing work, I can't wait for customers to get it into their hands.

@cjcenizal
Copy link
Contributor Author

@cchaos Could you take a look at the Sass? No UI review necessary at this point.

@jloleysens
Copy link
Contributor

@elasticmachine merge upstream

@cchaos
Copy link
Contributor

cchaos commented Mar 23, 2020

@cjcenizal If this is going to be released in 7.7, then I think it deserves a day for a simple UI/design review. Especially when it comes to the use of the bottom bar.

From the screenshots, I can tell that there's an overlap issue with the nav drawer, and the application is re-inventing where to get help. You should look at how the Advanced Settings page fixes the nav drawer overlap issue or reconsider if it's necessary to use the bottom bar vs putting the help in the header help menu.

We can push you a PR to cleanup spacing, sizing, etc issues.

@cjcenizal
Copy link
Contributor Author

@cchaos A PR for cleaning up layout would be great, thanks!

If this is going to be released in 7.7, then I think it deserves a day for a simple UI/design review.

Sounds great. All feedback is always welcome esp re that bottom bar.

Re some of your comments, #60833 will address the bottom bar overlap issue once merged. I have an issue with the way the help menu in the top nav makes it non-obvious that there is app-related help in there (it looks the same whether it contains app-related content or not). I'd like to surface it more prominently within the app until we find a way around that. I'd be happy to Zoom about this.

@cjcenizal cjcenizal requested a review from a team March 23, 2020 17:30
@cjcenizal
Validate Painless Lab index field (#60841)
d0d3036 
* Make JSON format of parameters field more prominent. Set default parameters to provide an example to users.
* Set default document to provide an example to users.
* Simplify context's updateState interface.
* Refactor store and context file organization.
  - Remove common directory, move constants and types files to root.
  - Move initialState into context file, where it's being used.
* Add validation for index input.
* Create context directory.
@cjcenizal
Fix bottom bar z-index.
e77eb94
@cjcenizal
Position flyout help link so it's bottom-aligned with the title and f…
111f2f8 
…arther from the close button.
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

mdefazio
mdefazio approved these changes Mar 23, 2020
View reviewed changes
Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

SCSS looks good. And CJ made some tweaks to the UI per our discussion. I think we'll want to revisit the structure of the right side. But we need a bit more time to figure out the right direction.

@cjcenizal
Copy link
Contributor Author

cjcenizal commented Mar 23, 2020 •
edited
Loading

@elastic/kibana-app I can't spot any changed files which fall under your purview, so I'm going to override the codeowner block and merge.

@cjcenizal cjcenizal merged commit f32a848 into master Mar 23, 2020
@cjcenizal cjcenizal deleted the app/painless branch March 23, 2020 23:10
@cjcenizal cjcenizal mentioned this pull request Mar 23, 2020
Closed
@cjcenizal cjcenizal added the Feature:Painless Lab Dev tool for learning Painless label Mar 23, 2020
@cjcenizal cjcenizal mentioned this pull request Mar 23, 2020
Merged
cjcenizal added a commit that referenced this pull request Mar 24, 2020
@cjcenizal @kertal
@jloleysens @elasticmachine @alisonelizabeth
Create Painless Lab app (#57538) (#61015)
1cfecfc 
* Create Painless Playground app (#54578)

* Replace heart script with smiley face script. (#57755)

* Rename Painless Playground -> Painless Lab. (#57545)

* Fix i18n namespace.

* Improve smiley face proportions.
- Add def keyword to Painless spec.
- Temporarily fix broken highlighting.
- Add small padding to main controls.

* [Painless Lab] Minor Fixes (#58135)

* Code restructure, improve types, add plugin id, introduced hook

Moved the code execution hook to a custom hook outside of main,
also chaining off promise to avoid lower level handling of
sequencing.

* Re-instated formatting code

To improve DX the execution error response from the painless API
was massaged to a more reader friendly state, only giving non-repeating
information.

Currently it is hard to determine the line and character information from
the painless endpoint. If the user wishes to see this raw information it
will be available in the API response flyout.

* Remove leading new line in default script

* Remove registration of feature flag

* Fix types

* Restore previous auto-submit request behaviour

* Remove use of null and remove old comment

Stick with "undefined" as the designation for something not existing.

* [Painless Lab] NP migration (#59794)

* Fix sample document editor.

* [Painless Lab] Fix float -> integer coercion bug (#60201)

* Clarify data and persistence flow. Fix floating point precision bug.
* Send a string to API and ES client instead of an object.

* Rename helpers lib to format. Add tests for formatRequestPayload.

* Add query parameter to score context (#60414)

* Fix typo and i18n

* Make state init lazy

Otherwise we are needlessly reading and JSON.parse'ing on every
state update

* Support the query parameter in requests to Painless

* Fix borked i18n

* Fix i18n

* Another i18n issue

* [Painless] Minor state update model refactor (#60532)

* Fix typo and i18n

* Make state init lazy

Otherwise we are needlessly reading and JSON.parse'ing on every
state update

* Support the query parameter in requests to Painless

* WiP on state refactor

* Some cleanup after manual testing

* Fix types and i18n

* Fix i18n in context_tab

* i18n

* [Painless] Language Service (#60612)

* Added language service

* Use the correct monaco instance and add wordwise operations

* Remove plugin context initializer for now

* [Painless] Replace hard-coded links (#60603)

* Replace hard-coded links

Also remove all props from Main component

* Pass the new links object to the request flyout too

* Link directly to painless execute API's contexts

* Remove responsive stacking from tabs with icons in them.

* Resize Painless Lab bottom bar to accommodate nav drawer width (#60833)

* Validate Painless Lab index field (#60841)

* Make JSON format of parameters field more prominent. Set default parameters to provide an example to users.
* Set default document to provide an example to users.
* Simplify context's updateState interface.
* Refactor store and context file organization.
  - Remove common directory, move constants and types files to root.
  - Move initialState into context file, where it's being used.
* Add validation for index input.
* Create context directory.

* Fix bottom bar z-index.

* Position flyout help link so it's bottom-aligned with the title and farther from the close button.

Co-authored-by: Matthias Wilhelm <[email protected]>
Co-authored-by: Jean-Louis Leysens <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Alison Goryachev <[email protected]>

Co-authored-by: Matthias Wilhelm <[email protected]>
Co-authored-by: Jean-Louis Leysens <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Alison Goryachev <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 24, 2020
@gmmorris
Merge branch 'master' into alerting/tls-warning
1b3d0b4 
* master: (34 commits)
  [APM] add service map config options to legacy plugin (elastic#61002)
  [App Arch] migrate legacy CSS to new platform (core_plugins/kibana_react) (elastic#59882)
  Migrated styles for "share" plugin to new platform (elastic#59981)
  [ML] Module setup with dynamic model memory estimation (elastic#60656)
  Drilldowns (elastic#59632)
  Upgrade mocha dev-dependency from 6.2.2 to 7.1.1 (elastic#60779)
  [SIEM] Overview: Recent cases widget (elastic#60993)
  [ML] Functional tests - stabilize df analytics clone tests (elastic#60497)
  [SIEM] Updates process and TLS tables to use ECS 1.5 fields (elastic#60854)
  Migrate doc view part of discover (elastic#58094)
  Revert "[APM] Collect telemetry about data/API performance (elastic#51612)"
  fix(NA): log rotation watchers usage (elastic#60956)
  [SIEM] [CASES] Build lego blocks case details view (elastic#60864)
  Create Painless Lab app (elastic#57538)
  [SIEM] Move Timeline Template field to first step of rule creation (elastic#60840)
  [Reporting/New Platform Migration] Use a new config service on server-side (elastic#55882)
  [Alerting] allow email action to not require auth (elastic#60839)
  [Maps] Default ES document layer scaling type to clusters and show scaling UI in the create wizard (elastic#60668)
  [APM] Collect telemetry about data/API performance (elastic#51612)
  Implement Kibana Login Selector (elastic#53010)
  ...
@kertal
Copy link
Member

kertal commented Mar 24, 2020

Been early at the party, and now I'm a bit late (busy with work before our features turn to ice):
LVGTM! Excited that this was merged! Let's have a painless lab birthday party 🎂 🎈 🎁 !

@jloleysens jloleysens mentioned this pull request Mar 30, 2020
Open
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stu-elastic stu-elastic stu-elastic approved these changes

@mdefazio mdefazio mdefazio approved these changes

@jdconrad jdconrad jdconrad approved these changes

@spalger spalger spalger approved these changes

@kertal kertal Awaiting requested review from kertal

@cchaos cchaos Awaiting requested review from cchaos

Assignees
No one assigned
Labels
Feature:Dev Tools Feature:Painless Lab Dev tool for learning Painless painless painless release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.