Collect version metadata for Fluentd

[florimondmanca]

What does this PR do?

• Collect version metadata for the Fluentd integration.

Motivation

• Ongoing effort to collect version metadata for Inventories across all integrations.

Additional Notes

• Implementation inspired by Add version metadata #4968 (esp. the fluentd command override trick).
• Some changes to tox.ini and the Compose setup as Fluentd doesn't always use the vX.Y.Z scheme for their Docker image tags.
• CAVEAT: will only find the version if the configured fluentd command is available in the Agent. See Collect version metadata for Fluentd #5057 (comment)

Review checklist (to be filled by reviewers)

• Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
• PR title must be written as a CHANGELOG entry (see why)
• Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
• PR must have changelog/ and integration/ labels attached

[codecov]

Codecov Report

Merging #5057 into master will increase coverage by 9.12%.
The diff coverage is 96.49%.

Impacted Files	Coverage Δ
fluentd/datadog_checks/fluentd/fluentd.py	92.64% <100%> (+3.28%)	⬆️
fluentd/tests/common.py	100% <100%> (ø)	⬆️
fluentd/tests/test_metadata.py	100% <100%> (ø)
fluentd/tests/mock/fluentd_version.py	100% <100%> (ø)
fluentd/tests/conftest.py	83.33% <33.33%> (-16.67%)	⬇️
fluentd/tests/test_unit.py	100% <0%> (ø)	⬆️
fluentd/tests/test_integration_and_e2e.py	85% <0%> (ø)	⬆️
fluentd/datadog_checks/fluentd/__init__.py	100% <0%> (ø)	⬆️
... and 845 more

[AlexandreYang]

Looks good overall 👍 , few comments.

[AlexandreYang]

It seems the the api doesn't provide currently the version, but can be added easily upstream here, example:

      def config_json(req)
        obj = {
          'pid' => Process.pid,
          'ppid' => Process.ppid
          'version' => Fluent::VERSION
        }.merge(@agent.fluentd_opts)
        opts = build_option(req)

        render_json(obj, pretty_json: opts[:pretty_json])
      end

https://github.com/fluent/fluentd/blob/master/lib/fluent/plugin/in_monitor_agent.rb#L65-L68

Even if we push this change upstream, it will be available only for new versions, so we will still need to retrieve versions via command line for old versions.

[florimondmanca]

Yup. I'll open an issue (or most likely a PR directly) on the fluentd repo to discuss adding the version to their monitoring REST API.

Agreed that this is not a blocker w.r.t. this PR, though? (i.e. we can add the API-based version lookup later.)

[florimondmanca]

@AlexandreYang I opened fluent/fluentd#2705 and fluent/fluentd#2706 on the Fluentd repo.

Will discuss there whether supporting accessing a remote Fluentd instance makes any sense at all from a monitoring perspective.

I'm now less sure that it can actually be a possible setup: most examples from the Fluentd monitoring docs show calls to localhost or similar.

So, if people always only install the integration on the host where Fluentd is installed, then the benefits we'd get from using the API are less clear IMO.

[l0k0ms]

hello, is there a reason why we add this parameter in the init_config and in the instance section ? :)

[florimondmanca]

Hi @l0k0ms, thanks for looking at this.

The fluentd parameter allows users to specify how the integration should call into Fluentd to retrieve its version. By default it should use the fluentd shell command, but users may have Fluentd locally available elsewhere, e.g. in a container or a path to a custom binary (as documented in the example config file).

On our side, we use it to test various cases — e.g. what happens if the Fluentd command does not exist, or if the command returns unexpected output?

This technique is currently in use in some other integrations, e.g. Gunicorn (see #4968 for discussion) and cassandra-nodetool.

[l0k0ms]

@florimondmanca ohhhh i missed that.. many thanks for the clarification :) Doc wise this PR is g2g

[bai]

👋Given that "remote version" PR has been merged into fluentd upstream, is there any way to make datadog agent use it instead of shelling out to fluentd --version?

It seems like there's no way to disable this version check and at 15k+ hosts we seem to be having a lot of logging noise about those failures:

2020-03-11 03:46:53 UTC | CORE | WARN | (pkg/collector/python/datadog_agent.go:118 in LogMessage) | fluentd:edfda3e12aed702d | (fluentd.py:117) | fluentd version not found in stdout: ``

Running datadog-agent on kubernetes so naturally no fluentd binary available within datadog container.

[florimondmanca]

Hi @bai, thanks for the heads up!

You're correct, upstream PR fluent/fluentd#2706 got merged and got released as part of Fluentd v1.8.0 in December (see changelog).

This means we can look at using the new REST endpoint in priority, and fallback to a shell command for older Fluentd installs.

We'd also need to update the integration to honor any enable_metadata_collection flag set in the Agent config. Unfortunately this is not checked in the code yet so I can't recommend a workaround for noisy logs yet.

We'll try to take a look at these items soon, and report back! Thanks.

[hithwen]

Disabling collection done in #6061

[hithwen]

REST call added in #6062

[hithwen]

@bai Fluentd check with latest changes is being released. Version is 1.6.0. You can install it with the integration command

[bai]

Many thanks, this is really helpful 🙏