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 option "units" to monitor only specified systemd units #34

Merged
merged 2 commits into from
Jan 11, 2017

Conversation

Mousius
Copy link
Contributor

@Mousius Mousius commented Sep 23, 2016

Fixes #3 by providing an option matching the journalctl -u $SERVICE with a config option unit: $SERVICE.

@nrvnrvn
Copy link
Collaborator

nrvnrvn commented Oct 2, 2016

Hi!
Two questions:

  1. What if I want to filter more than one unit? journalctl -u $1 -u $2
  2. Why not use filters (processors since alpha5):
filters:
  - drop_event:
      when:
        not:
          equals:
            journal.systemd_unit: ${DESIRED_SERVICE}

See more https://www.elastic.co/guide/en/beats/filebeat/master/configuration-processors.html

@Mousius
Copy link
Contributor Author

Mousius commented Oct 4, 2016

I was unaware of filters, I'll take a look and if they work we'll start using the main journalbeat branch as well as replying to #3

@mheese
Copy link
Owner

mheese commented Oct 6, 2016

@DaMouse404 I like this. The main reason why I haven't included the Matches yet is because I haven't tested it before. I also think - like @nicorevin said already - though that this needs to be configurable as an array, then we could merge this.

Also, what about support for filtering other things than units? systemd has support for this out-of-the-box, as should have our settings then.

@nicorevin even though the filters should work, I think we should still consider adding the possibility to configure the reader in the input. The difference is that the journal reader won't receive all messages and that can drastically reduce system load - especially on system's with a high log volume. What do you think?

Additionally, it rises again my concerns that we are using my fork of go-systemd instead of the main CoreOS branch. It was necessary at the time as the changes were necessary to get all features that we want. We should review this again, and if necessary, do a PR against the main branch.

@nrvnrvn
Copy link
Collaborator

nrvnrvn commented Oct 6, 2016 via email

@mheese
Copy link
Owner

mheese commented Oct 21, 2016

@DaMouse404 did you work on any updates as discussed above? I kind of would like to get this functionality in.

@Mousius
Copy link
Contributor Author

Mousius commented Oct 22, 2016

@mheese - apologies for the delay. I updated it to create a set of matchers if you pass units as an array. I'm unsure which other journald filters will be useful so holding off on doing them all just for the sake of it. Tested it briefly by logging one of our services plus sshd.service and looks like it selects the right things and also defaults back to no matchers when I specify nothing.

@Mousius Mousius changed the title Add option "unit" to monitor a single systemd unit Add option "units" to monitor a only specified systemd units Oct 22, 2016
@Mousius Mousius changed the title Add option "units" to monitor a only specified systemd units Add option "units" to monitor only specified systemd units Oct 22, 2016
@mheese
Copy link
Owner

mheese commented Nov 4, 2016

@DaMouse404 thanks, I'll have time the 2nd half next week to work on journalbeat. I'll take the time to merge this as well. Thanks again for the contribution.

@mheese mheese self-assigned this Nov 4, 2016
@Mousius
Copy link
Contributor Author

Mousius commented Dec 7, 2016

@mheese - any update? 😸

Copy link
Owner

@mheese mheese left a comment

Choose a reason for hiding this comment

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

Clean and straight forward code. It also works for me after some quick testing.

@mheese
Copy link
Owner

mheese commented Dec 28, 2016

@nicorevin if you could also quickly review and test this, then I feel comfortable to merge it.

@mheese mheese merged commit 4bce13e into mheese:master Jan 11, 2017
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