Fix #432: Uses class member vars on omited start/end time vars

[thican]

Signed-off-by: Thibaud CANALE thican@thican.net

Description

Merge request to fix issue raised in discussion #432.
I forgot to update variables starttime and endtime with their class member counterparts.

Checklist

• [Y] I have reviewed the contributing guidelines.
• [N] I have included unit tests for my changes or additions.
• [N] I have successfully run make test-docker with my changes.
• [N] I have updated the documentation.
• [N] I have updated the changelog.

[thican]

To comply with the project's CI, I just rebased this commit from branch dev/fix_test_rule_endtime on master instead of keeping it on previous branch, keeping its history (like I used to do on other projects).
I hope it will fix the issue, once again sorry I am unable to test it.

[nsano-rururu]

This is my personal opinion. ..
I think it is important to add test code, but in the future, I will also check whether the changed code part has been checked for operation, and if not, check the operation and submit the result. I think you need to do it.
I think it's a problem that I noticed if I checked the operation in the first place.

[thican]

This is my personal opinion. ..
I think it is important to add test code, but in the future, I will also check whether the changed code part has been checked for operation, and if not, check the operation and submit the result. I think you need to do it.
I think it's a problem that I noticed if I checked the operation in the first place.

It's not I don't want, I wish I could, unfortunately I can't.
Where I work I am unable to run the tests with Docker (proxy blocks requests), and at my place I don't have the environment to test (and currently I am also not at my place, hence I just created a commit without testing it).

[nsano-rururu]

It's not I don't want, I wish I could, unfortunately I can't.
Where I work I am unable to run the tests with Docker (proxy blocks requests), and at my place I don't have the environment to test (and currently I am also not at my place, hence I just created a commit without testing it).

different. make test-It's not about docker, it's a normal operation check. I want to make it mandatory for future pull requests. It's impossible that you haven't confirmed the operation.

[nsano-rururu]

After changing it, install it with python setup.py install and can't check the operation? ..

[nsano-rururu]

I haven't really checked the operation, so please check out the pull request locally and check the operation before merging, and I think I should have commented on the method for checking the operation.

[nsano-rururu]

I'm not angry, I just thought about how to prevent this from happening in the future.

[thican]

I once ran make test, however it raised issues out of the scope of my contributions or complaining about failing fetching content over our (blocking) corporate proxy.

And when I want to run manually a test_rule:

python3.9 elastalert/test_rule.py --config ../project_alert/config/our_config.yaml --save-json ../project_alert/data/our_data.json ../project_alert/rules/our_rule.yaml
Traceback (most recent call last):
  File "/path/to/elastalert2/elastalert/test_rule.py", line 15, in <module>
    from elastalert.config import load_conf
  File "/path/to/elastalert2/elastalert/elastalert.py", line 32, in <module>
    from elastalert import kibana
ImportError: cannot import name 'kibana' from partially initialized module 'elastalert' (most likely due to a circular import) (/path/to/elastalert2/elastalert/elastalert.py)

Tested from a pyenv and venv environment with all the dependencies from requirements-dev.txt.
I didn't report back this issue because it's not related to the issue I am concerned about.

About setup.py, I want to avoid calling it, my goal is to test from its repository.

[nsano-rururu]

Is it possible to wait and do it from home? .. Company? .. Do you mean you're doing it during business hours?

[nsano-rururu]

For the time being, I understood that I noticed if I checked the operation with the parameters "--start" and "--end".

[nsano-rururu]

About setup.py, I want to avoid calling it, my goal is to test from its repository.

That's the execution of test code, right? .. What I wanted to say was that I wanted the modified code to work locally.

[ferozsalam]

I can test this some time in the next 24h if no one gets to it first.

[nsano-rururu]

I can test this some time in the next 24h if no one gets to it first.

I appreciate your cooperation.

[thican]

