[Ingest Pipelines] Processor forms for processors A-D #72849

jloleysens · 2020-07-22T13:46:39Z

Summary

Added forms for the following processors:

Append Processor
Bytes Processor
Circle Processor
Convert Processor
CSV Processor
Date Processor
Date Index Name Processor
Dissect Processor
Dot Expander Processor
Drop Processor

How to test

Start Kibana with basic license
Go to ingest pipelines plugin in management section
Add a processor for each of the forms listed above (attempt to submit to see which fields are required)
Create the pipeline

Notes

Have not spent time on optimising the layout of the forms. We can definitely discuss this here
Copy review is still needed
Trying to devise a good form of test coverage for these forms
I intend to add processor forms in batches of ~10 so that each form gets proper review and testing

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

- Also refactored options to live in scoped objects to avoid overriding type (important fix!) - Have not polished copy or form layout.

- date_index_name - dissect - dot_expander - drop Fields refactored: - Field - Ignore missing

elasticmachine · 2020-07-22T13:46:41Z

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

jloleysens · 2020-07-22T13:46:51Z

@elasticmachine merge upstream

...mponents/pipeline_processors_editor/components/processor_settings_form/processors/circle.tsx

...ponents/pipeline_processors_editor/components/processor_settings_form/processors/convert.tsx

jloleysens · 2020-07-23T10:26:43Z

@elasticmachine merge upstream

alisonelizabeth

This is looking great @jloleysens! I did a light code review and verified all the processor types against the docs. I have not yet thoroughly tested adding the processor types and then creating/saving the pipeline. I also think it'd be great to get some of the ES engineers to review these forms for a sanity check.

Bugs

There were a few processor types where the field field wasn't marked as required: csv, date_index_name, and dissect.
? If I select the append processor type (for example), fill out the required fields and update, then go back to edit it and change processor types, any fields that were common retained the value. I think I would expect the fields to be cleared in this scenario.

UX feedback

It'd be great if we could make the "Condition" field more usable, similarly with "Pattern" (possibly others). I think both of these could potentially be fairly long, which could make the text field hard to work with.
I'm not used to seeing radio groups 😄 . This may be a personal preference, but what do you think about using a dropdown for the shape types instead (circle processor)?
I think we could borrow some of the UX in the mappings editor, where we use the toggle for optional fields. Also, I think it's really helpful to have help text next to each field as well. The doc link may not be applicable.

Example:

I noticed some of the optional fields have "(optional)" in the field name, and some do not. This point may not be applicable though if we follow the mappings editor UX.
How difficult would it be to use a human-readable name for the processor type in the dropdown? For example, "date index name" instead of "date_index_name".

Other

I noticed the ability to add an on_failure processor at the processor level is missing. Are you planning to add support for this?

alisonelizabeth · 2020-07-27T16:11:09Z

...ponents/pipeline_processors_editor/components/processor_settings_form/processors/convert.tsx

+      <UseField
+        componentProps={{
+          euiFieldProps: {
+            options: [


Per the docs, long is also supported here. https://www.elastic.co/guide/en/elasticsearch/reference/master/convert-processor.html

jloleysens · 2020-08-06T10:32:56Z

@elasticmachine merge upstream

jloleysens · 2020-08-10T11:12:27Z

@elasticmachine merge upstream

…nes/processor-forms-a-d * 'master' of github.com:elastic/kibana: (26 commits) [Telemetry][API Integration] size_in_bytes to be a number (elastic#74664) [ILM] Convert node details flyout to TS (elastic#73707) [Ingest Node Pipelines] Sentence-case processor names (elastic#74645) Bump angular dependency from 1.7.9 to 1.8.0 (elastic#74482) [ML] Fixing schema for custom rule conditions (elastic#74676) [ML] Refactor in preparation for new es client (elastic#74552) [ML] Adding initial file analysis overrides (elastic#74376) Allow any hostname for chromium proxy bypass (elastic#74693) [ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page (elastic#74533) [Metrics UI] Fix No Data preview pluralization (elastic#74399) [Bug][Security_Solution][Telemetry] Capitalize S in macOS (elastic#74688) Remove karma tests from legacy maps (elastic#74668) [Ingest Manager] stop creating events-* index pattern and placeholder index (elastic#74683) [Enterprise Search] Update the browser/document title on plugin navigation (elastic#74392) [visualizations] Add i18n translation for 'No results found' (elastic#74619) [maps] convert vector style properties to TS (elastic#74553) bump geckodriver binary to 0.27 (elastic#74638) fix: update apm agents to catch abort requests (elastic#74658) [Security Solution] Resolver children pagination (elastic#74603) add memoryStatus to df analytics page and analytics table in management (elastic#74570) ...

- Added help text for all fields - Updated layout so that required fields are always on first - Replaced circle radio group with a select drop down

alisonelizabeth · 2020-08-13T02:42:26Z

I tend to agree that it would be better if we cleared the form. (What if we switch to a form with the same name but the field type is different? I can't see a field that would trigger this at the moment, but it is technically possible).

IMO it would be the best if we provide the user with a fresh set of fields whenever they change type, and as a bonus if they go back to the type they were on it would be nice if those fields get repopulated if they are editing an existing processor.

👍 I will open up a separate issue to address this.

I have addressed all of these points in the latest commits except for adopting the toggle for optional fields of the mappings editor. After looking at the existing forms I thought the added toggle created a bit too much overhead for what is mostly very small forms.

One thing I would like to experiment with is putting the "common" fields (if, tag and ignore_failure) into their own section, possibly behind a toggle. WDYT @alisonelizabeth and @mdefazio ?

I'll defer to Michael, but I think that makes sense and would be interested in seeing how that would look.

Currently the only way to add an on-failure processor is through the more menu on the processor itself. I think this is sufficient for now as we may need a bit more thinking around how we would want to introduce that concept into a form.

👍

alisonelizabeth

Latest changes LGTM. Nice job!

I had two suggestions, but not blocking:

The append processor accepts an array for the value field. It might make more sense to use the combo box here instead of a text input.
It might be nice to add some client validation for the dot_expander processor, to ensure the field contains at least one . character.

Another thought - I don't want to speak for the docs team, but I wonder if it would be easier for them to review the copy as part of this PR, rather than all of the forms at once.

jloleysens · 2020-08-13T08:16:31Z

@esdocs

Would it be possible to get review on new copy added in this PR? Most of the new copy added is in x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors.

jloleysens · 2020-08-13T08:40:53Z

@alisonelizabeth Excellent points I'll address both of them (append and dot expander) and reach out to es-docs for review.

jloleysens · 2020-08-13T08:44:59Z

@elasticmachine merge upstream

kibanamachine · 2020-08-13T11:01:24Z

💚 Build Succeeded

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

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
ingestPipelines	390	+15	375

async chunks size

id	value	diff	baseline
ingestPipelines	643.9KB	+33.9KB	610.0KB

page load bundle size

id	value	diff	baseline
ingestPipelines	32.7KB	+489.0B	32.2KB

History

💔 Build #67872 failed 023543b
💚 Build #67698 succeeded 5ff03ea
💔 Build #67671 failed 106701f
💔 Build #67669 failed bbbe9d2
💔 Build #67461 failed 97eea65

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

* First few processors of the first batch - Also refactored options to live in scoped objects to avoid overriding type (important fix!) - Have not polished copy or form layout. * add type to shared imports * Refactors for repeated fields and added forms - date_index_name - dissect - dot_expander - drop Fields refactored: - Field - Ignore missing * Fix broken imports and some other small refactors * added text editor field and updated pattern and if fields * Large copy improvements and updates and other small refactors - Added help text for all fields - Updated layout so that required fields are always on first - Replaced circle radio group with a select drop down * update circle shape type field to select * Added "long" option for convert type * fix path import * fix types and i18n * add validation for dot expander fix append value to be a combobox * fix i18n Co-authored-by: Elastic Machine <[email protected]>

sebelga · 2020-08-13T13:05:19Z

Currently still investigating a good solution. I've also reached out to @sebelga and I think this should be addressed as a separate PR as we probably won't need to do this work at the individual form level.

I created #74950 to track this enhancement @jloleysens @alisonelizabeth

* upstream/master: (45 commits) [Metrics UI] Fix inventory footer misalignment (elastic#74707) Remove legacy optimizer (elastic#73154) Update design-specific GH code-owners (elastic#74877) skip test Reporting paginates content elastic#74922 [Metrics UI] Add Jest tests for alert previews (elastic#74890) Fixed tooltip (elastic#74074) [Ingest Pipelines] Processor forms for processors A-D (elastic#72849) [Observability] change ingest manager link (elastic#74928) [Task manager] Prevents edge case where already running tasks are reschedule every polling interval (elastic#74606) [Security Solution] Fix the status of timelines' bulk actions (elastic#74560) Data plugin: Suggested enhance pattern (elastic#74505) Use jest.useFakeTimers instead of hard coded timeout for tooltip tests. (elastic#74642) [Security Solution][lists] Adds tests for exception lists and items part 2 (elastic#74815) [Security Solution][Resolver] fix presentation role on edgeline (elastic#74869) [Security Solution][Detections] Refactor ML calls for newest ML permissions (elastic#74582) [bin/kibana-plugin] support KP plugins instead (elastic#74604) Reduce number of indexed fields in index pattern saved object (elastic#74817) [reporting] Pass along generic parameters in high-order route handler (elastic#74892) Migrated last pieces of legacy fixture code (elastic#74470) Empty index patterns page re-design (elastic#68819) ...

* First few processors of the first batch - Also refactored options to live in scoped objects to avoid overriding type (important fix!) - Have not polished copy or form layout. * add type to shared imports * Refactors for repeated fields and added forms - date_index_name - dissect - dot_expander - drop Fields refactored: - Field - Ignore missing * Fix broken imports and some other small refactors * added text editor field and updated pattern and if fields * Large copy improvements and updates and other small refactors - Added help text for all fields - Updated layout so that required fields are always on first - Replaced circle radio group with a select drop down * update circle shape type field to select * Added "long" option for convert type * fix path import * fix types and i18n * add validation for dot expander fix append value to be a combobox * fix i18n Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

jrodewig · 2020-08-19T20:04:10Z

Hey @jloleysens -- @lockewritesdocs agreed to look at the UI copy for this one. I'll handle #75054. Thanks for bringing this to our attention!

jloleysens added 3 commits July 20, 2020 18:07

First few processors of the first batch

e120f03

- Also refactored options to live in scoped objects to avoid overriding type (important fix!) - Have not polished copy or form layout.

add type to shared imports

71274a3

Refactors for repeated fields and added forms

1621eb8

- date_index_name - dissect - dot_expander - drop Fields refactored: - Field - Ignore missing

jloleysens added v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.10.0 Feature:Ingest Node Pipelines Ingest node pipelines management labels Jul 22, 2020

jloleysens requested review from sebelga and alisonelizabeth July 22, 2020 13:46

jloleysens requested a review from a team as a code owner July 22, 2020 13:46

Merge branch 'master' into ingest-pipelines/processor-forms-a-d

9691f9e

sebelga reviewed Jul 22, 2020

View reviewed changes

...mponents/pipeline_processors_editor/components/processor_settings_form/processors/circle.tsx Outdated Show resolved Hide resolved

sebelga reviewed Jul 22, 2020

View reviewed changes

...ponents/pipeline_processors_editor/components/processor_settings_form/processors/convert.tsx Outdated Show resolved Hide resolved

Merge branch 'master' into ingest-pipelines/processor-forms-a-d

0dad23e

alisonelizabeth reviewed Jul 27, 2020

View reviewed changes

Merge branch 'master' into ingest-pipelines/processor-forms-a-d

9952f9b

Merge branch 'master' into ingest-pipelines/processor-forms-a-d

1a52d64

jloleysens mentioned this pull request Aug 10, 2020

[ES UI Shared] [Form hook] Fields with same path get cleaned up on re-mounts of form #74677

Closed

jloleysens added 6 commits August 11, 2020 12:39

Fix broken imports and some other small refactors

8160f85

added text editor field and updated pattern and if fields

0bcd67a

Large copy improvements and updates and other small refactors

1555486

- Added help text for all fields - Updated layout so that required fields are always on first - Replaced circle radio group with a select drop down

update circle shape type field to select

7059261

Added "long" option for convert type

a529e38

jloleysens added 2 commits August 12, 2020 13:10

fix path import

106701f

fix types and i18n

5ff03ea

alisonelizabeth approved these changes Aug 13, 2020

View reviewed changes

add validation for dot expander fix append value to be a combobox

acc54d7

Merge branch 'master' into ingest-pipelines/processor-forms-a-d

023543b

jloleysens closed this Aug 13, 2020

jloleysens reopened this Aug 13, 2020

fix i18n

a6a0e1b

jloleysens merged commit 50f3328 into elastic:master Aug 13, 2020

jloleysens deleted the ingest-pipelines/processor-forms-a-d branch August 13, 2020 12:57

jloleysens mentioned this pull request Aug 13, 2020

[7.x] [Ingest Pipelines] Processor forms for processors A-D (#72849) #74949

Merged

jloleysens mentioned this pull request Aug 14, 2020

[Ingest Pipelines] Processor forms for processors E-J #75046

Closed

1 task

jloleysens mentioned this pull request Aug 14, 2020

[Ingest Pipelines] Processor forms for processors E-J #75054

Merged

1 task

sebelga mentioned this pull request Aug 18, 2020

[Form lib] Allow new "defaultValue" to be provided when resetting the… #75302

Merged

jrodewig requested review from jrodewig and lockewritesdocs and removed request for jrodewig August 19, 2020 17:13

jloleysens mentioned this pull request Aug 21, 2020

[Ingest Pipelines] Processor forms for processors K-S #75638

Merged

4 tasks

lockewritesdocs mentioned this pull request Aug 26, 2020

[Ingest Pipelines] Add descriptions for ingest processors A-D #75975

Merged

4 tasks

alisonelizabeth mentioned this pull request Sep 4, 2020

[Ingest pipelines] Forms for processors T-U #76710

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest Pipelines] Processor forms for processors A-D #72849

[Ingest Pipelines] Processor forms for processors A-D #72849

jloleysens commented Jul 22, 2020

elasticmachine commented Jul 22, 2020

jloleysens commented Jul 22, 2020

jloleysens commented Jul 23, 2020

alisonelizabeth left a comment

alisonelizabeth Jul 27, 2020

jloleysens commented Aug 6, 2020

jloleysens commented Aug 10, 2020

alisonelizabeth commented Aug 13, 2020

alisonelizabeth left a comment

jloleysens commented Aug 13, 2020

jloleysens commented Aug 13, 2020

jloleysens commented Aug 13, 2020

kibanamachine commented Aug 13, 2020

sebelga commented Aug 13, 2020

jrodewig commented Aug 19, 2020

[Ingest Pipelines] Processor forms for processors A-D #72849

[Ingest Pipelines] Processor forms for processors A-D #72849

Conversation

jloleysens commented Jul 22, 2020

Summary

How to test

Notes

Checklist

For maintainers

elasticmachine commented Jul 22, 2020

jloleysens commented Jul 22, 2020

jloleysens commented Jul 23, 2020

alisonelizabeth left a comment

Choose a reason for hiding this comment

Bugs

UX feedback

Other

alisonelizabeth Jul 27, 2020

Choose a reason for hiding this comment

jloleysens commented Aug 6, 2020

jloleysens commented Aug 10, 2020

alisonelizabeth commented Aug 13, 2020

alisonelizabeth left a comment

Choose a reason for hiding this comment

jloleysens commented Aug 13, 2020

jloleysens commented Aug 13, 2020

jloleysens commented Aug 13, 2020

kibanamachine commented Aug 13, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

async chunks size

page load bundle size

History

sebelga commented Aug 13, 2020

jrodewig commented Aug 19, 2020