Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make percentage_format_string support str.format() syntax #403

Merged
merged 6 commits into from
Aug 16, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
- Apply percentage_format_string to match_body percentage value; will appear in new percentage_formatted key - [#387](https://github.com/jertel/elastalert2/pull/387) - @iamxeph
- Add support for Kibana 7.14 for Kibana Discover - [#392](https://github.com/jertel/elastalert2/pull/392) - @nsano-rururu
- Add metric_format_string optional configuration for Metric Aggregation to format aggregated value - [#399](https://github.com/jertel/elastalert2/pull/399) - @iamxeph
- Make percentage_format_string support format() syntax in addition to old %-formatted syntax - [#403](https://github.com/jertel/elastalert2/pull/403) - @iamxeph

## Other changes
- [Tests] Improve test code coverage - [#331](https://github.com/jertel/elastalert2/pull/331) - @nsano-rururu
Expand Down
3 changes: 1 addition & 2 deletions docs/source/ruletypes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1347,8 +1347,7 @@ evaluated separately against the threshold(s).

``sync_bucket_interval``: See ``sync_bucket_interval`` in Metric Aggregation rule

``percentage_format_string``: An optional format string to apply to the percentage value in the alert match text. This also adds 'percentage_formatted' value to the match_body in addition to raw, unformatted 'percentage' value so that you can use the formatted value for ``alert_subject_args`` and ``alert_text_args``. Must be a valid python format string.
For example, "%.2f" will round it to 2 decimal places.
``percentage_format_string``: An optional format string applies to the percentage value in the alert match text and match_body. This adds 'percentage_formatted' value to the match_body in addition to raw, unformatted 'percentage' value so that you can use the values for ``alert_subject_args`` and ``alert_text_args``. Must be a valid python format string. Both format() and %-formatted syntax works. For example, both "{:.2f}" and "%.2f" will format '96.6666667' to '96.67'.
See: https://docs.python.org/3.4/library/string.html#format-specification-mini-language

``min_denominator``: Minimum number of documents on which percentage calculation will apply. Default is 0.
Expand Down
10 changes: 8 additions & 2 deletions elastalert/ruletypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ def __init__(self, *args):
def get_match_str(self, match):
percentage_format_string = self.rules.get('percentage_format_string', None)
message = 'Percentage violation, value: %s (min: %s max : %s) of %s items\n\n' % (
percentage_format_string % (match['percentage']) if percentage_format_string else match['percentage'],
self.format_string(percentage_format_string, match['percentage']) if percentage_format_string else match['percentage'],
self.rules.get('min_percentage'),
self.rules.get('max_percentage'),
match['denominator']
Expand Down Expand Up @@ -1297,7 +1297,7 @@ def check_matches(self, timestamp, query_key, aggregation_data):
match = {self.rules['timestamp_field']: timestamp, 'percentage': match_percentage, 'denominator': total_count}
percentage_format_string = self.rules.get('percentage_format_string', None)
if percentage_format_string is not None:
match['percentage_formatted'] = percentage_format_string % (match_percentage)
match['percentage_formatted'] = self.format_string(percentage_format_string, match_percentage)
if query_key is not None:
match = expand_string_into_dict(match, self.rules['query_key'], query_key)
self.add_match(match)
Expand All @@ -1308,3 +1308,9 @@ def percentage_violation(self, match_percentage):
if 'min_percentage' in self.rules and match_percentage < self.rules['min_percentage']:
return True
return False

def format_string(self, format_config, target_value):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about moving this format_string() into a util class so that it doesn't get repeated here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved format_string() to util.py and added tests for the function accordingly

if (format_config.startswith('{')):
return format_config.format(target_value)
else:
return format_config % (target_value)
6 changes: 6 additions & 0 deletions tests/rules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1303,6 +1303,12 @@ def test_percentage_match():
assert '76.1589403974' in rule.get_match_str(rule.matches[0])
assert rule.matches[0]['percentage'] == 76.15894039742994
assert 'percentage_formatted' not in rule.matches[0]
rules['percentage_format_string'] = '{:.2f}'
rule = PercentageMatchRule(rules)
rule.check_matches(datetime.datetime.now(), None, create_percentage_match_agg(76.666666667, 24))
assert '76.16' in rule.get_match_str(rule.matches[0])
assert rule.matches[0]['percentage'] == 76.15894039742994
assert rule.matches[0]['percentage_formatted'] == '76.16'
rules['percentage_format_string'] = '%.2f'
rule = PercentageMatchRule(rules)
rule.check_matches(datetime.datetime.now(), None, create_percentage_match_agg(76.666666667, 24))
Expand Down