Wechat alerter by daiwei233 · Pull Request #355 · jertel/elastalert2

daiwei233 · 2021-07-21T08:08:16Z

Description

Wechat alerter support, see: #280

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 updated the documentation.
I have updated the changelog.

Signed-off-by: daiwei <daiwei233@outlook.com>

ferozsalam

Thanks for your contribution! I've left some minor comments and suggestions.

ferozsalam · 2021-07-21T08:46:52Z

+
+Optional:
+
+``wechat_to_party`` & ``wechat_to_user`` & ``wechat_to_tag``: There must be one.


I would move this to the Required section and rephrase to:

'One of wechat_to_party, wechat_to_user, or wechat_to_tag'

ferozsalam · 2021-07-21T08:49:32Z

+
+``wechat_to_party`` & ``wechat_to_user`` & ``wechat_to_tag``: There must be one.
+
+``wechat_msgtype``: Wechat msgtype, default to ``text``. ``textcard``, ``markdown``.


nit: 'default to' should be 'defaults to' here and on the next four items.

Suggested change

``wechat_msgtype``: Wechat msgtype, default to ``text``. ``textcard``, ``markdown``.

``wechat_msgtype``: Wechat msgtype, defaults to ``text``. ``textcard``, ``markdown``.

ferozsalam · 2021-07-21T08:49:46Z

+
+``wechat_msgtype``: Wechat msgtype, default to ``text``. ``textcard``, ``markdown``.
+
+``wechat_textcard_url``: Wechat textcard url while ``wechat_msgtype`` is ``textcard``, default to ``null_url``.


Suggested change

``wechat_textcard_url``: Wechat textcard url while ``wechat_msgtype`` is ``textcard``, default to ``null_url``.

``wechat_textcard_url``: Wechat textcard url while ``wechat_msgtype`` is ``textcard``, defaults to ``null_url``.

ferozsalam · 2021-07-21T08:49:56Z

+
+``wechat_textcard_url``: Wechat textcard url while ``wechat_msgtype`` is ``textcard``, default to ``null_url``.
+
+``wechat_enable_duplicate_check``: ``enable_duplicate_check``, default to ``0``.


Suggested change

``wechat_enable_duplicate_check``: ``enable_duplicate_check``, default to ``0``.

``wechat_enable_duplicate_check``: ``enable_duplicate_check``, defaults to ``0``.

ferozsalam · 2021-07-21T08:50:04Z

+
+``wechat_enable_duplicate_check``: ``enable_duplicate_check``, default to ``0``.
+
+``wechat_duplicate_check_interval``: ``duplicate_check_interval``, default to ``1800``.


Suggested change

``wechat_duplicate_check_interval``: ``duplicate_check_interval``, default to ``1800``.

``wechat_duplicate_check_interval``: ``duplicate_check_interval``, defaults to ``1800``.

ferozsalam · 2021-07-21T08:53:41Z

