Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing get_hits_count for es 7.x #333

Merged
merged 6 commits into from
Aug 3, 2021
Merged

Fixing get_hits_count for es 7.x #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b9ff8b8
Fixing get_hits_count for es 7.x
JeffAshton Jul 7, 2021
901b5d0
Switching to check is_atleastsixtwo to align with other similar checks
JeffAshton Jul 7, 2021
04b90e2
Merge branch 'master' into es7_get_hits_count
JeffAshton Jul 7, 2021
56c3a28
Merge branch 'master' into es7_get_hits_count
jertel Jul 8, 2021
4f5d7c6
Merge branch 'master' into es7_get_hits_count
jertel Aug 3, 2021
6a95eb1
Update CHANGELOG.md
jertel Aug 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@
## Other changes
- [Tests] Improve test code coverage - [#331](https://github.com/jertel/elastalert2/pull/331) - @nsano-rururu
- [Docs] Upgrade Sphinx from 4.0.2 to 4.1.2- [#332](https://github.com/jertel/elastalert2/pull/332) [#343](https://github.com/jertel/elastalert2/pull/343) [#344](https://github.com/jertel/elastalert2/pull/344) [#369](https://github.com/jertel/elastalert2/pull/369) - @nsano-rururu
- Ensure hit count returns correct value for newer ES clusters - [#333](https://github.com/jertel/elastalert2/pull/333) - @jeffashton
- [Tests] Upgrade Tox from 3.23.1 to 3.24.0 - [#345](https://github.com/jertel/elastalert2/pull/345) - @nsano-rururu
- Upgrade Jinja from 2.11.3 to 3.0.1 - [#350](https://github.com/jertel/elastalert2/pull/350) - @mrfroggg
- [Tests] Add test code. Changed ubuntu version of Dockerfile-test from latest to 21.10. - [#354](https://github.com/jertel/elastalert2/pull/354) - @nsano-rururu
- [Tests] Improved test coverage for opsgenie.py 96% to 100% - [#364](https://github.com/jertel/elastalert2/pull/364) - @nsano-rururu
- Remove Python 2.x compatibility code - [#354](https://github.com/jertel/elastalert2/pull/354) - @nsano-rururu
- [Docs] Added Chatwork proxy settings to documentation - [#360](https://github.com/jertel/elastalert2/pull/360) - @nsano-rururu
- Add settings to schema.yaml(Chatwork proxy, Dingtalk proxy) - [#361](https://github.com/jertel/elastalert2/pull/361) - @nsano-rururu
- [Docs] Tidy Twilio alerter documentation - [#363](https://github.com/jertel/elastalert2/pull/363) - @ferozsalam
- [Docs] Tidy Datadog alerter documentation - [#380](https://github.com/jertel/elastalert2/pull/380) - @ferozsalam
- [Tests] Improve opsgenie test code coverage - [#364](https://github.com/jertel/elastalert2/pull/364) - @nsano-rururu
- [Tests] Improved test coverage for opsgenie.py 96% to 100% - [#364](https://github.com/jertel/elastalert2/pull/364) - @nsano-rururu
- [Docs] Update mentions of JIRA to Jira - [#365](https://github.com/jertel/elastalert2/pull/365) - @ferozsalam
- [Docs] Tidy Datadog alerter documentation - [#380](https://github.com/jertel/elastalert2/pull/380) - @ferozsalam

# 2.1.2
## Breaking changes
Expand Down
15 changes: 14 additions & 1 deletion elastalert/elastalert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,21 @@ def get_hits_count(self, rule, starttime, endtime, index):
five=rule['five']
)

es_client = self.thread_data.current_es
try:
res = self.thread_data.current_es.count(index=index, doc_type=rule['doc_type'], body=query, ignore_unavailable=True)
if es_client.is_atleastsixtwo():
res = es_client.count(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}}

index=index,
body=query,
ignore_unavailable=True
)
else:
Copy link
Contributor Author

@JeffAshton JeffAshton Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. :(

Copy link
Contributor Author

@JeffAshton JeffAshton Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

res = es_client.count(
index=index,
doc_type=rule['doc_type'],
body=query,
ignore_unavailable=True
)
except ElasticsearchException as e:
# Elasticsearch sometimes gives us GIGANTIC error messages
# (so big that they will fill the entire terminal buffer)
Expand Down