[Metricbeat] Update Couchdb to v2

[sayden]

What does this PR do?

Updates module to use a Strategy patern and decide between v1 and v2 Couchdb parsing. To do so, it has to do a pre HTTP request to root path / to know which version is deployed on each fetch (done in fetch to allow hot upgrades in container environment).

I have removed testdata tests because one of them was a bit unnecessary with our current testdata implementation (a timeout test which was testing testdata framework more than the module itself). The other testdata doesn't work anymore because more than one request is necessary now to check version.

Because we can only test in CI with a single version, I maintained current Dockerfile

Why is it important?

ATM, we were only parsing v1 metrics and v3 is going to be released soon. This PR introduces v2 parsing maintaining compatibility with our previous version.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
  - [ ] I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

• Launch v1 and v2 containers

docker run -d --name couchdb16 -p 15984:5984 couchdb:1.6
docker run -d --name couchdb23 -p 25984:5984 -p 25986:5986 couchdb:2.3

• Setup Metricbeat Couchdb module to point to both servers ./metricbeat modules enable couchdb and leave config similar to this:

- module: couchdb
  metricsets: ["server"]
  period: 10s
  hosts: ["localhost:15984", "localhost:25984/_node/_local/_stats", "localhost:25984/_node/nonode@nohost/_stats", "localhost:25986"]

• Check that events from v1 and v2 are being received in, for example, Kibana. You can see version in field service.version and the service.id must be the same for all paths of v2.
• Extra mile, check dashboards, they must maintain compatibility and show info of both servers.

Related issues

• Closes metricbeat couchdb module not working with couch 2.x #16352
• Relates CouchDB metribeat module #10354

Screenshots

I checked that dashboard still works.

[mtojek]

I think it looks similarly to libbeat/common/version.go (semver parsing).

[sayden]

Good catch!

[mtojek]

I'm not convinced about the version checking routine being called during every Fetch(). I wonder if it's not better to determine it once and then use it. You can also revalidate it in case of an error (e.g. invalid schema) - to cover the case of potential upgrade.

[sayden]

Historically this was the preferred way to achieve this. Even in very heavy operations like opening connections to SQL databases. Frankly, I never liked very much the idea but I understood the implications (a long living SQL connection that may drop and leave the beat requiring a restart.

Revalidation using schema errors or any other more complex stuff is against the common "guides" (written nowhere) in Beats of trying to have explicit, easy to reason / easy to code / easy to maintain codebase. In Couchdb it's not a very good idea because the differences are very very small and can lead to unexpected results. An example are incomplete JSON returns by design or a mix of errors from the http transport layer with the database layer that can make your life a hell 😄

TLDR: I'll disagree and commit and move it to New 😉

[mtojek]

Hmm... I wonder if it can lead to a potential problem - beats app starts before couchdb so it can't determine the API version.

[ycombinator]

Hmm... I wonder if it can lead to a potential problem - beats app starts before couchdb so it can't determine the API version.

Yes! We ran into this exact issue with a couple of the stack monitoring modules in Metricbeat and had to move some API calls from New into Fetch. See #15258 (fixed by #15270) and #15276 (fixed by #15306).

One solution I've seen in the past is caching such "infrequently changing" information for some period of time to save on checking it every Fetch() period. But that can introduce its own issues such as staleness so it might not be worth it. In general I err towards keeping it simple until it starts to be a problem.

[mtojek]

Regarding the offline discussion - I think you can move the code determining API version just before calling the right fetcher. Both fetchers should be stateless, so you freely initialize them in New() and get rid of any unnecessary mem allocations in Fetch.

[mtojek]

Do you think this can be optimized to not create a new instance every time the Fetch is called?

[sayden]

See above

[sayden]

jenkins, test this

[sayden]

I have done a last update by fetching on New and retrying on Fetch in case something of service unavailable on New.

Let's park the implementation here until we have any feedback from community and users before over engineering this more. The real problem here is that a JSON parsing from v1 vs v2 won't raise any error. A custom JSON parser must be implemented to prevent silent errors on hot upgrades. As this is a very short-living corner case, it's reasonable to stop here.

[mtojek]

nit: I would hide this condition inside retrieveFetcher. The provider creates an instance if it's not ready.

[sayden]

Moved it inside, thanks!

[mtojek]

Only one nit-pick