Add metric_agg_script to MetricAggregationRule

[dequis]

Description

Copied from SpikeMetricAggregationRule, which originally copied
generate_aggregation_query from MetricAggregationRule and added these
two lines.

So this commit just makes the two generate_aggregation_query look the
same, which just works and I'm not really sure why no one has done it
before.

Should not be a breaking change since it just adds an optional rule attribute.

Checklist

• I have reviewed the contributing guidelines.
• I have included unit tests for my changes or additions.
• I have successfully run make test-docker with my changes.
• I have manually tested all relevant modes of the change in this PR.
• I have updated the documentation.
• I have updated the changelog.

Questions or Comments

No idea how to unit test this. It runs against a mocked elasticsearch, right? I was planning to not write any until I saw this checklist so uh, that's a tomorrow problem.

Most of my manual testing was with an out of tree extension, but I haven't gotten around to test this specific version of the code, so that one is unchecked.

I can't really add a changelog entry before creating the pull request.

About the code: to be honest, this is a really lazy change, but it follows the trend of someone copypasting one version of generate_aggregation_query, modifying it a little and never applying that change to the original.

I don't know what's the best way to deduplicate these things, and doing something like MetricAggregationRule.generate_aggregation_query(self) would be needlessly obscure imo. Can't really deduplicate the docs.

[dequis]

Relevant issue in the old issue tracker Yelp/elastalert#2901

[jertel]

Thanks for the contribution! A unit test can be added to the existing tests/rules_test.py, following the template of:

1. Creating a rule object with the desired configuration
2. Instantiating a new rule type object, and passing in the rule as the input param
3. Invoking the modifying function
4. Comparing the output to the expected ouput.

Once you have this included, and confirmed the latest codebase works with your PR please update the checklist and we'll get this merged in.

[dequis]

There you go! Rebased to master and added a test. (Also now I see how this works without a whole elasticsearch in the background, makes sense!)

[jertel]

I'm approving this to get it merged in. It would be good to update the schema.yaml file with this metric_agg_script parameter at some point, for both rule types. It's currently missing completely so that's why I'm not holding up this PR.