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

filter fluentd container logs #402

Merged
merged 21 commits into from Feb 12, 2020

Conversation

vsinghal13
Copy link
Contributor

@vsinghal13 vsinghal13 commented Feb 7, 2020

Description

This PR changes the default fluentd log level to be info and modifies the fluentd pipeline to ingest only specific fluentd logs into Sumo.

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in
  • Confirm only filtered fluentd logs ingested in Sumo

Copy link
Contributor

@frankreno frankreno left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rvmiller89 rvmiller89 left a comment

Choose a reason for hiding this comment

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

Let's talk about this -- it's not what I had in mind.

@@ -8,51 +8,64 @@
time ${[record["log"].split(/[\n\t]+/)[0]].map! {|item| JSON.parse(item)["time"]}.join("")}
</record>
</filter>
# only match fluentd logs based on fluentd container name and label them @FLUENT
Copy link
Contributor

Choose a reason for hiding this comment

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

we should try to see if these logs are already tagged by Fluentd itself: https://docs.fluentd.org/deployment/logging#capture-fluentd-logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The label @FLUENT_LOG uses <match fluent.**> which will not stop fluentd from outputting to stdout. We will then have duplicate fluentd logs.
Ref: fluent/fluentd#641 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Did it not work starting fluentd with -o /dev/null to output logs to file /dev/null instead of stdout? https://docs.fluentd.org/deployment/logging#output-to-log-file

@vsinghal13
Copy link
Contributor Author

@rvmiller89 have modified the existing pipeline NORMAL label to filter out the fluentd logs instead of having a different pipeline for it.

@samjsong Still kept the filter plugin confs in separate files to keep the fluentd pipeline dry and easy to read.

Copy link
Contributor

@samjsong samjsong left a comment

Choose a reason for hiding this comment

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

Overall LGTM. I have mixed feelings about pulling out each of logs.*.filter.conf filter configs, mostly because it creates another layer of indirection, though I agree it may make the pipeline itself easier to read. I am okay either way.

@type grep
<regexp>
key log
pattern /\[info\]|\[fatal\]|drop_oldest_chunk/
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, we want to ingest info logs here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, I had changed that to test locally. Will fix it to be error.

# only ingest fluentd logs of levels: {error, fatal} and warning messages if buffer is full
@type grep
<regexp>
key log
Copy link
Contributor

Choose a reason for hiding this comment

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

need to indent this line and the next?

@vsinghal13 vsinghal13 removed the request for review from lei-sumo February 12, 2020 01:11
@vsinghal13 vsinghal13 merged commit 1bf4b51 into master Feb 12, 2020
@vsinghal13 vsinghal13 deleted the vsinghal-filter-fluentd-container-logs branch February 12, 2020 04:24
Comment on lines -31 to +32
<filter **>
# sumologic kubernetes metadata enrichment filter plugin
<filter containers.**>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@samjsong @rvmiller89 I suspect it is because of this change.

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

5 participants