New pipeline functions remove_single_field and remove_multiple_fields
#19268
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 tasks
bernd
reviewed
May 7, 2024
| @@ -69,7 +69,7 @@ public FunctionDescriptor<Void> descriptor() { | |||
| .params(ImmutableList.of(fieldParam, messageParam, invertParam)) | |||
| .description("Removes the named field from message, unless the field is reserved. If no specific message is provided, it removes the field from the currently processed message.") | |||
There was a problem hiding this comment.
Please add a note to the description about why the function is deprecated and the alternatives.
...main/java/org/graylog/plugins/pipelineprocessor/functions/messages/RemoveMultipleFields.java
Outdated
Show resolved
Hide resolved
...main/java/org/graylog/plugins/pipelineprocessor/functions/messages/RemoveMultipleFields.java
Outdated
Show resolved
Hide resolved
...main/java/org/graylog/plugins/pipelineprocessor/functions/messages/RemoveMultipleFields.java
Outdated
Show resolved
Hide resolved
moesterheld
approved these changes
May 8, 2024
| @@ -67,9 +70,11 @@ public FunctionDescriptor<Void> descriptor() { | |||
| .name(NAME) | |||
| .returnType(Void.class) | |||
| .params(ImmutableList.of(fieldParam, messageParam, invertParam)) | |||
| .description("Removes the named field from message, unless the field is reserved. If no specific message is provided, it removes the field from the currently processed message.") | |||
| .description("Removes the named field from message, unless the field is reserved. " + | |||
| "If no specific message is provided, it uses the currently processed message." + | |||
There was a problem hiding this comment.
Suggested change
| "If no specific message is provided, it uses the currently processed message." + | |
| "If no specific message is provided, it uses the currently processed message. " + |
Missing space
| .description("Removes the specified field(s) from message, unless the field name is reserved. If no specific message is provided, it uses the currently processed message.") | ||
| .ruleBuilderEnabled() | ||
| .ruleBuilderName("Remove field - multiple") | ||
| .ruleBuilderTitle("Remove multiple fields by regex<#if pattern??> '${pattern}'</#if> or name list<#if names??> '${names}'</#if>") |
There was a problem hiding this comment.
Suggested change
| .ruleBuilderTitle("Remove multiple fields by regex<#if pattern??> '${pattern}'</#if> or name list<#if names??> '${names}'</#if>") | |
| .ruleBuilderTitle("Remove multiple fields by<#if pattern??> regex '${pattern}'</#if><#if pattern?? && names??> or</#if><#if names??> name list '${names}'</#if>") |
There was a problem hiding this comment.
done. Could still be improved to better handle empty strings, but this is good enough!
9 tasks
dennisoelkers
pushed a commit
that referenced
this pull request
May 8, 2024
…lds` (#19268) * new pipeline functions remove_single_field and remove_multiple_fields * CL * improve rulebuilder descriptions * compile regex and other feedback changes * pre-compile pattern * UI string improvements
patrickmann
added a commit
that referenced
this pull request
Jun 5, 2024
…ultiple_fields (#19301) * New pipeline functions `remove_single_field` and `remove_multiple_fields` (#19268) * new pipeline functions remove_single_field and remove_multiple_fields * CL * improve rulebuilder descriptions * compile regex and other feedback changes * pre-compile pattern * UI string improvements (cherry picked from commit 4a1cbb4) * modify cherry-pick for things missing in 5.1 * Update changelog/unreleased/issue-19098.toml Co-authored-by: Maxwell <98284293+kodjo-anipah@users.noreply.github.com> --------- Co-authored-by: Maxwell <98284293+kodjo-anipah@users.noreply.github.com>
patrickmann
added a commit
that referenced
this pull request
Jun 5, 2024
…ultiple_fields (#19300) * New pipeline functions `remove_single_field` and `remove_multiple_fields` (#19268) * new pipeline functions remove_single_field and remove_multiple_fields * CL * improve rulebuilder descriptions * compile regex and other feedback changes * pre-compile pattern * UI string improvements (cherry picked from commit 4a1cbb4) * adjust unit test for 5.2 * Update issue-19098.toml
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #19098
Address performance and functional regression in pipeline function remove_field by introducing new, unambiguous functions.
Description
GL 5.1 added regex-matching to the pipeline function
remove_field. This breaks existing pipeline rules that callremove_fieldwith a field name containing a regex reserved character, notably.. Performance of existing rulesmay also be degraded.
Both issues are addressed by introducing more specific functions:
remove_single_fieldremoves just a single field specified by name. It is simple and fast.remove_multiple_fieldsremoves fields matching a regex pattern and/or list of names. Depending on thecomplexity of the matching it is slower.
Motivation and Context
See discussion in linked issue and #19259.
How Has This Been Tested?
org.graylog.plugins.pipelineprocessor.processors.PipelineInterpreter.executionTimeTypes of changes
Checklist: