fix compound query key in metric aggregation with bucket_interval

[just1900]

Fix metric aggregation while using compound query key and bucket_interval together

What happened here

1. config rules with type metric_aggregation, compound query_key and bucket_interval like below.

type: metric_aggregation
query_key: ["prometheus.labels.instance","prometheus.labels.app"]
bucket_interval:
  seconds: 30

2. Debugging into the call stack.
   the payload_data param in function add_aggregation_data will contain compound bucket_aggskey looks like below https://github.com/jertel/elastalert/blob/c8949c1e715ca0b9b5b923a5e3625c52163a4068/elastalert/ruletypes.py#L1034-L1041

{
    "bucket_aggs": {
        "doc_count_error_upper_bound": 0,
        "sum_other_doc_count": 0,
        "buckets": [
            {
                "key": "kafka",
                "doc_count": 32916,
                "bucket_aggs": {
                    "doc_count_error_upper_bound": 0,
                    "sum_other_doc_count": 0,
                    "buckets": [
                        {
                            "key": "stress-test-kafka",
                            "doc_count": 17628,
                            "interval_aggs": {
                                "buckets": [
                                    {
                                        "key_as_string": "2021-03-31T02:11:30.000Z",
                                        "key": 1617156690000,
                                        "doc_count": 1469,
                                        "metric_prometheus.metrics.kafka_controller_kafkacontroller_activecontrollercount_sum": {
                                            "value": 1
                                        }
                                    },
...(duplicated character removed)

then after indexing https://github.com/jertel/elastalert/blob/c8949c1e715ca0b9b5b923a5e3625c52163a4068/elastalert/ruletypes.py#L1039 and unwarp_term_bucket https://github.com/jertel/elastalert/blob/c8949c1e715ca0b9b5b923a5e3625c52163a4068/elastalert/ruletypes.py#L1048-L1053
the aggregation_data param in function check_matches looks like

{
    "key": "kafka",
    "doc_count": 32916,
    "bucket_aggs": {
        "doc_count_error_upper_bound": 0,
        "sum_other_doc_count": 0,
        "buckets": [
            {
                "key": "stress-test-kafka",
                "doc_count": 17628,
                "interval_aggs": {
                    "buckets": [
                        {
                            "key_as_string": "2021-03-31T02:11:30.000Z",
                            "key": 1617156690000,
                            "doc_count": 1469,
                            "metric_prometheus.metrics.kafka_controller_kafkacontroller_activecontrollercount_sum": {
                                "value": 1
                            }
                        },
...(duplicated character removed)

What we could see is that the interval_aggs key is not unwrapped here. Also, the function check_matches_recursive only unwrap bucket_aggs recursively.
https://github.com/jertel/elastalert/blob/c8949c1e715ca0b9b5b923a5e3625c52163a4068/elastalert/ruletypes.py#L1112-L1127
So if the bucket_interval is configured in the rules, and line 1127 will run into the KeyError while indexing self.metric_key.

How to Fix it

• adding unwrap for interval_aggs key in function check_matches_recursive

[just1900]

the pr in the upstream Yelp/elastalert#3161

[jertel]

Thanks for the contribution. I see some related tests in tests/rules_test.py, specifically test_base_aggregation_payloads(). Would be good to see some coverage added for this scenario.

[mrfroggg]

Could this solution works for percentage_match?
Yelp/elastalert#1281 / Yelp/elastalert#2133

[jertel]

Good question. It looks unrelated but there could be something I'm not seeing.

The submitter hasn't responded to the unit test topic. If anyone else needs this change merged, please add a quick unit test to the existing rules_test.py, and I'll merge it in.

[just1900]

@jertel PTAL, I've just got some spare time to look into this, sorry for the long delay.
The unit test have been added and I've modified the implementation for the reason that there should be only one interval_aggs bucket, so if the recursive call have seen interval_agg, it means the recursive call reaches the end.

@mrfroggg I checked your commented issue and the code in Yelp#2133, few things I have noticed

• Yelp#1281 is the issue with compound query_key but this issue is mainly focus on bucket_interval, you could check the code change in this PR for clearer understanding.

• the implementation there is basiclly the same with current implementation https://github.com/jertel/elastalert/blob/c8949c1e715ca0b9b5b923a5e3625c52163a4068/elastalert/ruletypes.py#L1112-L1127, so I'm afraid the problem happened in this issue remain unresolved there.

• since these two methods are basiclly the same, I guess it is worth a refactoring, maybe you could dive into it more.

[jertel]

Thanks for your contributions @just1900