Skip to content

[Ingest Pipelines] add "keep" option to remove processor#225638

Merged
damian-polewski merged 8 commits intoelastic:mainfrom
damian-polewski:ingest_pipelines/add_keep_option_to_remove_processor
Jul 7, 2025
Merged

[Ingest Pipelines] add "keep" option to remove processor#225638
damian-polewski merged 8 commits intoelastic:mainfrom
damian-polewski:ingest_pipelines/add_keep_option_to_remove_processor

Conversation

@damian-polewski
Copy link
Contributor

@damian-polewski damian-polewski commented Jun 27, 2025 •
edited
Loading

Adds #223117

Summary

This PR introduces support for keep option in the Remove processor.

Updated field

New toggle has been added that allows user to switch from using field to keep option and change the processor behavior.

field keep
Screenshot 2025-06-27 at 15 55 01 Screenshot 2025-06-27 at 15 55 13

Updated description

Description of the processor has been adjusted to reflect used option.

Screenshot 2025-06-27 at 15 56 29

Demo

Screen.Recording.2025-06-27.at.16.02.05.mov

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • 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
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
  • Flaky Test Runner was used on any tests changed
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
  • Review the backport guidelines and apply applicable backport:* labels.

Release note

The remove processor in the ingest pipelines now supports "keep" option.
@damian-polewski damian-polewski self-assigned this Jun 27, 2025
@damian-polewski damian-polewski added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// Feature:Ingest Node Pipelines Ingest node pipelines management backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Jun 27, 2025
@damian-polewski damian-polewski marked this pull request as ready for review June 27, 2025 14:23
@damian-polewski damian-polewski requested a review from a team as a code owner June 27, 2025 14:23
@elasticmachine
Copy link
Contributor

elasticmachine commented Jun 27, 2025

Pinging @elastic/kibana-management (Team:Kibana Management)
sabarasaba
sabarasaba reviewed Jun 30, 2025
View reviewed changes
Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

Tested locally with test documents and UI works well! I left few comments, let me know what you think
florent-leborgne
florent-leborgne reviewed Jul 1, 2025
View reviewed changes
Copy link
Contributor

@florent-leborgne florent-leborgne left a comment

Choose a reason for hiding this comment

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

Hey @damian-polewski. Leaving a few suggestions that hopefully make sense. In general:

  • Try to use active voice whenever possible
  • Do not add help text unless there is something specific to say or hint.

Another comment that's not purely text related is the way to switch between remove and keep modes. I don't know if it's very clear. Perhaps that's an established pattern in processor forms like this? If it is, then please disregard this comment:
My own recommendation would be to have radio buttons before the field, where:

  • the first and default option selected is "Define fields to remove"
  • the second is "Define fields to keep".
    Or see my comments inline to try to make this new option clearer with just wording tweaks.

Last and connected to this: The docs page indicates that "fields" is required but it seems that it's rather fields OR keep that is required, looking at the exemples.
This might be good to note in the docs.
image
@damian-polewski
Copy link
Contributor Author

damian-polewski commented Jul 1, 2025

@sabarasaba @florent-leborgne Thank you for your feedback! I made changes according to your suggestions!

@florent-leborgne regarding your other questions:

  1. This is already used pattern that we already use in Network Directions processor.
  2. I think it would be good to updated the documentation - the ES doesn't support having both field and keep in same remove processor so it's a bit confusing.
florent-leborgne
florent-leborgne reviewed Jul 1, 2025
View reviewed changes
...ublic/application/components/pipeline_editor/components/processor_form/processors/remove.tsx
helpText: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.removeForm.fieldNameHelpText',
{
defaultMessage: 'You can use plain text or template snippets.',
Copy link
Contributor

@florent-leborgne florent-leborgne Jul 1, 2025

Choose a reason for hiding this comment

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

If we're not going to show an example of what a template snippet looks like, we could link to the docs
florent-leborgne
florent-leborgne approved these changes Jul 1, 2025
View reviewed changes
Copy link
Contributor

@florent-leborgne florent-leborgne left a comment

Choose a reason for hiding this comment

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

Updated copy LGTM, thanks!
sabarasaba
sabarasaba approved these changes Jul 7, 2025
View reviewed changes
Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

latest changes lgtm
@elasticmachine
Copy link
Contributor

elasticmachine commented Jul 7, 2025

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
ingestPipelines 359.7KB 361.6KB +1.9KB

History

cc @damian-polewski
@damian-polewski damian-polewski merged commit 26adf63 into elastic:main Jul 7, 2025
10 checks passed
@damian-polewski damian-polewski deleted the ingest_pipelines/add_keep_option_to_remove_processor branch July 7, 2025 08:38
@damian-polewski damian-polewski linked an issue Jul 7, 2025 that may be closed by this pull request
Include "keep" option for the remove processor in the ingest pipeline UI #223117
Closed
kertal pushed a commit to kertal/kibana that referenced this pull request Jul 25, 2025
@damian-polewski @kertal
[Ingest Pipelines] add "keep" option to remove processor (elastic#225638
c6bf952 
)

Adds elastic#223117

## Summary
This PR introduces support for `keep` option in the [Remove
processor](https://www.elastic.co/docs/reference/enrich-processor/remove-processor).
@florent-leborgne florent-leborgne mentioned this pull request Sep 22, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Ingest Node Pipelines Ingest node pipelines management needs_docs release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v9.2.0

5 participants

@damian-polewski @elasticmachine @sabarasaba @florent-leborgne @kibanamachine