percentage_match rule Kibana link generating wrong query

[aliosmanisikk]

Not sure yet if I'm doing sth wrong, so decided to create a discussion rather than an issue.

I'm running elastalert2 version 2.18.0

My alert rule looks like this

name: High error rate
description: "80% of requests should be successful"
type: percentage_match
index: prod-*
doc_type: _doc

filter:
  - query_string:
      query: 'message:"Response sent"'

match_bucket_filter:
  - query_string:
      query: 'message:"Response sent with errors"'

buffer_time:
  hours: 1
realert:
  hours: 1

max_percentage: 20
min_denominator: 20

query_key:
  - body.queryDetails.operationName.keyword
include:
  - body.queryDetails.operationName.keyword

# Generate link
generate_opensearch_discover_url: true
kibana_discover_app_url: "https://opensearch.grandvision.io/app/data-explorer/discover#"
kibana_discover_index_pattern_id: "prod-*"
kibana_discover_version: "2.11"

# Alert format
alert:
  - slack
slack_channel_override: "#graph-prod-alerts"
slack_title: "Query/mutation error rate higher than 20%"
alert_text_args:
  - body.queryDetails.operationName.keyword
  - opensearch_discover_url
alert_text_type: alert_text_only
alert_text: |
  *{0}* error rate is more than 20%.
  <{1}|*Open OpenSearch query*>

My alert text looks like this {'keyword': 'getCart'} error rate is more than 20%. while it should looks like getCart error rate is more than 20%.

And when I click the kibana link, the query looks like this

{
  "query": {
    "match": {
      "body.queryDetails.operationName.keyword": {
        "query": {
          "keyword": "getCart"
        }
      }
    }
  }
}

instead of

{
  "query": {
    "match": {
      "body.queryDetails.operationName.keyword": "getCart"
    }
  }
}

As a result, the query on Opensearch fails. It used to work correctly in v2.9.0 . This started occurring after upgrading to v2.18.0

[jertel]

I'm surprised the OpenSearch link worked at all in 2.9.0, given that it wasn't put into the product until 2.15.0. Is there any other info you can provide, such as the rule yaml before the upgrade, or the data view for operationName as shown in a curl fetch or Discover screenshot?

[jertel]

It would help if you can tell us which specific version of ElastAlert 2 stopped working as expected. Since there were 9 versions in between 2.9 to 2.18 that is a lot of time that has passed and a lot of code that has changed.

[aliosmanisikk]

Unfortunately, we upgraded 9 versions in one go. Let me check if I can find specific version

[aliosmanisikk]

@jertel

I installed Elasticsearch, Kibana, Elastalert2 locally and I was able to reproduce the issue.

Including v2.15, the Kibana link is good

{
  "query": {
    "match_phrase": {
      "mykey.keyword": {
        "query": "ali"
      }
    }
  }
}

Starting from v2.16, query has the issue

{
  "query": {
    "match_phrase": {
      "mykey.keyword": {
        "query": {
          "keyword": "ali"
        }
      }
    }
  }
}

Hope this helps investigating the issue.

[jertel]

Thanks for locating the specific release that broke.

@jmacdone, In PR #1330 the change to utils.py where it now trims the .keyword suffix from the lookup field is causing fields that have a keyword subfield to have the returned value to be a map containing a keyword key, as shown above, instead of returning the actual value.

Specifically, this change in #1330: https://github.com/jertel/elastalert2/pull/1330/files#diff-c94be3c6634086358da8944c7febe9c9e98fc1b4425fcc4e116e0740f5cf4eb6R88

The modified PR unit tests for that change don't directly test for this condition, where the field in the match has a nested dict with a keyword key. If a new unit test is added such as below, the test fails:

Ex:

It worked before your change because the code later in that function successfully found the keyword subfield by iterating into the nest dicts.

Can you offer any suggestions on how to handle this case where a field does have the keyword subfield?

[jmacdone]

It'll take me a while to get back into the headspace for that PR. Spitballing: perhaps a new variable like stripped_term instead of clobbering the existing term variable for this phase of the lookup? https://github.com/jertel/elastalert2/pull/1330/files#diff-c94be3c6634086358da8944c7febe9c9e98fc1b4425fcc4e116e0740f5cf4eb6R88-R90

[jmacdone]

Perhaps this isn't the correct approach, but if I rewrite _find_es_dict_by_key from iterative to recursive, and have it prefer keyword as fieldname, followed by treating keyword as a multifiled subfield, it passes unit tests, including the proposed one above and a few smoke tests with live data.
master...jmacdone:elastalert2:discussion/1450
@aliosmanisikk is that something you can out to see if it restores desired behavior?

[aliosmanisikk]

@jmacdone Yes, your fix is working for my use case

[jertel]

That's good to hear. Since all the unit tests are passing I think it's worth submitting a PR for this. If anyone can think of additional lookup scenarios that aren't represented by the unit tests we can get them added as new tests.