+def test_wechat_required_error(wechat_corp_id, wechat_secret, wechat_agent_id, expected_data):
+    try:
+        rule = {
+            'name': 'Test DingTalk Rule',


Suggested change

'name': 'Test DingTalk Rule',

'name': 'Test Wechat Rule',

ferozsalam · 2021-07-21T08:54:00Z

+            'name': 'Test DingTalk Rule',
+            'type': 'any',
+            'alert': [],
+            'alert_subject': 'Test DingTalk'


Suggested change

'alert_subject': 'Test DingTalk'

'alert_subject': 'Test Wechat'

Thank you for your advice.

Signed-off-by: daiwei <daiwei233@outlook.com>

Signed-off-by: daiwei233 <daiwei233@outlook.com>

nsano-rururu · 2021-07-21T09:47:25Z

Although it is a setting of schema.yaml, Wechat should be described between Twilio and Zabbix.

  ### Wechat
  wechat_corp_id: {type: string}
  wechat_secret: {type: string}
  wechat_agent_id: {type: integer}

Signed-off-by: daiwei <daiwei233@outlook.com>

daiwei233 · 2021-07-21T09:52:51Z

Although it is a setting of schema.yaml, Wechat should be described between Twilio and Zabbix.
  ### Wechat
  wechat_corp_id: {type: string}
  wechat_secret: {type: string}
  wechat_agent_id: {type: integer}

ok.

nsano-rururu · 2021-07-21T10:03:43Z

Bring the latest information on jertel: master into your repository before adding change logs.
In 2.x.x New features
https://github.com/jertel/elastalert2/blob/master/CHANGELOG.md

Signed-off-by: daiwei <daiwei233@outlook.com>

daiwei233 · 2021-07-21T11:36:56Z

master

Done.

jertel · 2021-07-21T13:13:56Z

+
+``wechat_secret``:  The wechat agent secret.
+
+``wechat_to_party`` & ``wechat_to_user`` & ``wechat_to_tag``: One of wechat_to_party, wechat_to_user, or wechat_to_tag.


Should change the & to | to avoid confusion since not all are required.

jertel · 2021-07-21T13:17:49Z

+        self.wechat_message_url = 'https://qyapi.weixin.qq.com/cgi-bin/message/send?access_token={}'
+        self.expires_in = datetime.datetime.now() - datetime.timedelta(seconds=3600)
+
+    def get_token(self):


This function could use some unit test coverage.

jertel · 2021-07-21T13:18:09Z

+        self.wechat_access_token = token_json['access_token']
+        self.expires_in = datetime.datetime.now() + datetime.timedelta(seconds=token_json['expires_in'])
+
+    def format_body(self, body):


Could use a simple unit test.

jertel · 2021-07-21T13:20:35Z

  twilio_message_service_sid: {type: string}
  twilio_use_copilot: {type: boolean}

+  ### Wechat


There should be more schema definitions listed here since this alerter has a large number of input parameters.

jertel

Thanks for the new alerter! This is looking good. I added some code review feedback, primarily around the need for more unit test coverage, and also a couple minor updates. The team has been working hard to get to 100% test coverage so I'm hesitant to merge anything that moves us backwards on the test coverage.

nsano-rururu · 2021-07-21T13:48:27Z

@daiwei233

It is recommended to define all in schema.yaml. When the rule is executed, jsonschema will perform validation check with the settings described in schema.yaml and generate an error.

example

  ### Wechat
  wechat_corp_id: {type: string}
  wechat_secret: {type: string}
  wechat_agent_id: {type: integer}
  wechat_msgtype: {type: string, enum: ['text', 'textcard', 'markdown']}
  wechat_to_party: {type: string}
  wechat_to_user: {type: string}
  wechat_to_tag: {type: string}
  wechat_textcard_url: {type: string}
  wechat_enable_duplicate_check: {type: integer}
  wechat_duplicate_check_interval: {type: integer}
  wechat_proxy: {type: string}
  wechat_proxy_login: {type: string}
  wechat_proxy_pass: {type: string}

nsano-rururu · 2021-07-21T13:57:23Z

@daiwei233

There is an error in flake8. Did you really run make test-docker and make sure there are no errors?

./tests/alerters/wechat_test.py:1:1: F401 'json' imported but unused
./tests/alerters/wechat_test.py:51:1: W293 blank line contains whitespace
./tests/alerters/wechat_test.py:53:34: E225 missing whitespace around operator
./tests/alerters/wechat_test.py:65:40: W292 no newline at end of file
ERROR: InvocationError for command /home/elastalert/.tox/py39/bin/flake8 . (exited with code 1)

nsano-rururu · 2021-07-21T14:13:26Z

@jertel

The overall coverage is 67% because the pull request was made without merging the latest repository state,
When I checked out the pull request locally, merged the latest latest repository state and ran the test, the overall coverage was 72%, so the overall coverage is not degraded.
The coverage of wechat.py was 41%.

wechat.py       41%   40-56, 59, 62-107

@daiwei233

Is it possible to add unimplemented test code?

nsano-rururu · 2021-07-21T14:23:12Z

@daiwei233

Wechat is mentioned in ruletypes.rst and elastalert.rst, but when I look at the official website, c seems to be an uppercase C. Officially WeChat

nsano-rururu · 2021-07-21T14:36:53Z

@daiwei233

Do you need the following functions on lines 58 and 59? .. It doesn't seem to be used.

wechat.py

    def format_body(self, body):
        return body.encode('utf8')

nsano-rururu · 2021-07-21T19:08:05Z

example

84% 41, 47-48, 53, 59, 63, 72, 95->101, 105-106

wechat_test.py

import logging
import pytest

from unittest import mock

from elastalert.loaders import FileRulesLoader
from elastalert.alerters.wechat import WechatAlerter


def test_wechat_getinfo():
    rule = {
        'name': 'Test Wechat Alerter',
        'type': 'any',
        'wechat_corp_id': 'test_wechat_corp_id',
        'wechat_secret': 'test_wechat_secret',
        'wechat_agent_id': 'test_wechat_agent_id',
        'alert': [],
        'alert_subject': 'Test Wechat Alert'
    }
    rules_loader = FileRulesLoader({})
    rules_loader.load_modules(rule)
    alert = WechatAlerter(rule)

    expected_data = {'type': 'wechat'}
    actual_data = alert.get_info()
    assert expected_data == actual_data


@pytest.mark.parametrize('wechat_corp_id, wechat_secret, wechat_agent_id, expected_data', [
    ('', '', '', 'Missing required option(s): wechat_corp_id, wechat_secret, wechat_agent_id'),
    ('xxxx1', '', '', 'Missing required option(s): wechat_corp_id, wechat_secret, wechat_agent_id'),
    ('', 'xxxx2', '', 'Missing required option(s): wechat_corp_id, wechat_secret, wechat_agent_id'),
    ('xxxx1', 'xxxx2', '', 'Missing required option(s): wechat_corp_id, wechat_secret, wechat_agent_id'),
    ('xxxx1', '', 'xxxx3', 'Missing required option(s): wechat_corp_id, wechat_secret, wechat_agent_id'),
    ('', 'xxxx2', 'xxxx3', 'Missing required option(s): wechat_corp_id, wechat_secret, wechat_agent_id'),
    ('xxxx1', 'xxxx2', 'xxxx3',
        {
            'type': 'wechat'
        }),
])
def test_wechat_required_error(wechat_corp_id, wechat_secret, wechat_agent_id, expected_data):
    try:
        rule = {
            'name': 'Test Wechat Rule',
            'type': 'any',
            'alert': [],
            'alert_subject': 'Test Wechat'
        }

        if wechat_corp_id:
            rule['wechat_corp_id'] = wechat_corp_id

        if wechat_secret:
            rule['wechat_secret'] = wechat_secret

        if wechat_agent_id:
            rule['wechat_agent_id'] = wechat_agent_id

        rules_loader = FileRulesLoader({})
        rules_loader.load_modules(rule)
        alert = WechatAlerter(rule)

        actual_data = alert.get_info()
        assert expected_data == actual_data
    except Exception as ea:
        assert expected_data in str(ea)


def test_wechat_msgtype_text(caplog):
    caplog.set_level(logging.INFO)
    rule = {
        'name': 'Test WeChat Alerter',
        'type': 'any',
        'wechat_corp_id': 'test_wechat_corp_id',
        'wechat_secret': 'test_wechat_secret',
        'wechat_agent_id': 'test_wechat_agent_id',
        'wechat_to_user': 'user',
        'alert': [],
        'alert_subject': 'Test Wechat Alert'
    }
    rules_loader = FileRulesLoader({})
    rules_loader.load_modules(rule)
    alert = WechatAlerter(rule)
    match = {
        '@timestamp': '2021-01-01T00:00:00',
        'somefield': 'foobarbaz'
    }
    with mock.patch('requests.get') as mock_get_request:
        with mock.patch('requests.post') as mock_post_request:
            mock_response = mock.Mock()
            mock_response.json.return_value = {'access_token': 'xxxx', 'expires_in': 3600}
            mock_response.status_code = 200
            mock_get_request.return_value = mock_response
            alert.alert([match])

    mock_post_request.assert_called_once_with(
        'https://qyapi.weixin.qq.com/cgi-bin/message/send?access_token=xxxx',
        data=mock.ANY,
        headers={'content-type': 'application/json'},
        proxies=None,
        auth=None
    )

    expected_data = '{"touser": "user", "toparty": "", "totag": "", \
"agentid": "test_wechat_agent_id", "enable_duplicate_check": 0, \
"duplicate_check_interval": 1800, "msgtype": "text", "text": \
{"content": \
"Test WeChat Alerter\\n\\n@timestamp: 2021-01-01T00:00:00\\nsomefield: foobarbaz\\n"}}'

    actual_data = mock_post_request.call_args_list[0][1]['data']
    assert expected_data == actual_data
    assert ('elastalert', logging.INFO, 'Alert sent to wechat.') == caplog.record_tuples[0]


def test_wechat_msgtype_textcard(caplog):
    caplog.set_level(logging.INFO)
    rule = {
        'name': 'Test WeChat Alerter',
        'type': 'any',
        'wechat_corp_id': 'test_wechat_corp_id',
        'wechat_secret': 'test_wechat_secret',
        'wechat_agent_id': 'test_wechat_agent_id',
        'wechat_to_user': 'user',
        'wechat_msgtype': 'textcard',
        'alert': [],
        'alert_subject': 'Test Wechat Alert'
    }
    rules_loader = FileRulesLoader({})
    rules_loader.load_modules(rule)
    alert = WechatAlerter(rule)
    match = {
        '@timestamp': '2021-01-01T00:00:00',
        'somefield': 'foobarbaz'
    }
    with mock.patch('requests.get') as mock_get_request:
        with mock.patch('requests.post') as mock_post_request:
            mock_response = mock.Mock()
            mock_response.json.return_value = {'access_token': 'xxxx', 'expires_in': 3600}
            mock_response.status_code = 200
            mock_get_request.return_value = mock_response
            alert.alert([match])

    mock_post_request.assert_called_once_with(
        'https://qyapi.weixin.qq.com/cgi-bin/message/send?access_token=xxxx',
        data=mock.ANY,
        headers={'content-type': 'application/json'},
        proxies=None,
        auth=None
    )

    expected_data = '{"touser": "user", "toparty": "", "totag": "", \
"agentid": "test_wechat_agent_id", "enable_duplicate_check": 0, \
"duplicate_check_interval": 1800, "msgtype": "textcard", \
"textcard": {"title": "Test Wechat Alert", \
"description": "Test WeChat Alerter\\n\\n@timestamp: 2021-01-01T00:00:00\\nsomefield: foobarbaz\\n", \
"url": "null_url"}}'

    actual_data = mock_post_request.call_args_list[0][1]['data']
    assert expected_data == actual_data
    assert ('elastalert', logging.INFO, 'Alert sent to wechat.') == caplog.record_tuples[0]


def test_wechat_msgtype_markdown(caplog):
    caplog.set_level(logging.INFO)
    rule = {
        'name': 'Test WeChat Alerter',
        'type': 'any',
        'wechat_corp_id': 'test_wechat_corp_id',
        'wechat_secret': 'test_wechat_secret',
        'wechat_agent_id': 'test_wechat_agent_id',
        'wechat_to_user': 'user',
        'wechat_msgtype': 'markdown',
        'alert': [],
        'alert_subject': 'Test Wechat Alert'
    }
    rules_loader = FileRulesLoader({})
    rules_loader.load_modules(rule)
    alert = WechatAlerter(rule)
    match = {
        '@timestamp': '2021-01-01T00:00:00',
        'somefield': 'foobarbaz'
    }
    with mock.patch('requests.get') as mock_get_request:
        with mock.patch('requests.post') as mock_post_request:
            mock_response = mock.Mock()
            mock_response.json.return_value = {'access_token': 'xxxx', 'expires_in': 3600}
            mock_response.status_code = 200
            mock_get_request.return_value = mock_response
            alert.alert([match])

    mock_post_request.assert_called_once_with(
        'https://qyapi.weixin.qq.com/cgi-bin/message/send?access_token=xxxx',
        data=mock.ANY,
        headers={'content-type': 'application/json'},
        proxies=None,
        auth=None
    )

    expected_data = '{"touser": "user", "toparty": "", "totag": "", \
"agentid": "test_wechat_agent_id", "enable_duplicate_check": 0, \
"duplicate_check_interval": 1800, "msgtype": "markdown", \
"markdown": {"content": "Test WeChat Alerter\\n\\n@timestamp: 2021-01-01T00:00:00\\nsomefield: foobarbaz\\n"}}'

    actual_data = mock_post_request.call_args_list[0][1]['data']
    assert expected_data == actual_data
    assert ('elastalert', logging.INFO, 'Alert sent to wechat.') == caplog.record_tuples[0]

daiwei233 · 2021-07-22T01:44:23Z

Thank you guys, I will make it better! (That is my first pull request.😅)

nsano-rururu · 2021-07-22T14:42:25Z

What is the string null_url in wechat_textcard_url? .. It's essential, so I thought it would be rough if I forced it.
https://work.weixin.qq.com/api/doc/90000/90135/90236

self.wechat_textcard_url = self.rule.get('wechat_textcard_url', 'null_url')

nsano-rururu · 2021-07-22T15:04:35Z

When msg_type is text and markdown, I think the following implementation is fine, but when msg_type is textcard, there seems to be a limit of 128 for title and 512 for description. Is this overlooked?
https://work.weixin.qq.com/api/doc/90000/90135/90236#%E6%96%87%E6%9C%AC%E5%8D%A1%E7%89%87%E6%B6%88%E6%81%AF

        if len(body) > 2048:
            body = body[:2045] + "..."

nsano-rururu · 2021-07-22T15:15:41Z

Although it is described in the document of wechat_proxy, I thought that it is better to add only supports https..

nsano-rururu · 2021-07-22T15:37:15Z

It is described in the document of wechat_enable_duplicate_check, but are the values that can be set 0 and 1? .. If so, should I mention setting 1 to enable it?

Wechat_duplicate_check_interval is described as within 4 hours (14400 in seconds?) According to the document, so it would be better to describe the maximum setting value in seconds. ..

``wechat_enable_duplicate_check``: ``enable_duplicate_check``, defaults to ``0``.

``wechat_duplicate_check_interval``: ``duplicate_check_interval``, defaults to ``1800``.

nsano-rururu · 2021-07-22T15:55:16Z

Wechat_enable_duplicate_check and wechat_duplicate_check_interval have default values when they are not specified in the api specifications, so it should be implemented to be included in the request only when specified.
I thought it would be good to implement it in the request only when any other settings are set.

current

       self.wechat_enable_duplicate_check = self.rule.get('wechat_enable_duplicate_check', 0)
       self.wechat_duplicate_check_interval = self.rule.get('wechat_duplicate_check_interval', 1800)

        payload = {
            'touser': self.wechat_to_user or '',
            'toparty': self.wechat_to_party or '',
            'totag': self.wechat_to_tag or '',
            'agentid': self.wechat_agent_id,
            'enable_duplicate_check': self.wechat_enable_duplicate_check,
            'duplicate_check_interval': self.wechat_duplicate_check_interval
        }

Fix

       self.wechat_enable_duplicate_check = self.rule.get('wechat_enable_duplicate_check', None)
       self.wechat_duplicate_check_interval = self.rule.get('wechat_duplicate_check_interval', None)

        payload = {
            'agentid': self.wechat_agent_id,
        }

        if self.wechat_to_user:
            payload['touser'] = self.wechat_to_user

        if self.wechat_to_party:
            payload['toparty'] = self.wechat_to_party

        if self.wechat_to_tag:
            payload['totag'] = self.wechat_to_tag

        if self.wechat_enable_duplicate_check:
            payload['enable_duplicate_check'] = self.wechat_enable_duplicate_check

        if self.wechat_duplicate_check_interval:
            payload['duplicate_check_interval'] = self.wechat_duplicate_check_interval

nsano-rururu · 2021-10-20T13:48:08Z

@daiwei233

What happened after that? .. Did you give up?

jertel · 2021-11-16T12:28:07Z

Closing PR due to lack of interest by OP.

weidai218169 added 5 commits July 21, 2021 12:05

Wechat alerter support.

4be0270

Signed-off-by: daiwei <daiwei233@outlook.com>

Wechat alerter support.

696c01a

Signed-off-by: daiwei <daiwei233@outlook.com>

Wechat alerter support.

224274a

Signed-off-by: daiwei <daiwei233@outlook.com>

Wechat alerter support.

62748c9

Signed-off-by: daiwei <daiwei233@outlook.com>

Wechat alerter support.

9857568

Signed-off-by: daiwei <daiwei233@outlook.com>

ferozsalam reviewed Jul 21, 2021

View reviewed changes

weidai218169 added 2 commits July 21, 2021 17:02

Wechat alerter support.

7c61964

Signed-off-by: daiwei <daiwei233@outlook.com>

Wechat alerter support.

23120e9

Signed-off-by: daiwei233 <daiwei233@outlook.com>

Wechat alerter support.

195d3d8

Signed-off-by: daiwei <daiwei233@outlook.com>

Wechat alerter support.

63df979

Signed-off-by: daiwei <daiwei233@outlook.com>

jertel reviewed Jul 21, 2021

View reviewed changes

jertel requested changes Jul 21, 2021

View reviewed changes

jertel marked this pull request as draft October 15, 2021 12:27

jertel closed this Nov 16, 2021

github-actions Bot locked as resolved and limited conversation to collaborators Feb 20, 2024


		Optional:

		``wechat_to_party`` & ``wechat_to_user`` & ``wechat_to_tag``: There must be one.


		``wechat_to_party`` & ``wechat_to_user`` & ``wechat_to_tag``: There must be one.

		``wechat_msgtype``: Wechat msgtype, default to ``text``. ``textcard``, ``markdown``.


		``wechat_msgtype``: Wechat msgtype, default to ``text``. ``textcard``, ``markdown``.

		``wechat_textcard_url``: Wechat textcard url while ``wechat_msgtype`` is ``textcard``, default to ``null_url``.


		``wechat_textcard_url``: Wechat textcard url while ``wechat_msgtype`` is ``textcard``, default to ``null_url``.

		``wechat_enable_duplicate_check``: ``enable_duplicate_check``, default to ``0``.


		``wechat_enable_duplicate_check``: ``enable_duplicate_check``, default to ``0``.

		``wechat_duplicate_check_interval``: ``duplicate_check_interval``, default to ``1800``.

	'alert_subject': 'Test DingTalk'
	'alert_subject': 'Test Wechat'


		``wechat_secret``: The wechat agent secret.

		``wechat_to_party`` & ``wechat_to_user`` & ``wechat_to_tag``: One of wechat_to_party, wechat_to_user, or wechat_to_tag.

Conversation

daiwei233 commented Jul 21, 2021

Description

Checklist

Uh oh!

ferozsalam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nsano-rururu commented Jul 21, 2021

Uh oh!

daiwei233 commented Jul 21, 2021

Uh oh!

nsano-rururu commented Jul 21, 2021

Uh oh!

daiwei233 commented Jul 21, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jertel left a comment

Choose a reason for hiding this comment

Uh oh!

nsano-rururu commented Jul 21, 2021

Uh oh!

nsano-rururu commented Jul 21, 2021

Uh oh!

nsano-rururu commented Jul 21, 2021

Uh oh!

nsano-rururu commented Jul 21, 2021

Uh oh!

nsano-rururu commented Jul 21, 2021

Uh oh!

nsano-rururu commented Jul 21, 2021

Uh oh!

daiwei233 commented Jul 22, 2021

Uh oh!

nsano-rururu commented Jul 22, 2021

Uh oh!

nsano-rururu commented Jul 22, 2021

Uh oh!

nsano-rururu commented Jul 22, 2021

Uh oh!

nsano-rururu commented Jul 22, 2021

Uh oh!

nsano-rururu commented Jul 22, 2021

Uh oh!

nsano-rururu commented Oct 20, 2021

Uh oh!

jertel commented Nov 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants