-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce add_labels and add_tags processors #9973
Conversation
193c6fc
to
1b9de01
Compare
} | ||
|
||
for name, test := range cases { | ||
test := test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great example for me. Thanks! Just curious, is this test := test
necessary here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends. I tend to put it here so to bind the name locally in case I enable t.Parallel()
. functions/go-routines in a for-block accessing a loop variable will see the value being changed otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handy tricks, I should use it more often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see these 2 new processor. Left some feedback related to ECS.
If we remove the feature in 7.0, would be perhaps provide some migration code to modify the config to use the processors instead? Not sure how comment the usage of these 2 is, but I assume not rare.
Note: I will add documentation later. There is some more cleanup/renaming of other processors I want to do as well first. |
d1ded38
to
acc6372
Compare
} | ||
|
||
for name, test := range cases { | ||
test := test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handy tricks, I should use it more often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the following config work?
processors:
add_labels:
labels:
foo.bar: 17
I'm mainly concerned about the .
in the key. If not, is that something we want to support? Would be nice to have some system tests to confirm this works or not.
What do you mean by work. It works the same current
The way we read config files one can only choose between dedot or flat dottet output always. The information whether a dot was used in the config or not is lossed thanks to the config file parser. |
Introduce add_fields and add_tags processors. The fields/tags settings will be marked deprecated starting with 6.7.
Replace custom processors in pipeline/processors.go with new definitions.
Cleanup pipeline/processors.go by removing dead code.
Fix recent regressions from
beat
->agent
renaming potentially triggering a panic due to concurrent map updates.