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

Fix Multiline support for kubernetes collection #313

Merged
merged 13 commits into from
Nov 26, 2019

Conversation

vsinghal13
Copy link
Contributor

@vsinghal13 vsinghal13 commented Nov 25, 2019

Description

In order to fix the multiline support the following config was added:

  1. Add a new regex parser for multiline detection
  2. Switch on the Multiline feature of fluentbit for multiline log detection
  3. Replace FluentD concat plugin with record_transformer plugin to process the multiline logs in proper format.

Multiline log :
multiline message

Single line log:
single line message

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

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.

Can you upload a screenshot of the final results, maybe for a single line message and a multiline message? You can drag an image into the PR description and it will automatically upload and embed.

deploy/helm/sumologic/values.yaml Outdated Show resolved Hide resolved
@rvmiller89
Copy link
Contributor

Can you merge master as well, I think your branch hasn't picked up the libsonnet stuff

enable_ruby
renew_record true
<record>
log ${record["log"].split(/[\n\t]+/).map! {|item| JSON.parse(item)["log"]}.join("")}
Copy link
Contributor

Choose a reason for hiding this comment

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

just confirming: This won't cause issues if somehow the log coming from containers.** doesn't have log/stream/time keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only expecting the log key and using that to generate the stream and time key value pairs from that, so it should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

But if the string contained in log doesn't have stream and time, i.e. if the user is not using docker, then this wouldn't work, correct? Maybe we should document somewhere that we only support docker for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually it would work even if the log doesn't have stream and timekeys. Its just that if the key is not present, the JSON.parse value will be empty string for the key.

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.

LGTM, thanks Vijit!

@vsinghal13 vsinghal13 merged commit faae809 into master Nov 26, 2019
@vsinghal13 vsinghal13 deleted the vsinghal-fix-multiline branch November 26, 2019 19:53
regex:
- name: multi_line
regex: (?<log>^{"log":"\d{4}-\d{1,2}-\d{1,2} \d{2}:\d{2}:\d{2}.*)

Choose a reason for hiding this comment

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

@vsinghal13 are we using these values somewhere else? because we have defined same thing in deploy/helm/fluent-bit-overrides.yaml

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 fluent-bit-overrides.yaml is an auto-generated file for all the fluent-bit values that have been overridden in values.yaml . This is done for non-helm users.

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

4 participants