Hello again,
Indeed, I am fixing this issue during business hours because we use this module for our tests and alerting. However this is also not my task, I am doing this while I am not supposed to, because I want to make my tests working, which is my main task.
And at my place, I don't have my corporate environment to test on.

Meanwhile I decided to test python setup.py install and fortunately it was better than I expected, it copied itself in my venv instead of my system, which is great for me.
Unfortunately, I met an issue while executing elastalert-test-rule, both with previous master (commit 50e591c) and my current MR (commit 02ab7e3):

elastalert-test-rule --config ../alerting/config/gcp.yaml --save-json ../alerting/data/qac.json ../alerting/rules/qac_license_unavailable.yml
INFO:elastalert:Note: In debug mode, alerts will be logged to console but NOT actually sent.
            To send them but remain verbose, use --verbose instead.

*DISCARDED FOR PRIVACY*

Traceback (most recent call last):
  File "/path/to/elastalert2/venv/bin/elastalert-test-rule", line 33, in <module>
    sys.exit(load_entry_point('elastalert2==2.2.1', 'console_scripts', 'elastalert-test-rule')())
  File "/path/to/elastalert2/venv/lib/python3.9/site-packages/elastalert2-2.2.1-py3.9.egg/elastalert/test_rule.py", line 455, in main
    test_instance.run_rule_test()
  File "/path/to/elastalert2/venv/lib/python3.9/site-packages/elastalert2-2.2.1-py3.9.egg/elastalert/test_rule.py", line 447, in run_rule_test
    self.run_elastalert(rule_yaml, conf, args)
  File "/path/to/elastalert2/venv/lib/python3.9/site-packages/elastalert2-2.2.1-py3.9.egg/elastalert/test_rule.py", line 318, in run_elastalert
    client.run_rule(rule, endtime, starttime)
  File "/path/to/elastalert2/venv/lib/python3.9/site-packages/elastalert2-2.2.1-py3.9.egg/elastalert/elastalert.py", line 899, in run_rule
    for x in range(len(rule['agg_matches'])):
KeyError: 'agg_matches'

I guess this proof is good enough at least to tell the issue raised in MR #432 has been fixed.

Best regards,

[nsano-rururu]

@thican

The KeyError is probably because the first error prevented some setup code from running.
There may be a problem in building the environment.
I will also check the operation. No further investigation is needed for now.

[nsano-rururu]

@thican

elasticseearch is 7.14, and it is running with docker.
I ran the test rule with the current master and before you made a pull request. You can see the error just by moving it normally. Also, there is no problem before you make a pull request.

Current master

