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

[Console Monaco Migration] Implement language definition #176926

Closed
yuliacech opened this issue Feb 14, 2024 · 2 comments
Closed

[Console Monaco Migration] Implement language definition #176926

yuliacech opened this issue Feb 14, 2024 · 2 comments
Assignees
@yuliacech
Labels
Feature:Console Dev Tools Console Feature Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more

Comments

@yuliacech
Copy link
Contributor

yuliacech commented Feb 14, 2024 •
edited
Loading

After #176520 is merged, the new Console language definition will be registered in the Monaco editor. This issue contains a list of tasks that need to be implemented to enable autocomplete support in Monaco.

Tokenizer

The input in the editor first goes through the tokenizer and is split into an array of tokens. The tokenizer does an initial check if the input is valid but no in depth validation yet. The main goal of the tokenizer is to look at whitespace characters (like whitespace, tabs and new lines) and group words together into something like keywords, identifiers, comments etc.
The tokenizer is defined in the constant lexerRules: monaco.languages.IMonarchLanguage in the file packages/kbn-monaco/src/console/language.ts and consists mainly of regex rules.
The tokenizer needs to recognize following tokens:

  1. one-line and multi-line comments
  2. a line with a method and a url path
  3. a json object for the request body
  4. variables in the url path and in the request body ${}

Theme

After the tokenizer splits the input into recognized tokens, they all receive a category (like keyword, identifier, comment etc). The theme used in the Monaco editor will set the highlighting color to the tokens based on the category.

Parser (#177194)

The parser gets the array of tokens from the tokenizer and builds a semantic AST model (tree) that can be validated and used for autocomplete suggestions. The parser needs to be registered with a worker to offload the processing of the input and not block the main thread. The code of the existing parser ('sense_editor/mode/worker_parser' in the file src/plugins/console/public/application/models/legacy_core_editor/mode/worker/worker.js) could be re-used and registered similar to the xjson worker in this file packages/kbn-monaco/src/xjson/grammar.ts.

Autocomplete suggestions (needs more research)

After the tokenizer and parser are implemented, the editor has context for each token and autocomplete suggestions can be added.
One way of re-implementing autocomplete suggestions for the Monaco editor is via the CoreEditor interface. There is a lot of functionality in the SenseEditor component that relies on the code editor implementing the CoreEditor interface in the file src/plugins/console/public/types/core_editor.ts. This needs to be re-implemented for the Monaco editor anyways for functionality like auto indentation. The autocomplete suggestions are added to the editor via registerAutocompleter function.
Alternatively, an autocomplete provider (monaco.languages.CompletionItemProvider) can be registered for the Console language in the Monaco editor globally.

Follow up issues

This list is to keep track of work needed after the initial language definition is completed.
@yuliacech yuliacech added Feature:Console Dev Tools Console Feature Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more labels Feb 14, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@yuliacech yuliacech mentioned this issue Feb 15, 2024
Merged
yuliacech added a commit that referenced this issue Feb 19, 2024
@yuliacech @kibanamachine
[Console] Add a dev config for the editor migration (#176520)
4d7f593 
## Summary

This PR adds a dev config to Console to enable a migration to the Monaco
editor. The config value is `false` by default and can be changed by
adding `console.dev.enableMonaco: true` to `kibana.dev.yml`. Also a new
custom Monaco language is registered for Console in the package
`kbn/monaco` and will be worked on behind the config flag iteratively in
the next weeks.
To not let the dev config get into 8.13 release, this PR will only be
merged **after** 8.13 has been branched.
After this PR is merged, the team will start re-implementing existing
Console features in the new Monaco editor, see
#176723 and
#176926

---------

Co-authored-by: kibanamachine <[email protected]>
@yuliacech yuliacech mentioned this issue Feb 20, 2024
Merged
10 tasks
yuliacech added a commit that referenced this issue Feb 22, 2024
@yuliacech
[Console] Add Console parser to kbn/monaco (#177194)
1d4057f 
## Summary

Related meta [issue](#176926) 

This PR adds the parser used by Ace in Console to the Console language
definition in `kbn/monaco`.
Changes introduced by this PR: 
- Copy the code for `'sense_editor/mode/worker_parser'` from the file
`src/plugins/console/public/application/models/legacy_core_editor/mode/worker/worker.js`
into the `kbn/monaco` package
- Move the code for the webworker from the `xjson` folder in
`kbn/monaco` to a shared folder `ace_migration`
- Register the parser worker for the Console language in `kbn/monaco`

### How to test
#### Test the parser in Console
1. Add `console.dev.enableMonaco: true` to kibana.dev.yml
2. Open Dev Tools Console and try to type a valid request, check that
there are no red markers in the editor
3. Type an invalid request and check that there are red markers in the
editor

#### Test that `xjson` language parser still works
1. Navigate to Ingest pipelines and click the "create from csv" button
2. Load a valid csv file, for example this
[one](https://github.com/kgeller/ecs-mapper/blob/master/example/mapping.csv)
3. In the editor that now display a valid json, try changing the value
and check that red markers appear for invalid json

### Screenshots
#### Invalid request (red markers in the editor)
<img width="786" alt="Screenshot 2024-02-19 at 18 06 13"
src="https://github.com/elastic/kibana/assets/6585477/bac1bdfd-c402-45f1-9b9b-a9cc29ccb123">


#### Valid request
<img width="795" alt="Screenshot 2024-02-19 at 18 06 23"
src="https://github.com/elastic/kibana/assets/6585477/c06b1163-1077-43c6-bddc-1d86d0116266">


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.

When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| Multiple Spaces&mdash;unexpected behavior in non-default Kibana Space.
| Low | High | Integration tests will verify that all features are still
supported in non-default Kibana Space and when user switches between
spaces. |
| Multiple nodes&mdash;Elasticsearch polling might have race conditions
when multiple Kibana nodes are polling for the same tasks. | High | Low
| Tasks are idempotent, so executing them multiple times will not result
in logical error, but will degrade performance. To test for this case we
add plenty of unit tests around this logic and document manual testing
procedure. |
| Code should gracefully handle cases when feature X or plugin Y are
disabled. | Medium | High | Unit tests will verify that any feature flag
or plugin combination still results in our service operational. |
| [See more potential risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) |


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
fkanout pushed a commit to fkanout/kibana that referenced this issue Mar 4, 2024
@yuliacech @kibanamachine @fkanout
[Console] Add a dev config for the editor migration (elastic#176520)
9674128 
## Summary

This PR adds a dev config to Console to enable a migration to the Monaco
editor. The config value is `false` by default and can be changed by
adding `console.dev.enableMonaco: true` to `kibana.dev.yml`. Also a new
custom Monaco language is registered for Console in the package
`kbn/monaco` and will be worked on behind the config flag iteratively in
the next weeks.
To not let the dev config get into 8.13 release, this PR will only be
merged **after** 8.13 has been branched.
After this PR is merged, the team will start re-implementing existing
Console features in the new Monaco editor, see
elastic#176723 and
elastic#176926

---------

Co-authored-by: kibanamachine <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this issue Mar 4, 2024
@yuliacech @fkanout
[Console] Add Console parser to kbn/monaco (elastic#177194)
516e142 
## Summary

Related meta [issue](elastic#176926) 

This PR adds the parser used by Ace in Console to the Console language
definition in `kbn/monaco`.
Changes introduced by this PR: 
- Copy the code for `'sense_editor/mode/worker_parser'` from the file
`src/plugins/console/public/application/models/legacy_core_editor/mode/worker/worker.js`
into the `kbn/monaco` package
- Move the code for the webworker from the `xjson` folder in
`kbn/monaco` to a shared folder `ace_migration`
- Register the parser worker for the Console language in `kbn/monaco`

### How to test
#### Test the parser in Console
1. Add `console.dev.enableMonaco: true` to kibana.dev.yml
2. Open Dev Tools Console and try to type a valid request, check that
there are no red markers in the editor
3. Type an invalid request and check that there are red markers in the
editor

#### Test that `xjson` language parser still works
1. Navigate to Ingest pipelines and click the "create from csv" button
2. Load a valid csv file, for example this
[one](https://github.com/kgeller/ecs-mapper/blob/master/example/mapping.csv)
3. In the editor that now display a valid json, try changing the value
and check that red markers appear for invalid json

### Screenshots
#### Invalid request (red markers in the editor)
<img width="786" alt="Screenshot 2024-02-19 at 18 06 13"
src="https://github.com/elastic/kibana/assets/6585477/bac1bdfd-c402-45f1-9b9b-a9cc29ccb123">


#### Valid request
<img width="795" alt="Screenshot 2024-02-19 at 18 06 23"
src="https://github.com/elastic/kibana/assets/6585477/c06b1163-1077-43c6-bddc-1d86d0116266">


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.

When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| Multiple Spaces&mdash;unexpected behavior in non-default Kibana Space.
| Low | High | Integration tests will verify that all features are still
supported in non-default Kibana Space and when user switches between
spaces. |
| Multiple nodes&mdash;Elasticsearch polling might have race conditions
when multiple Kibana nodes are polling for the same tasks. | High | Low
| Tasks are idempotent, so executing them multiple times will not result
in logical error, but will degrade performance. To test for this case we
add plenty of unit tests around this logic and document manual testing
procedure. |
| Code should gracefully handle cases when feature X or plugin Y are
disabled. | Medium | High | Unit tests will verify that any feature flag
or plugin combination still results in our service operational. |
| [See more potential risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) |


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
@yuliacech yuliacech mentioned this issue Mar 22, 2024
Merged
yuliacech added a commit that referenced this issue Mar 27, 2024
@yuliacech @kibanamachine
[Console] Add theme and more lexer rules (#178757)
c59016e 
## Summary

This PR adds a theme for the Console language in Monaco editor and adds
more lexer rules to bring the highlighting of the input closed to the
original in Ace editor.

### Screenshots
Monaco editor 
<img width="682" alt="Screenshot 2024-03-19 at 12 38 07"
src="https://github.com/elastic/kibana/assets/6585477/98a1acc7-3a8a-4ad9-a79e-5236091c4c39">

Ace editor
<img width="651" alt="Screenshot 2024-03-19 at 12 37 52"
src="https://github.com/elastic/kibana/assets/6585477/37935a68-923b-493c-ac56-ef4982f27fdf">

### How to test
1. Add `console.dev.enableMonaco: true` to `kibana.dev.yml``
2. Type different requests into Console and check that the highlighting
works the same as in Ace. For example, use the following requests

```
GET ${pathVariable}/_search
{
 "query": {
   "match": {
     "${bodyNameVariable}": "${bodyValueVariable}",
     "number_property": 1234,
     "array_property": ["test1", 1234, false], 
     "boolean_property": true,
     "text_property": "text_value",
     "triple_quote": """
     inside triple quote
     """
     // line comment
     /* 
      block comment
    */
   }
 }
}

// line comment
/* 
block comment
*/

GET _sql
{
  "query": """
  SELECT "field" FROM "index-*" WHERE "column" = "value"
  """
}
```
3. To check that `xjson` highlighting still works
 a. Navigate to Ingest pipelines and click the "create from csv" button
b. Load a valid csv file, for example this
[one](https://github.com/kgeller/ecs-mapper/blob/master/example/mapping.csv)

#### Known issues that will be addressed in follow up PRs
- SQL highlighting needs to be re-implemented (added to the follow up
list in #176926)
- Strings inside triple quotes are not using italics (added to the
follow up list in #176926)
- Font size needs to be set via settings and the default value provided
(fixed via #178982)
- Font family: do we want to use the same font as for other Monaco
languages are use the one for Ace? (added to the follow up list in
#176926)
- In the future, we might want to use the same theme for `xjson` and
Console (added to the follow up list in
#176926)

---------

Co-authored-by: kibanamachine <[email protected]>
@yuliacech yuliacech mentioned this issue Apr 5, 2024
Closed
@yuliacech
Copy link
Contributor Author

Most of this issue was implemented in #177194 and #178757. Autocomplete support work will be completed in #180207

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yuliacech yuliacech

Labels
Feature:Console Dev Tools Console Feature Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@yuliacech @elasticmachine