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.

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

Add host.name in the events #7051

Merged
merged 9 commits into from
May 10, 2018
Merged

Conversation

tsg
Copy link
Contributor

@tsg tsg commented May 9, 2018

As a solution for #7050, we're adding a host.name field to
all events. This is duplicate information from beat.name,
but is used to avoid the mapping conflict and to slowly
introduce the "host as an object" approach.

To remove the duplication, you can remove beat.name like this:

processors:
  - drop_fields.fields: ["beat.name"]

Closes #7050.

@tsg tsg added bug needs_backport PR is waiting to be backported to other branches. review in progress Pull request is currently in progress. labels May 9, 2018
@tsg tsg requested review from urso and ruflin May 9, 2018 10:58
@@ -93,6 +94,7 @@ type Settings struct {
type Annotations struct {
Beat common.MapStr
Event common.EventMetadata
Host common.MapStr
Copy link

Choose a reason for hiding this comment

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

Hmmm.... do we consider to introduce more namespaces? If so, how about changing Annotations to:

type Annotations struct {
  Event common.EventMetadata // additional event fields configured by user
  Builtin common.MapStr
}

And then init Annotations with:

Annotations{
  Event: config.EventMetaData,
  Builtin: common.MapStr{
    "beat": ...
    "host": ...
  },
}

This way the logic how/when the builtin namespaces are merged needs to be done only once.

if meta := global.beatsMeta; len(meta) > 0 {
processors.add(makeAddFieldsProcessor("beatsMeta", meta, needsCopy))
}
if meta := global.hostMeta; len(meta) > 0 {
processors.add(makeAddFieldsProcessor("hostMeta", meta, needsCopy))
Copy link

Choose a reason for hiding this comment

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

This is safe, as needsCopy is true if any global processors are configured.

We can also 'reduce' some copies by introducing needsGlobalCopy := global.alwaysCopy || global.processors != nil and needsCopy := needsGlobalCopy || local.processors != nil.

@andrewkroh
Copy link
Member

andrewkroh commented May 9, 2018

This new host.name field also needs to be added to fields.yml. (nm - It should already be there from add_host_metadata.)

IIUC this change and the add_host_metadata processor could cause mapping conflicts for anyone using LS in their pipeline today. In particular it will cause issues for anyone not using versioned index names (e.g. %{[@metadata][beat]}-%{[@metadata][version]}-%{+YYYY.MM.dd}).

And it could causes issues with any LS config that expects [host] to be a string (e.g. LS conditionals). So I think we might want to put something in the breaking changes section about this.

tsg added 5 commits May 10, 2018 00:37
As a solution for elastic#7050, we adding a `host.name` field to
all events. This is duplicate information from `beat.name`,
but is used to avoid the mapping conflict and to slowly
introduce the "host as an object" approach.

To remove the duplication, you can remove `beat.name` like this:

    processors:
      - drop_fields.fields: ["beat.name"]

Closes elastic#7050.
@tsg tsg force-pushed the add_host_name_by_default branch from ba9fb18 to 6be41ce Compare May 9, 2018 22:38
@tsg tsg removed the in progress Pull request is currently in progress. label May 9, 2018
@tsg
Copy link
Contributor Author

tsg commented May 9, 2018

@urso @ruflin Can you have a second look? Also, @andrewkroh has a point above, this is kind of a breaking change. I hope we're not missing other cases like this. cc @jsvd as well.

@tsg
Copy link
Contributor Author

tsg commented May 10, 2018

Jenkins, test this again

@@ -36,13 +36,14 @@ exec { go get -u github.com/jstemmer/go-junit-report }
echo "Building $env:beat"
exec { go build } "Build FAILURE"

# always build the libbeat fields
cp ..\libbeat\_meta\fields.common.yml ..\libbeat\_meta\fields.generated.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

As soon as #6911 is in this should become much simpler.

"name": name,
"hostname": beatInfo.Hostname,
"version": beatInfo.Version,
Builtin: common.MapStr{
Copy link
Contributor

Choose a reason for hiding this comment

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

I like that change because it prepares us for more fields coming from ECS.

@ruflin ruflin merged commit 15d9539 into elastic:master May 10, 2018
tsg added a commit to tsg/beats that referenced this pull request May 10, 2018
As a solution for elastic#7050, we're adding a `host.name` field to
all events. This is duplicate information from `beat.name`,
but is used to avoid the mapping conflict and to slowly
introduce the "host as an object" approach.

To remove the duplication, you can remove `beat.name` like this:

    processors:
      - drop_fields.fields: ["beat.name"]

Closes elastic#7050.

(cherry picked from commit 15d9539)
@tsg tsg added v6.3.0 and removed needs_backport PR is waiting to be backported to other branches. labels May 10, 2018
monicasarbu pushed a commit that referenced this pull request May 10, 2018
* Add host.name in the events (#7051)

As a solution for #7050, we're adding a `host.name` field to
all events. This is duplicate information from `beat.name`,
but is used to avoid the mapping conflict and to slowly
introduce the "host as an object" approach.

To remove the duplication, you can remove `beat.name` like this:

    processors:
      - drop_fields.fields: ["beat.name"]

Closes #7050.

(cherry picked from commit 15d9539)

* changelog cleanup
stevea78 pushed a commit to stevea78/beats that referenced this pull request May 20, 2018
As a solution for elastic#7050, we're adding a `host.name` field to
all events. This is duplicate information from `beat.name`,
but is used to avoid the mapping conflict and to slowly
introduce the "host as an object" approach.

To remove the duplication, you can remove `beat.name` like this:

    processors:
      - drop_fields.fields: ["beat.name"]

Closes elastic#7050.
stevea78 pushed a commit to stevea78/beats that referenced this pull request May 20, 2018
As a solution for elastic#7050, we're adding a `host.name` field to
all events. This is duplicate information from `beat.name`,
but is used to avoid the mapping conflict and to slowly
introduce the "host as an object" approach.

To remove the duplication, you can remove `beat.name` like this:

    processors:
      - drop_fields.fields: ["beat.name"]

Closes elastic#7050.
@orarnon
Copy link

orarnon commented Aug 1, 2018

hi @tsg , We still have issues with Filebeat 6.3.1:

[2018-08-01T09:19:04,028][WARN ][logstash.outputs.elasticsearch] Could not index event to Elasticsearch. {:status=>400, :action=>["index", {:_id=>nil, :_index=>"tag-mediation-2018.08.01", :_type=>"logs", :_routing=>nil}, 2018-08-01T09:19:03.185Z {"name":"tagmediation-i-0c6dadf01c99ec637.eu-west-1.production"} %{message}], :response=>{"index"=>{"_index"=>"tag-mediation-2018.08.01", "_type"=>"logs", "_id"=>"AWT0yJcsvdiggc-2mDWo", "status"=>400, "error"=>{"type"=>"mapper_parsing_exception", "reason"=>"failed to parse [host]", "caused_by"=>{"type"=>"illegal_state_exception", "reason"=>"Can't get text on a START_OBJECT at 1:656"}}}}}

@urso
Copy link

urso commented Aug 1, 2018

@orarnon This error message looks very unrelated. Please use https://discuss.elastic.co to ask for some help.

leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…#7068)

* Add host.name in the events (elastic#7051)

As a solution for elastic#7050, we're adding a `host.name` field to
all events. This is duplicate information from `beat.name`,
but is used to avoid the mapping conflict and to slowly
introduce the "host as an object" approach.

To remove the duplication, you can remove `beat.name` like this:

    processors:
      - drop_fields.fields: ["beat.name"]

Closes elastic#7050.

(cherry picked from commit 1889f9c)

* changelog cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants