Fluentd: concatenate long logs

[desaintmartin]

What this PR does / why we need it:

• Fluentd: concatenate long logs (>16KB) which have been splitted by Docker into several lines.
• Add fluentd-plugin-concat to fluent Docker image.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #52444

Special notes for your reviewer:

• This fix uses fluentd-plugin-concat, but maybe there are better solutions out there.
• This fix does not use the "partial_message" feature in the upcoming Docker release

Release note:

Fluentd: concatenate long logs (>16KB) which have been splitted by Docker into several lines, using fluentd-plugin-concat plugin.

[neolit123]

/sig instrumentation
/kind bug
/ok-to-test

[desaintmartin]

Up?
Using this in production for about a month. Although it might add some CPU usage for fluentd, it works well.

[desaintmartin]

/retest

[desaintmartin]

@neolit123 all test are passing, are you interested in reviewing?

[neolit123]

@desaintmartin
LGTM, but it's up to sig-instrumentation to merge.
you can try pinging them in the slack channel (#sig-instrumentation) if this stalls on reviews.

[neolit123]

might be a good idea to add a release note under Release note: as this is a bug fix.

[desaintmartin]

Good suggestion, thanks.

[coffeepac]

@desaintmartin thanks for your contribution! please up the patch level for the following versioned items:

• fluentd-es-config (currently 0.1.5, should be 0.1.6)
  • need to up the patch level in fluentd-es-ds as well
• fluentd-es-ds (currently 2.2.0, should move to 2.2.0)

[coffeepac]

/assign @coffeepac

[desaintmartin]

@coffeepac Thanks! isn't the versions in the fluentd-es-ds.yaml file tied to the used image which is still v2.2.0? Fixed for the configmap file.

[coffeepac]

@desaintmartin often the same but not necessarily. the version for the daemonset spec is for the entire spec and any versioned resources in it. I expect them to align again when the image revs a minor version.

[coffeepac]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: coffeepac, desaintmartin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• cluster/addons/fluentd-elasticsearch/OWNERS [coffeepac]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[desaintmartin]

/test pull-kubernetes-integration

[desaintmartin]

@coffeepac Am I doing something wrong or is it OK that this test does not pass?

[coffeepac]

that test is required to pass. I will take a quick look.

[desaintmartin]

May I rebase from master to see if the test is passing?

[coffeepac]

@desaintmartin sure. sorry, got a bit sidetracked from this.

[coffeepac]

/test pull-kubernetes-e2e-gke

[desaintmartin]

It finally passes.

[coffeepac]

it takes a village.

[coffeepac]

/lgtm

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

[coffeepac]

/test pull-kubernetes-integration

[desaintmartin]

Thanks. Should someone generate a new version of gcr.io/google-containers/fluentd-elasticsearch ?

[coffeepac]

@desaintmartin yes.

I will submit a PR and go ping the appropriate person in the next few minutes.

[mmacfadden]

@desaintmartin Thanks for this, this has been kicking my butt! One question. I noticed that the concat filter was put in the output section right before going to elasticsearch. We are using the json parser like this for one of our containers that exports logs as JSON:

    # This configuration handles the filtering of the Convergence Server
    <filter **.some-kubernetes-tags>
      @type parser
      key_name log
      reserve_data false

      <parse>
        @type json
      </parse>
    </filter>

The parser has been failing because the JSON log message has been split by the 16k limitation that this PR is addressing. However, it seems like the rejoining of the logs is going to happen after this filter, which means this filter will likely still fail.

Am I correct in my assumption? Would there be a way to move the concat plugin up in the chain?

[desaintmartin]

Can't you put this filter after the concatenation?
what I'm doing is

    # Concatenate multi-line logs (>=16KB)
    <filter kubernetes.var.log.containers.**> # yeah, small improvement since the PR
      @type concat
      key log
      multiline_end_regexp /\n$/
      separator ""
    </filter>

    <filter kubernetes.var.log.containers.**>
      @type parser
      key_name log
      <parse>
        @type multi_format
        <pattern>
          format json
        </pattern>
        <pattern>
          format nginx
        </pattern>
        <pattern>
          format none
        </pattern>
      </parse>
    </filter>

And it works well.

[mmacfadden]

@desaintmartin

Yes, I think that would work. That is what I was planning on doing. I just wanted to see if I was crazy in my assumption. Thanks for your help.