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

query_key value is a string instead of a dict #340

Closed
AntoineBlaud opened this issue Jul 13, 2021 · 5 comments
Closed

query_key value is a string instead of a dict #340

AntoineBlaud opened this issue Jul 13, 2021 · 5 comments

Comments

@AntoineBlaud
Copy link
Contributor

AntoineBlaud commented Jul 13, 2021

Behavior

Method check_matches in ruletypes.py appends the query key as a string.
So match is a dict composed of simple type like string or integer.

Rules types impacted:

  • metric
  • percentage

On another side, all other rules create match from documents content. The method involved is add_data(self, data)

Example

Suppose that we store documents like this one in the index:
{'@timestamp': value, metadata': { 'ip': value} }

  • If we use a frequency rule on this index , the match generated will have this template :

    [{**'metadata': {'ip**': "10.0.0.1"}, '@timestamp': datetime.datetime(20...o=tzutc()), '@version': '1', '_id': 'tR46i3oBTsznpu6_lwkh', '_index': 'index', '_type': '_doc'}]

  • If we use a metric rule, the match generated will have this template :

    [{**'metadata.ip**': '10.0.0.1' , '@timestamp': datetime.datetime(20...o=tzutc())'}]

Possible Solution

Fix check_matches by converting query_key value="string1.string2.stringN" to ["string1']["string2]['stringN"]

@AntoineBlaud
Copy link
Contributor Author

I can submit a pull request if you're agree with the problem.

@AntoineBlaud
Copy link
Contributor Author

This bug is especially annoying when we use match_enhancement because depending of the rule type, match data does not have the same data structure.

@jertel
Copy link
Owner

jertel commented Jul 15, 2021

This could break existing behavior for those who have written rules that rely upon the string format "foo.bar": 12 instead of "foo":{"bar": 12}. With that in mind it might be best to add the structured object to the match dict, as you are suggesting, but also keep the existing string match for now, and potentially mark it as deprecated in the docs.

@AntoineBlaud
Copy link
Contributor Author

AntoineBlaud commented Jul 15, 2021

I understand the problematic but we can't put both version of a field (string and dict) into the same doc. Elasticsearch accepts only one because it converts automatically { "string1.string2.string3" : "..."} to {"string1": { "string2": { "string3" }}}

@jertel
Copy link
Owner

jertel commented Jul 15, 2021

If that's the case then the we will have to identify this as a breaking change in the next release. Long term, I think making all rule types store match data consistently is the right direction.

AntoineBlaud added a commit to AntoineBlaud/elastalert2 that referenced this issue Jul 16, 2021
AntoineBlaud added a commit to AntoineBlaud/elastalert2 that referenced this issue Jul 23, 2021
[+] add test for expend_string_into_dict
[+] add check in test_metric_match and test_percentage_match
jertel added a commit that referenced this issue Jul 25, 2021
@jertel jertel closed this as completed Jul 27, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants