Fixing get_hits_count for es 7.x

[JeffAshton]

get_hits_count isn't matching in 7.x because of the doc_type. This is an especially bad because it fails to match silently.

[JeffAshton]

Without doc_type:

es_client.count(
                    index=index,
                    body={'query': {'match_all': {}}},
                    ignore_unavailable=True
                )
{'count': 14074, '_shards': {'total': 1, 'successful': 1, 'skipped': 0, 'failed': 0}}

[JeffAshton]

With doc_type:

es_client.count(
                    index=index,
                    doc_type=rule['doc_type'],
                    body={'query': {'match_all': {}}},
                    ignore_unavailable=True
                )
{'count': 0, '_shards': {'total': 1, 'successful': 1, 'skipped': 0, 'failed': 0}}

This is silently just not matching any documents in 7.x. :(

[JeffAshton]

Looks to match if you use the implicit _doc type, but this makes side by side migrations not possible where 6.x clusters had a doc_type different from the default.

[jertel]

I noticed no unit test was included. I'm assuming because we're already missing unit coverage for this method?

[JeffAshton]

I noticed no unit test was included. I'm assuming because we're already missing unit coverage for this meth

I noticed no unit test was included. I'm assuming because we're already missing unit coverage for this method?

I can look at adding some coverage. Found the issue and wanted to get people aware. This is one where we really should have a few integration tests with an Elasticsearch docker instance. Mocking could have easily hidden the same issue.

[jertel]

Any additional coverage you can provide would be very appreciated. A small improvement to your change would be to define a new helper method called determineDocType() which would return None when using ES 6.2+. That would serve three purposes:

1. Eliminates maintenance of two similar code statements that call the count() function.
2. Allows for simple unit testing of the new helper method.
3. Provides a reusable method that we're likely going to need for ES 8.x compatibility.

[JeffAshton]

Any additional coverage you can provide would be very appreciated. A small improvement to your change would be to define a new helper method called determineDocType() which would return None when using ES 6.2+. That would serve three purposes:

1. Eliminates maintenance of two similar code statements that call the count() function.
2. Allows for simple unit testing of the new helper method.
3. Provides a reusable method that we're likely going to need for ES 8.x compatibility.

I was following the established pattern:

elastalert2/elastalert/elastalert.py

Line 1691 in 56c3a28

if self.writeback_es.is_atleastsixtwo():

There's a few of these in this file. I think my preference would be to move this branching logic into the Elasticsearch provider. I wish they hadn't done inheritance, but composition instead, so that all methods used from the external library are veted in one location.