(myvenv) [CORP\sano@a-ngft53r34ong work]$ git clone https://github.com/jertel/elastalert2.git
(myvenv) [CORP\sano@a-ngft53r34ong work]$ cd elastalert2/
(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ pip install "setuptools>=11.3"
(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ elastalert-create-index
Elastic Version: 7.14.0
Reading Elastic 6 index mappings:
Reading index mapping 'es_mappings/6/silence.json'
Reading index mapping 'es_mappings/6/elastalert_status.json'
Reading index mapping 'es_mappings/6/elastalert.json'
Reading index mapping 'es_mappings/6/past_elastalert.json'
Reading index mapping 'es_mappings/6/elastalert_error.json'
New index elastalert_status created
Done!
(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ elastalert-test-rule examples/rules/a.yaml
INFO:elastalert:Note: In debug mode, alerts will be logged to console but NOT actually sent.
            To send them but remain verbose, use --verbose instead.
Traceback (most recent call last):
  File "/home/sano/myvenv/bin/elastalert-test-rule", line 11, in <module>
    load_entry_point('elastalert2==2.2.1', 'console_scripts', 'elastalert-test-rule')()
  File "/home/sano/myvenv/lib/python3.7/site-packages/elastalert2-2.2.1-py3.7.egg/elastalert/test_rule.py", line 518, in main
    test_instance.run_rule_test()
  File "/home/sano/myvenv/lib/python3.7/site-packages/elastalert2-2.2.1-py3.7.egg/elastalert/test_rule.py", line 464, in run_rule_test
    rule_yaml = conf['rules_loader'].load_yaml(self.args.file)
  File "/home/sano/myvenv/lib/python3.7/site-packages/elastalert2-2.2.1-py3.7.egg/elastalert/loaders.py", line 236, in load_yaml
    loaded = self.get_yaml(filename)
  File "/home/sano/myvenv/lib/python3.7/site-packages/elastalert2-2.2.1-py3.7.egg/elastalert/loaders.py", line 578, in get_yaml
    return read_yaml(filename)
  File "/home/sano/myvenv/lib/python3.7/site-packages/elastalert2-2.2.1-py3.7.egg/elastalert/yaml.py", line 6, in read_yaml
    with open(path) as f:
FileNotFoundError: [Errno 2] No such file or directory: 'examples/rules/BaseRule.config'
(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ elastalert-test-rule examples/rules/a.yaml
INFO:elastalert:Note: In debug mode, alerts will be logged to console but NOT actually sent.
            To send them but remain verbose, use --verbose instead.
Got 192 hits from the last 0 day

Available terms in first hit:
	message
	@timestamp
	@log_name

Traceback (most recent call last):
  File "/home/sano/myvenv/bin/elastalert-test-rule", line 11, in <module>
    load_entry_point('elastalert2==2.2.1', 'console_scripts', 'elastalert-test-rule')()
  File "/home/sano/myvenv/lib/python3.7/site-packages/elastalert2-2.2.1-py3.7.egg/elastalert/test_rule.py", line 518, in main
    test_instance.run_rule_test()
  File "/home/sano/myvenv/lib/python3.7/site-packages/elastalert2-2.2.1-py3.7.egg/elastalert/test_rule.py", line 508, in run_rule_test
    self.run_elastalert(rule_yaml, conf)
  File "/home/sano/myvenv/lib/python3.7/site-packages/elastalert2-2.2.1-py3.7.egg/elastalert/test_rule.py", line 402, in run_elastalert
    conf['run_every'] = endtime - starttime
UnboundLocalError: local variable 'endtime' referenced before assignment

Merge pull request ＃415 from Neuro-HSOC/markdown-summary-table

(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ git switch -d 50e591cb479987ca3106a1dd988f3c2e4743cb02
HEAD is now at 50e591c Merge pull request #415 from Neuro-HSOC/markdown-summary-table
(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ python setup.py install
(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ elastalert-test-rule examples/rules/a.yaml
INFO:elastalert:Note: In debug mode, alerts will be logged to console but NOT actually sent.
            To send them but remain verbose, use --verbose instead.
Didn't get any results.
INFO:elastalert:Note: In debug mode, alerts will be logged to console but NOT actually sent.
                To send them but remain verbose, use --verbose instead.
1 rules loaded
INFO:apscheduler.scheduler:Adding job tentatively -- it will be properly scheduled when the scheduler starts
INFO:elastalert:Queried rule a from 2021-08-31 23:09 JST to 2021-08-31 23:14 JST: 10 / 10 hits
INFO:elastalert:Alert for a at 2021-08-31T14:10:12.934895Z:
INFO:elastalert:Test 		    54 Quit	  123 bb

At least 2 events occurred between 2021-08-31 23:05 JST and 2021-08-31 23:10 JST

@log_name: mysqld.general
@timestamp: 2021-08-31T14:10:12.934895Z
_id: tcGMnHsB0_4bCg91UE4c
_index: mariadblog-2021.08.31
_type: _doc
message: 		    54 Quit	
num_hits: 10
num_matches: 5

INFO:elastalert:Ignoring match for silenced rule a
INFO:elastalert:Ignoring match for silenced rule a
INFO:elastalert:Ignoring match for silenced rule a
INFO:elastalert:Ignoring match for silenced rule a

Would have written the following documents to writeback index (default is elastalert_status):

silence - {'exponent': 0, 'rule_name': 'a', '@timestamp': datetime.datetime(2021, 8, 31, 14, 14, 31, 622578, tzinfo=tzutc()), 'until': datetime.datetime(2021, 8, 31, 14, 19, 31, 622568, tzinfo=tzutc())}

elastalert_status - {'rule_name': 'a', 'endtime': datetime.datetime(2021, 8, 31, 14, 14, 31, 547254, tzinfo=tzutc()), 'starttime': datetime.datetime(2021, 8, 31, 14, 9, 28, 547254, tzinfo=tzutc()), 'matches': 5, 'hits': 10, '@timestamp': datetime.datetime(2021, 8, 31, 14, 14, 31, 624028, tzinfo=tzutc()), 'time_taken': 0.022794008255004883}

[nsano-rururu]

@thican

I brought the one you are currently issuing a pull request locally and checked the operation. There is no problem.

(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ git fetch origin pull/433/head:fix_test_rule_endtime
(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ python setup.py install
(myvenv) [CORP\sano@a-ngft53r34ong elastalert2]$ elastalert-test-rule examples/rules/a.yaml
INFO:elastalert:Note: In debug mode, alerts will be logged to console but NOT actually sent.
            To send them but remain verbose, use --verbose instead.
Didn't get any results.
INFO:elastalert:Note: In debug mode, alerts will be logged to console but NOT actually sent.
                To send them but remain verbose, use --verbose instead.
1 rules loaded
INFO:apscheduler.scheduler:Adding job tentatively -- it will be properly scheduled when the scheduler starts
INFO:elastalert:Queried rule a from 2021-08-31 23:25 JST to 2021-08-31 23:30 JST: 8 / 8 hits
INFO:elastalert:Alert for a at 2021-08-31T14:26:20.923736Z:
INFO:elastalert:Test 		    86 Quit	  123 bb

At least 2 events occurred between 2021-08-31 23:21 JST and 2021-08-31 23:26 JST

@log_name: mysqld.general
@timestamp: 2021-08-31T14:26:20.923736Z
_id: 9cGanHsB0_4bCg91pk43
_index: mariadblog-2021.08.31
_type: _doc
message: 		    86 Quit	
num_hits: 8
num_matches: 4

INFO:elastalert:Ignoring match for silenced rule a
INFO:elastalert:Ignoring match for silenced rule a
INFO:elastalert:Ignoring match for silenced rule a

Would have written the following documents to writeback index (default is elastalert_status):

silence - {'exponent': 0, 'rule_name': 'a', '@timestamp': datetime.datetime(2021, 8, 31, 14, 30, 31, 309085, tzinfo=tzutc()), 'until': datetime.datetime(2021, 8, 31, 14, 35, 31, 309066, tzinfo=tzutc())}

elastalert_status - {'rule_name': 'a', 'endtime': datetime.datetime(2021, 8, 31, 14, 30, 31, 188195, tzinfo=tzutc()), 'starttime': datetime.datetime(2021, 8, 31, 14, 25, 28, 188195, tzinfo=tzutc()), 'matches': 4, 'hits': 8, '@timestamp': datetime.datetime(2021, 8, 31, 14, 30, 31, 313111, tzinfo=tzutc()), 'time_taken': 0.03411078453063965}

[nsano-rururu]

@jertel

Is there anything else you would like to do?
If there is no problem, merge it.

[thican]

Sorry, I didn't understand the goal of your test @nsano-rururu.

We know and we all agreed current master is broken because of my previous MR.
Hence why there is a new MR -- this precise one -- to fix the bug I created.

Your test should point to my current branch, which contains the fix.

[jertel]

Nothing else needed. You can approve and merge it. Thank you @nsano-rururu for the thorough review, and @thican for the quick fix.

[thican]

You can discard my last comment, we cross commented therefore I didn't see your last response.

[thican]

Thanks for your reviews.