move agent metadata to a processor #9952
Conversation
and make it disable-able
| @@ -76,6 +76,10 @@ type ClientConfig struct { | |||
| // if the normalization step should be skipped set this to true. | |||
| SkipNormalization bool | |||
|
|
|||
| // By default events are decorated with agent metadata. | |||
| // To skip adding that metadata set this to true. | |||
| SkipAgentMetadata bool | |||
There was a problem hiding this comment.
Was not even aware we have these options 👍
There was a problem hiding this comment.
The settings 'Fields', 'Meta', 'EventMetadata', 'DynamicFields' are legacy fields we will remove in the future in favor of processors (I'm in the middle of introducing add_fields and add_tags processor).
I think I'd prefer to be able to set the agent metadata in the pipeline constructor, and have this optionally be empty in comparison to build more logic/support for the client to change/overwrite global settings. But I know it's not really possible yet to 'influence' the constructor.
By introducing SkipAgentMetadata we loose the Beats meta-data always. Is this really intended? I wonder if we want to change but the beats agent meta-data into another namespace (e.g. collector.agent...).
There was a problem hiding this comment.
By introducing SkipAgentMetadata we loose the Beats meta-data always. Is this really intended?
Yes, for APM we never want libbeat to set these fields, even if apm-server hasn't set them.
Another namespace would be fine for the APM case, as we had planned to put them under observer. In that case, we'd want these to be merged (overwrite would be fine even) as observer will have other fields too.
There was a problem hiding this comment.
Perhaps we can later put the Beat info under @metadata so apm-server could still fetch it.
I suggest for now to move forward with this as the only customer of this for now will be apm-server and we can still improve it later. On the Beats side this should not change anything.
| metadata.Put("name", info.Name) | ||
| } | ||
| return newProcessor("add_agent_metadata", func(event *beat.Event) (*beat.Event, error) { | ||
| _, err := event.Fields.Put("agent", metadata) |
There was a problem hiding this comment.
Use metadata.Clone() or pass the needsCopy bool to make sure you don't need to clone. If user adds processors adding/removing fields from the agent namespace beats will panic here or in the json encoder due to concurrent access.
Why use put instead of (Deep)Update? This is another change in semantics. With put you overwrite agent, while update (old behavior) would merge the contents if metadata if agent is already known to Fields.
There was a problem hiding this comment.
Good catch on both, will make those changes.
and address race condition per @urso
|
jenkins, retest this please |
|
I forgot, but can you please add an entry to Changelog.next.developer.asciidoc? |
and make it disable-able