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

Preserve remote host name if reading from remote journal #12714

Merged
merged 7 commits into from Jul 2, 2019

Conversation

@kvch
Copy link
Contributor

kvch commented Jun 28, 2019

When entries are coming from a remote journal, add_host_metadata overwrites the remote hostname with the local one. To preserve information on the remote server, I added a new config option save_source_hostname. Users need to set this when they are using add_host_metadata and reading from remote journals. Thus, the hostname is copied to log.source.address before the processor kicks in.

Closes #12528

@kvch

This comment has been minimized.

Copy link
Contributor Author

kvch commented Jul 1, 2019

Failing tests are unrelated.

Copy link
Contributor

faec left a comment

A couple high-level concerns:

  • "remote" is a really general flag name. If the only effect is to save the source hostname i'd prefer something like "save_source_hostname" so it's clear what setting it will do.
  • is this flag specific to journalbeat because other beats don't have a host.hostname field for add_host_metadata to override? It looks like add_host_metadata and its config would be a more natural place to put the flag otherwise. If this issue is specific to journalbeat, maybe add some comments to clarify why.
@@ -40,6 +40,8 @@ type Config struct {
CursorSeekFallback config.SeekMode `config:"cursor_seek_fallback"`
// Matches store the key value pairs to match entries.
Matches []string `config:"include_matches"`
// Remote defines if the journal is form a remote host
Remote bool

This comment has been minimized.

Copy link
@faec

faec Jul 1, 2019

Contributor

this should have a config:... annotation, right?

if r.config.Remote {
remoteHostname, err := fields.GetValue("host.hostname")
if err == nil {
fields.Put("source.hostname", remoteHostname)

This comment has been minimized.

Copy link
@andrewkroh

andrewkroh Jul 2, 2019

Member

Is the same thing in Filebeat called log.source.address (docs)? If so then I think it would be good to stay consistent.

journalbeat/reader/config.go Outdated Show resolved Hide resolved
journalbeat/input/config.go Outdated Show resolved Hide resolved
kvch and others added 6 commits Jun 28, 2019
Co-Authored-By: Andrew Kroh <andrew.kroh@elastic.co>
Co-Authored-By: Andrew Kroh <andrew.kroh@elastic.co>
@kvch kvch force-pushed the kvch:fix-journalbeat-preserve-remote-hostname branch from 446a2eb to 9e040d1 Jul 2, 2019
@faec

This comment has been minimized.

Copy link
Contributor

faec commented Jul 2, 2019

Oh, sorry, I still meant for the internal variable to match too, e.g. SaveSourceHostname... while reading code when I see a flag called Remote I expect it might be used elsewhere or have a lot of effects, whereas a function that says if flags.SaveSourceHostname { ...save source hostname... } is clearly doing all it needs to :-)

@kvch

This comment has been minimized.

Copy link
Contributor Author

kvch commented Jul 2, 2019

Failing tests are unrelated.

@faec
faec approved these changes Jul 2, 2019
Copy link
Contributor

faec left a comment

thanks!

@kvch kvch merged commit 620f283 into elastic:master Jul 2, 2019
3 of 4 checks passed
3 of 4 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
CLA All commits in pull request signed
Details
Hound No violations found. Woof!
beats-ci Build finished.
Details
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
When entries are coming from a remote journal, `add_host_metadata` overwrites the remote hostname with the local one. To preserve information on the remote server, I added a new config option `save_source_hostname`. Users need to set this when they are using `add_host_metadata` and reading from remote journals. Thus, the hostname is copied to `log.source.address` before the processor kicks in.

Closes elastic#12528
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.