Reorder processors in publisher pipeline #5149
Merged
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
added 3 commits
September 11, 2017 16:45
- reorder processors, such that all client processors run before the global processors - remove support for EventsMetadataKey
- combine client internal field updates and configured field updates into one dictionary - combine proessors for adding client and global fields+tags if no client processors are configured
Do not Clone (deep copy) fields being added in the pipeline if no processors are configured. As processors might add/remove fields and potentially modify shared field objects, these must be copied if there is a chance global shared structured being overwritten by processors. Especially if processors are guarded by conditions.
urso
added
libbeat
needs_backport
Problem is local fields+tags must be applied after the globaly configured fields and tests, while client processors must be run before the global ones.
ruflin
reviewed
Sep 12, 2017
| p.processors = tmp | ||
| } | ||
|
|
||
| fields := common.MapStr{} |
There was a problem hiding this comment.
I suggest to move this to line 391 as it is only used there.
There was a problem hiding this comment.
Yeah. Will simplify fields initialization.
| @@ -11,9 +11,8 @@ import ( | |||
| // Event metadata constants. These keys are used within libbeat to identify | |||
| // metadata stored in an event. | |||
| const ( | |||
| EventMetadataKey = "_event_metadata" | |||
There was a problem hiding this comment.
I assume this already has become obsolete in previous refactoring?
There was a problem hiding this comment.
The symbol still has been used in the processors pipeline. This PR removes support for _event_metadata in the pipeline -> can finally remove EventMetadataKey
kvch
reviewed
Sep 12, 2017
| func makePipelineProcessors( | ||
| annotations Annotations, | ||
| processors *processors.Processors, | ||
| disabled bool, |
There was a problem hiding this comment.
Parameter disabled is not used in this function. Please remove it or use it.
There was a problem hiding this comment.
Uh, thanks. It should be passed to pipelineProcessors.
In the end it moves the processor adding the |
urso
pushed a commit
to urso/beats
that referenced
this pull request
Sep 12, 2017
* Reorder processors pipeline order - reorder processors, such that all client processors run before the global processors - remove support for EventsMetadataKey * Combine fields and tags if possible - combine client internal field updates and configured field updates into one dictionary - combine proessors for adding client and global fields+tags if no client processors are configured * Do not copy if no processors are defined Do not Clone (deep copy) fields being added in the pipeline if no processors are configured. As processors might add/remove fields and potentially modify shared field objects, these must be copied if there is a chance global shared structured being overwritten by processors. Especially if processors are guarded by conditions. * Fix processors order once again Problem is local fields+tags must be applied after the globaly configured fields and tests, while client processors must be run before the global ones. * review - use 'disabled' flag - simplify pipeline fields init (cherry picked from commit 6082603)
urso
removed
the
needs_backport
andrewkroh
pushed a commit
that referenced
this pull request
Sep 12, 2017
* Reorder processors pipeline order - reorder processors, such that all client processors run before the global processors - remove support for EventsMetadataKey * Combine fields and tags if possible - combine client internal field updates and configured field updates into one dictionary - combine proessors for adding client and global fields+tags if no client processors are configured * Do not copy if no processors are defined Do not Clone (deep copy) fields being added in the pipeline if no processors are configured. As processors might add/remove fields and potentially modify shared field objects, these must be copied if there is a chance global shared structured being overwritten by processors. Especially if processors are guarded by conditions. * Fix processors order once again Problem is local fields+tags must be applied after the globaly configured fields and tests, while client processors must be run before the global ones. * review - use 'disabled' flag - simplify pipeline fields init (cherry picked from commit 6082603)
leweafan
pushed a commit
to leweafan/beats
that referenced
this pull request
Apr 28, 2023
* Reorder processors pipeline order - reorder processors, such that all client processors run before the global processors - remove support for EventsMetadataKey * Combine fields and tags if possible - combine client internal field updates and configured field updates into one dictionary - combine proessors for adding client and global fields+tags if no client processors are configured * Do not copy if no processors are defined Do not Clone (deep copy) fields being added in the pipeline if no processors are configured. As processors might add/remove fields and potentially modify shared field objects, these must be copied if there is a chance global shared structured being overwritten by processors. Especially if processors are guarded by conditions. * Fix processors order once again Problem is local fields+tags must be applied after the globaly configured fields and tests, while client processors must be run before the global ones. * review - use 'disabled' flag - simplify pipeline fields init (cherry picked from commit b66abfe)
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.
beatnamespace can not be removed from client processors