[mongo] make timeout configurable and increase the default

[benmccann]

We have one MongoDB instance that's misbehaving. It's a bit slow to connect to it because of the problems it's having and so it's difficult to tell what's wrong because we're not getting any metrics from it.

[yannmh]

Thanks for contributing @benmccann !

Yes, let's assess this change for the 5.5.0 agent release. Out of curiosity, do you know long the check takes to connect to your instance ?

[benmccann]

Normally it connects very quickly. But it looks like every so often there's some job that's just pounding our instance with unoptimized queries, and so when that happens we're seeing it take about 15s.

[remh]

Can we make it configurable instead ?

[benmccann]

Making it configurable sounds reasonable. I'd still like to increase the default since I think it's lower than it needs to be.

Do you have an example of how we'd make it configurable? E.g. is there a certain file we'd edit to set the config value? And then would we read it from init_config or agentConfig?

[remh]

etcd is a good example.
https://github.com/DataDog/dd-agent/blob/5.4.3/conf.d/etcd.yaml.example#L7
https://github.com/DataDog/dd-agent/blob/5.4.3/checks.d/etcd.py#L74

I agree that we can increase the default timeout as well, although 60secs seems a bit excessive.

Does 20 secs seem reasonable ?

Thanks again for the feedback and the PR!

[benmccann]

Would there be any problems caused by having the timeout be too high? Seems like it doesn't cost us much to make it higher.

I'd settle for 30 secs ;-) We were taking about 15 secs to connect, which is dangerously close to 20 secs.

[remh]

Having a connection hanging for too long can triggers the watchdog that would kill and restart the agent.
Basically the watchdog kicks off if the overall check loop takes more than 2minutes.

So we are still fine with 30 secs but it's something we have to be cautious with :)

Thanks again for this!

[yannmh]

Thanks a lot @benmccann, tests should be passing now 🎉.

Since we can now set this timeout parameter from the YAML configuration file, I think it would make sense to switch back the default value to 10 seconds, so that collector doesn't hang for too long when the instance is unreachable.

Also, we recently introduced some contribution guidelines to keep a nice, homogeneous, and easy to browse git history.
Would you mind rewording the commit title following these rules please ? Again, thank you for your contribution.

[benmccann]

Thanks @yannmh. I rebased to get the fix for the network tests and updated the commit message.

Regarding the timeout, I'm not sure 10s is a good default since we regularly go over that and I wouldn't be surprised if others hit the same issue. I understand we want to stay clear of the 2 min mark that @remh mentioned, but he seemed fine with 30s. Are there other issues? Does it hang the collection of your other metrics as well?

[benmccann]

Btw, @yannmh @remh I'll let you guys decide the default timeout issue and update the PR accordingly. I do think that 10s is rather low though and setting it to at least 20 would solve most of the problems we've seen. It's not clear to me what negative effects there might be from setting it higher

[benmccann]

Yep, all tests are passing now! Thanks guys!

[yannmh]

Thanks for the further explanations @benmccann. Let's keep it at 30s then.

Merging 🚀 !