jertel / elastalert2

ElastAlert 2 is a continuation of the original yelp/elastalert project. Pull requests are appreciated!
https://elastalert2.readthedocs.org
Apache License 2.0
914 stars 287 forks source link

Fields parameter introduced in 2.12.0 breaks existing usage of fields parameter for new_term rules #1397

Closed boris-de-groot closed 6 months ago

boris-de-groot commented 6 months ago

We recently upgraded our Elastalert version from 2.10.0 to 2.17.0 and since then we started receiving query errors on our alerts that are of type new_term. After testing intermediate versions this started from 2.12.0 and reviewing the changelog a new fields parameter was introduced. however there already was a fields parameter to be used in conjunction with the new_term type rule: https://elastalert2.readthedocs.io/en/stable/ruletypes.html#new-term

the rule (simplified) which gives this error is:

name: rulename
type: new_term
index: index-*
filter:
  - query_string:
      query: "field0: \"INFO\" AND field1: \"value\" AND _exists_: field2"

alert:
  - "opsgenie"

query_key: field2

fields:
  - - field2
    - field3

I have tried dropping both the _exists_ and query_key parts but the error remains:

WARNING:elasticsearch:POST https://domain.example.com:443/index-*/_search?_source_includes=field2%2C%40timestamp%2C%2A&ignore_unavailable=true&scroll=30s&size=10000 [status:400 request:0.009s]
ERROR:elastalert:Error running query: RequestError(400, 'x_content_parse_exception', '[1:412] [docvalues_field] Expected START_OBJECT but was: VALUE_STRING')

We currently running OpenSearch 2.7, but this rule has also ran without issue against ES7

please advice how to upgrade beyond 2.11.0

jertel commented 6 months ago

@Goggin, PR #1193 re-used an existing fields property. Unfortunately this has broken new_term rules. Can you submit a new PR to rename your new property to something that will not collide with the existing fields property? Perhaps include_fields since this is somewhat related to the include property.

jertel commented 6 months ago

Fixed, and now available via the the latest docker image tag.

Goggin commented 6 months ago

Sorry, didn't mean to leave this. Thanks for making that change.