Reason for hardcoding endtime of top_count_keys query to timestamp + 10 minutes?

[owidjaja]

Hello, referencing from #1462 (comment), is there a specific reason behind hardcoding the end time to be: timestampX + 10 minutes ?

I guess this must be the line of code for that:

elastalert2/elastalert/elastalert.py

Line 1365 in d400d00

end = ts_to_dt(lookup_es_key(match, rule['timestamp_field'])) + datetime.timedelta(minutes=10)

Not sure I understand the reason from referenced comment:

Range Stop: timestampX + 10 minutes (hardcoded, in case there were logs mis timestamped from the future)

My understanding is that timestampX belongs to the final record that reaches the num_events param of frequency rule, so not sure the intent for stretching the end time beyond this record's timestamp.

And wouldn't this be the reason for causing the mismatch of count if there happens to be subsequent immediate records in Elastic that matches the filter e.g. 20 hits during 3:20 ~ 3:21 in previous example.

---

For our use case, we are experiencing the mismatch similar to #1462 (comment), and found that num_hits seems to match better with what we see on ELK log (Kibana).

Assuming the explanation from #1462 (comment) to be the culprit of the mismatch (hits from previous run):

So most likely a previous rule run, such as from the 3:18 to 3:19 query, had also found some hits, but not enough to trigger the alert. Then the next rule run found 77 hits, and so the top_count calculation included some hits from the previous minute.

Any solution then for making sure top_count_keys matches the num_hits numbers? We've tried playing around with timeframe, buffer_time, run_every, but failed to have the numbers to match.

Use case is timeframe to be 1 minute, and the rest is flexible.

Appreciate any suggestions please, thx

[owidjaja]

For what it's worth, I did just remove the addition of the datetime.timedelta(minutes=10) , and now the top_count_keys count matches exactly the ELK log from between the transmitted event's @timestamp - timeframe to @timestamp.

Could use some help still though about the rationale behind the 10 minutes addition in the first place.

[jertel]

I doubt anyone can give the original reason for that 10 minute addition, as the original author of that change likely isn't watching this forum. My comment in the referenced post is my best guess as to what it's doing.

I think a better use of time would be to submit a PR that makes that hardcoded value configurable. If you're interested in submitting that PR, please read through the short list of contribution guidelines first.

[owidjaja]

Hey, thanks for the replies. Yep, my bad, forgot this project has been ongoing for 10 years! (and a fork). Was just surprised not many others have been running against this issue as well.

Based on Yelp/elastalert@5285a3d, it seems that the intent was to have top_count_keys:

start = @timestamp - get_or_default(timeframe, 10 mins)
end = @timestamp + 10 mins

If we have no further insight towards the hardcoded 10 mins in the end time, would you guys agree then for a PR to change the behavior to something like:

start = @timestamp - get_or_default(top_count_timeframe, timeframe)
end = @timestamp + get_or_default(top_count_timeframe, 0)

where top_count_timeframe is a configurable setting if top_count_keys is set.

Maybe the top_count_timeframe in the config can have a default value of 10 mins as well for backwards behavior sake?

[nsano-rururu]

I think it would be better to make the following changes so that they don't affect existing users. What do you think?

if rule.get('top_count_timeframe'):
    # This additional code
else:
    # If there is no setting for top_count_timeframe, it will behave the same as before.
    start = ts_to_dt(lookup_es_key(match, rule['timestamp_field'])) - timeframe
    end = ts_to_dt(lookup_es_key(match, rule['timestamp_field'])) + datetime.timedelta(minutes=10)

[owidjaja]

Yeah, I think that would be better actually. And in that case, default value of top_count_timeframe could be 0 minutes then?

[owidjaja]

Wait nvm on default of 0, I guess the code would look like:

if rule.get('top_count_timeframe'):
    start = @timestamp - rule.get('top_count_timeframe')
    end = @timestamp + rule.get('top_count_timeframe')
else:
    # If there is no setting for top_count_timeframe, it will behave the same as before.
    start = ts_to_dt(lookup_es_key(match, rule['timestamp_field'])) - timeframe
    end = ts_to_dt(lookup_es_key(match, rule['timestamp_field'])) + datetime.timedelta(minutes=10)

with top_count_timeframe as an optional setting in the config, and we can explain the default (else block) behavior on the docs.
does this go along with what you had in mind?

[nsano-rururu]

That's as expected.