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

input-journald-upload: use removeFields from config, fix include/exclude test #277

Merged
merged 1 commit into from
Jul 2, 2021

Conversation

mikolasan
Copy link
Contributor

After some old refactoring in 64f435872178ea16357e4bcad13b3a3db63f6802 the function parseLine became unused and still in the code, moreover removeFields parameter from configuration was not working. This PR returns usage of removeFields as it was before the refactoring.

Also fixed a bug with include/exclude parameter (hint: log._SYSTEMD_UNIT !== log[_SYSTEMD_UNIT]).

And also ignore messages in journald not coming from systemd services (I had audit and kernel syslog messages flooding my logs). This is a questionable change applied only to my setup, so please review it.

@otisg otisg requested a review from adnanrahic May 17, 2021 15:26
@otisg
Copy link
Member

otisg commented Jul 1, 2021

WDYT @adnanrahic ? Everything except the "And also ignore messages in journald not coming from systemd services " part? We could leave the else + comment + comment out that return to disable this change?

Copy link
Contributor

@adnanrahic adnanrahic left a comment

Choose a reason for hiding this comment

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

This looks good! Removing the one return statement is the only thing to fix. Then we can get this merged. 😄

lib/plugins/input/journald-upload.js Outdated Show resolved Hide resolved
Copy link
Contributor

@adnanrahic adnanrahic left a comment

Choose a reason for hiding this comment

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

Looks great!
Thanks a lot for this PR @mikolasan, I'll merge it right away.

@adnanrahic adnanrahic merged commit c8328e4 into sematext:master Jul 2, 2021
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.

None yet

3 participants