[Runtime fields] Add support in index template by sebelga · Pull Request #84184 · elastic/kibana

sebelga · 2020-11-24T09:31:35Z

This PR integrates the runtime field editor (from the runtimeFields x-pack plugin) into the mappings editor (index_management x-pack plugin). As the mappings editor is integrated into composable index template, legacy index template, and component template wizard, the runtime section of the mappings is automatically supported in those three resources.

Note: I removed the release note from #79940 and added it here.

Notes for code reviewers

How to test

Navigate to the index management app
Click on the index template tab and then "Create template" button
Enter a name and an index pattern in the "Logistics" step
Navigate to the mappings step. There should be a "Runtime fields" tab
Click on the "Runtime fields" tab. There should be an empty prompt with a button to create a runtime field
Note: There should be a "Learn more" link. I asked for the link to the docs team and pointed it correctly. It does not work yet as the docs are not deployed yet. cc @DocTeam
Click the "Create a runtime field" button. The editor should open
Give a name to the field (e.g. "myField") and a type. For the script emit a test value: emit("test")
Click "Save". The field should appear in the list.
You should be able to edit and delete the field.

Shadowing mapped field

Navigate to the "Mapped fields" tab and create a field (e.g. with name "mappedField")
Navigate to the "Runtime fields" tab and click "Add field".
Give the same name to the field as the mapped field. A callout should appear indicating that it will shadow the mapped field
Proceed anyway. Add any script and click "Save"
The field should appear in the list.
Navigate to the "Mapped fields". The mapped field should have a "shadowed" badge on it with a tooltip

Prevent duplicated runtime field

Navigate to the "Runtime fields" tab and click "Add field" button
Give the same name as one of the existing runtime fields (e.g. "myField" from the first example)
There should be a validation error indicating that a field with the same name already exists

Handling ES error

As part of this PR I fixed #77165 which was required to help the user better understand possible errors in the runtime script when saving the template. For that, I created an es_error_parser.ts reusable handler in our "es_ui_shared/server" folder.

Tests

I added all the necessary component integration tests for the support of the runtime tab in the mappings. I also updated the API integration tests to verify that the es_error_parser correctly wraps the returned error from our endpoint.

Copy review

Empty prompt

Field shadowed

If a runtime field has the same name as a mapped field, a "shadowed" badge appears with a tooltip.

In the runtime field editor, a callout indicates that this field is shadowing a mapped field.

Tag line

When there are 1 or more fields, there is a tagline on top with a "Learn more" link.

Release note

Index templates and component templates can now be configured with runtime fields in their mappings.

Fixes #77165

Fixes elastic#77165

…field editor

… shadowing

sebelga · 2020-11-30T08:57:51Z

Thanks for the review @jloleysens ! I will wait on a review from the @esdocs team before merging 👍

sebelga · 2020-12-02T15:01:20Z

@elasticmachine merge upstream

lockewritesdocs

Overall, looks really good! I left some suggested changes for wording, and also think we can provide more targeted URLs in a few spots. I left a comment about error handling when a script isn't provided and tagged Luca for his input.

lockewritesdocs · 2020-12-02T21:48:57Z

+        {
+          validator: emptyField(
+            i18n.translate('xpack.runtimeFields.form.validations.scriptIsRequiredErrorMessage', {
+              defaultMessage: 'Script must emit() a value.',


@sebelga, a script is no longer required to create a runtime field since @romseygeek merged 64981. I believe the script is still the preferred way, so perhaps we want users to create a script. But throwing in an error that forces users to create a script - when they don't have to - seems incorrect.

cc: @javanna to get his input on how we want to handle this situation.

Great catch 👍 I will add an (optional) to the label and let the user save the field without a script.

sorry for being late, maybe users will wonder what the runtime field will do when a script is not defined, it may be worth to include that in the UI

Good catch Luca. I created a PR to add text that explains this behavior: #85204

lockewritesdocs · 2020-12-02T22:00:16Z


      {/* Script */}
-      <UseField<string> path="script">
+      <UseField<string> path="script.source">


This is a comment about links.painlessSyntax on line 229, which just goes to the main page for the Painless guide. I don't think that information is particularly useful in this context. Instead, we could link to the section for Mapping a runtime field, which provides users information about writing a Painless script in the context of a runtime field.

I believe the resulting URL would be: https://www.elastic.co/guide/en/elasticsearch/reference/master/runtime.html#runtime-mapping-fields

This is great idea! I will change the link 👍

…mappings_editor/__jest__/client_integration/runtime_fields.test.tsx Co-authored-by: Adam Locke <adam.locke@elastic.co>

Co-authored-by: Adam Locke <adam.locke@elastic.co>

…-mappings-2

sebelga · 2020-12-03T09:38:49Z

Thanks for the review @lockewritesdocs ! I've committed your change suggestions and made the suggested change. Thanks for pointing out that the script is now optional! 👍

Can you have another look? Cheers!

lockewritesdocs

👏 Looks great @sebelga! I built locally and tested. Thanks for integrating these changes.

kibanamachine · 2020-12-03T14:44:01Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: ea928f6

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`indexManagement`	512	514	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`indexManagement`	1.5MB	1.5MB	+12.0KB

Distributable file count

id	before	after	diff
`default`	43460	43461	+1
`oss`	22744	22745	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`esUiShared`	278.1KB	278.2KB	+99.0B
`indexManagement`	114.2KB	114.4KB	+161.0B
`runtimeFields`	24.2KB	29.8KB	+5.6KB
total			+5.8KB

History

💔 Build #91680 failed 89b6dbf
💔 Build #91651 failed 4fa275a
💚 Build #91371 succeeded 5f1fb37
💚 Build #90710 succeeded 8a65bc8
💚 Build #90609 succeeded cd2dabf

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

sebelga force-pushed the runtime-fields-in-mappings-2 branch from d311bde to ad8fb03 Compare November 24, 2020 13:47

sebelga added 23 commits November 26, 2020 09:59

Revert changes to add runtime field to mappings properties

1ad89cc

Update state and reducer with new actions

3ff47f9

Add tab to mappings editor for runtime fields

48ab889

Export flyout content props from runtime fields

bb4e7f7

Add default ownFocus "true" to global flyout

4c657d9

Use NormalizedRuntimeField in state

8514aac

List runtime fields from state

7952308

Delete runtime field from list

9e7be04

Edit runtime field

916d9d6

List provided runtime fields

29c8fe9

Put script inside source prop

9c33b0b

Add create field button below list

3085f50

Forward runtime fields to parent component

5f8d8c9

Improve error description when creating or updating index template

d6f6452

Fixes elastic#77165

Enhance error from simulate template

a83d6aa

Use ConfigProvider in mappings editor to provide docLinks to runtime …

2c39d68

…field editor

Fix tests and TS issues

9b0a240

Fix test

0327775

Add badge to indicate when a field is shadowed

e32fe7e

Add copy description in runtime fields list

7d67daa

Validate that duplicate runtime fields are not allowed + callout when…

36e3bb9

… shadowing

Fix i18n issue

fcb1e0e

Add Api integration tests for the ES error parser

ed4e84d

sebelga force-pushed the runtime-fields-in-mappings-2 branch from 7eb5c08 to ed4e84d Compare November 26, 2020 09:17

sebelga added 5 commits November 26, 2020 10:38

Fix issue validating duplicates when editing a field

800d568

Add missing aria-label to form

8919c37

Update README.md

0148830

Update loadEditor handler to accept ctx object

4f597c3

Add es_error_parser unit test

b25fc70

lockewritesdocs self-requested a review November 30, 2020 13:51

Update copy text

8a65bc8

cjcenizal added Feature:Index Management Index and index templates UI Feature:Mappings Editor Index mappings editor UI labels Nov 30, 2020

Merge branch 'master' into runtime-fields-in-mappings-2

5f1fb37

alisonelizabeth mentioned this pull request Dec 2, 2020

[ES UI] Audit plugins using <SectionError/> component #84801

Closed

lockewritesdocs reviewed Dec 2, 2020

View reviewed changes

sebelga and others added 6 commits December 3, 2020 10:03

Update x-pack/plugins/index_management/public/application/components/…

59a01db

…mappings_editor/__jest__/client_integration/runtime_fields.test.tsx Co-authored-by: Adam Locke <adam.locke@elastic.co>

Apply suggestions from code review

8fe6dd4

Co-authored-by: Adam Locke <adam.locke@elastic.co>

Change callout color to warning for shadowing field

c1fc24a

Update link for runtime fields painless syntax

446c2cf

Mark script form field as optional

b3a6cd2

Merge remote-tracking branch 'upstream/master' into runtime-fields-in…

4fa275a

…-mappings-2

sebelga added 2 commits December 3, 2020 12:42

Fix tests

89b6dbf

Fix linting issue

ea928f6

lockewritesdocs self-requested a review December 3, 2020 14:24

lockewritesdocs approved these changes Dec 3, 2020

View reviewed changes

sebelga merged commit e83bbfd into elastic:master Dec 3, 2020

sebelga deleted the runtime-fields-in-mappings-2 branch December 3, 2020 14:59

sebelga mentioned this pull request Dec 3, 2020

[7.x] [Runtime fields] Add support in index template (#84184) #84928

Merged

sebelga added a commit that referenced this pull request Dec 4, 2020

[7.x] [Runtime fields] Add support in index template (#84184) (#84928)

a0be161

This was referenced Dec 4, 2020

Update runtime fields URL #78609

Closed

Add help text for runtime fields source. #85204

Merged

Validate runtime fields sans scripts #85205

Closed

lockewritesdocs added v7.11.0 and removed v7.1.1 labels Dec 8, 2020

Conversation

sebelga commented Nov 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes for code reviewers

How to test

Shadowing mapped field

Prevent duplicated runtime field

Handling ES error

Tests

Copy review

Release note

Uh oh!

sebelga commented Nov 30, 2020

Uh oh!

sebelga commented Dec 2, 2020

Uh oh!

lockewritesdocs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lockewritesdocs Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

sebelga Dec 3, 2020

Choose a reason for hiding this comment

Uh oh!

sebelga Dec 3, 2020

Choose a reason for hiding this comment

Uh oh!

javanna Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

lockewritesdocs Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

lockewritesdocs Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

sebelga Dec 3, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sebelga commented Dec 3, 2020

Uh oh!

lockewritesdocs left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Dec 3, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

Page load bundle

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sebelga commented Nov 24, 2020 •

edited

Loading