Improve log messages for data stream support checks

[edmocosta]

What this PR does?

• Refactored the check_data_stream_config! method to reduce the cyclomatic complexity.
• Changed the data_stream_default log levels from debug to info. Those messages are helpful to users understanding the data stream auto-configuration mechanism/default and troubleshooting possibly incompatible settings.
• Logs were being printed twice due to a race condition on the data_stream_config? method, caused by the register function.

Closes #1070

[edmocosta]

This comment looks to be true only for the assert_es_version_supports_data_streams method (which does the connected elastic search version validation).
Also, before this PR, there were no guarantees that this method would be invoked only AFTER the @after_successful_connection_thread was completed, the register method was also invoking it, making it prone to race conditions.

[andsel]

Nice refactoring.
I've left a doubt on the usage of #data_stream_config? because on the method signature is explicitly cited that has to be used after #after_successful_connection is invoked.

Left a couple of suggestions I would like to know your ideas

[andsel]

LGTM

[jsvd]

Thanks for the changes here.
My major concern in this PR is the lack of testing for the new validation logic.
We should have a few tests describing the new permutations of settings and the validations being added here.

[edmocosta]

Thanks for the changes here. My major concern in this PR is the lack of testing for the new validation logic. We should have a few tests describing the new permutations of settings and the validations being added here.

Hey @jsvd, thanks for reviewing! :)

I'm probably missing something, but this PR should not be adding/changing any new validation logic.
Except for the log messages text, It should behave exactly the same and should be already covered by the existing tests.
We did some refactoring on the check_data_stream_config! method, moving code around, and a small change suggested on this PR thread, which should not affect the existing logic.

Could you please provide more details on that?

[jsvd]

Fair enough, I just happened to be playing around with the PR and removed the check for settings incompatible with explicitly enabling data streams (here), and didn't cause any tests to fail in the data_stream_support_spec.rb. That said I realize it's not an issue with your refactoring, it was a missing test case already.

Overall, after some extra manual testing I'm good with merging.

[jsvd]

LGTM