Skip to content
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.

Sign up for GitHub

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

move agent metadata to a processor #9952

Merged
merged 5 commits into from
Jan 9, 2019
Merged

move agent metadata to a processor #9952

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
733346a
move agent metadata to a processor
graphaelli Jan 8, 2019
1ba4454
fix tests
graphaelli Jan 8, 2019
6f8ec6a
retain previous agent metadata merge behavior
graphaelli Jan 8, 2019
e6f631e
update dev changelog
graphaelli Jan 9, 2019
4d44f83
Merge branch 'master' into agent-meta-processor
graphaelli Jan 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions libbeat/beat/pipeline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

Was not even aware we have these options 👍

Copy link

Choose a reason for hiding this comment

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

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...).

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.


// ACK handler strategies.
// Note: ack handlers are run in another go-routine owned by the publisher pipeline.
// They should not block for to long, to not block the internal buffers for
Expand Down
11 changes: 0 additions & 11 deletions libbeat/publisher/pipeline/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,6 @@ func Load(
Annotations: Annotations{
Event: config.EventMetadata,
Builtin: common.MapStr{
"agent": common.MapStr{
"type": beatInfo.Beat,
"hostname": beatInfo.Hostname,
"version": beatInfo.Version,
"id": beatInfo.ID.String(),
"ephemeral_id": beatInfo.EphemeralID.String(),
},
"host": common.MapStr{
"name": name,
},
Expand All @@ -97,10 +90,6 @@ func Load(
},
}

if name != beatInfo.Hostname {
settings.Annotations.Builtin.Put("agent.name", name)
}

queueBuilder, err := createQueueBuilder(config.Queue, monitors)
if err != nil {
return nil, err
Expand Down
24 changes: 23 additions & 1 deletion libbeat/publisher/pipeline/processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,12 @@ func newProcessorPipeline(
processors.add(makeAddFieldsProcessor("beatsMeta", meta, needsCopy))
}

// setup 7: pipeline processors list
// setup 7: add agent metadata
if !config.SkipAgentMetadata {
processors.add(makeAddAgentMetadataProcessor(info))
}

// setup 8: pipeline processors list
processors.add(global.processors)

// setup 9: debug print final event (P)
Expand Down Expand Up @@ -290,6 +295,23 @@ func makeAddDynMetaProcessor(
})
}

func makeAddAgentMetadataProcessor(info beat.Info) *processorFn {
metadata := common.MapStr{
"type": info.Beat,
"ephemeral_id": info.EphemeralID.String(),
"hostname": info.Hostname,
"id": info.ID.String(),
"version": info.Version,
}
if info.Name != info.Hostname {
metadata.Put("name", info.Name)
}
return newProcessor("add_agent_metadata", func(event *beat.Event) (*beat.Event, error) {
_, err := event.Fields.Put("agent", metadata)
Copy link

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch on both, will make those changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

return event, err
})
}

func debugPrintProcessor(info beat.Info) *processorFn {
// ensure only one go-routine is using the encoder (in case
// beat.Client is shared between multiple go-routines by accident)
Expand Down