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

Revert "Introduce SkipAddHostName setting. (#10728)" #10769

Merged
merged 2 commits into from
Mar 8, 2019

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Feb 15, 2019

This reverts commit 72df5e5.

@urso figured out that adding this SkipAddHostName flag changes the order in which processors are applied and also changes that the host name now gets added to beats that are using the pipeline.New method (e.g. xpack monitoring).

Thus this commit will be reverted and another solution will be implemented that allows to not set the host.name for every beat.

@simitt simitt requested a review from urso February 15, 2019 13:47
@simitt simitt requested a review from a team as a code owner February 15, 2019 13:47
@ph ph changed the title Revert "Intoduce SkipAddHostName setting. (#10728)" Revert "Introduce SkipAddHostName setting. (#10728)" Feb 15, 2019
@graphaelli
Copy link
Member

graphaelli commented Feb 15, 2019

I don't follow the reasoning here. Adding fields that aren't in the monitoring index template shouldn't affect anything other than adding unnecessary fields to the monitoring payload, as the template already includes dynamic: false. Couldn't we fix that issue any time later and leave this bug fix in as is until then?

@simitt
Copy link
Contributor Author

simitt commented Feb 21, 2019

@urso do you have any update on this?

@urso
Copy link

urso commented Feb 21, 2019

Follow up PR is in progress: #10801

I have it based on your revert. The idea was to revert here and so we only have to backport #10801.

@simitt
Copy link
Contributor Author

simitt commented Feb 21, 2019

ok, please go ahead an merge this revert then any time (I don't seem to have permissions without an approval).

@urso urso merged commit 225f484 into elastic:master Mar 8, 2019
fearful-symmetry pushed a commit to fearful-symmetry/beats that referenced this pull request Mar 8, 2019
…10769)

This reverts commit 72df5e5.

@urso figured out that adding this `SkipAddHostName` flag changes the order in which processors are applied and also changes that the host name now gets added to beats that are using the `pipeline.New` method (e.g. xpack monitoring). 

Thus this commit will be reverted and another solution will be implemented that allows to not set the `host.name` for every beat.
@simitt simitt deleted the revert-adding-skipaddhostname branch May 6, 2019 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants