Update AlertService for Bucket-Level Alerting

[qreshi]

Signed-off-by: Mohammad Qureshi qreshi@amazon.com

Issue #, if available: #86

Description of changes:
Update AlertService implementation with utility methods needed for Bucket-Level Monitor execution

CheckList:
[x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[rishabhmaurya]

this constant defined here looks ugly, move somewhere else if it makes sense.

[rishabhmaurya]

This 500 size could be too big and consume big chunk of heap if individual bucket size is big. Will loading alerts in paginated way help here?
We have a requirement in MonitorRunner to load all alerts and then dedupe any existing ones. Can this process of dedupe/categorization be done in a paginated way? If we load all current alerts in an ordered way and also, paginate and keep discarding old pages to categorize the new alerts, wouldn't that help in limiting the memory here?
We don't have to fix it now, but something we can think about and probably create an issue for future optimization if you think its feasible.

[qreshi]

Right, the reason this was a harcoded value and not defined as a constant was because it was arbitrary for the time being. We should discuss with @elfisher and see where we want to draw the cutoff.

The problem is that this size is limiting the Alerts that are being fetched at the Monitor level whereas any circuit breakers we add might make more sense at the Trigger level (since that's where we de-dupe). Imposing a circuit breaker in general to the max Alert count will require some refactoring in the MonitorRunner logic since currently it is indexing new/de-duped Alerts as it paginates composite agg results (so we wouldn't be able to circuit break halfway without having partial results unless we removed that behavior).

For the time being, I'll move this value of 500 to a constant variable and once all changes are in main, at the very least I'll add a warn log that this value has been exceeded at the Monitor level. If time permits, let's discuss a circuit breaker option (and if we are willing to fail the Monitor here). Either way, we will be creating an issue for this if it is not part of the initial release.

[rishabhmaurya]

do we always load full error history? can this be optimized as it can be occupy a big chunk of heap. Another possible optimization which can be added later if you think its a problem.

[qreshi]

Yeah, looks like errorHistory is capped to 10 elements but I agree, we can revisit some of these operations to think through optimizations.