Fix WeChat issue #1229

songjiayang · 2018-02-07T11:56:33Z

For issue #1218

Changes:

change weChatCreateMessage and weChatCloseMessage to weChatMessage
cache WeChat AccessToken with accessTokenAt
code refactor & format.

Test config:

global:
  resolve_timeout: 5m
  wechat_api_secret: 'xxxx'

route:
  group_by: ['alertname']
  receiver: 'wechat'

receivers:
- name: 'wechat'
  wechat_configs:
  - corp_id: 'xxxx'
    to_party: '1'
    agent_id: 'xxx'

Work with this config.

stuartnelson3 · 2018-02-08T14:02:20Z

notify/impl.go

+
+	var accessToken string
+	// Refresh AccessToken over 2 hours
+	if n.accessToken != "" && time.Now().Sub(n.accessTokenAt) < 110*time.Minute {


You can change 110*time.Minute to 2*time.Hour

This could be changed to

if n.accessToken == "" || time.Now().Sub(n.accessTokenAt) > 2*time.Hour { // refresh token // Set n.accessToken and n.AccessTokenAt } // Down below, use n.accessToken directly

stuartnelson3 · 2018-02-08T14:03:46Z

notify/impl.go

+
+		u.RawQuery = parameters.Encode()
+
+		level.Debug(n.logger).Log("msg", "Sending Wechat  message", "incident", key, "url", u.String())


s/Sending/sending/
There is an extra space between Wechat and message that can be removed.

stuartnelson3 · 2018-02-08T14:05:48Z

notify/impl.go

+
+		level.Debug(n.logger).Log("msg", "Sending Wechat  message", "incident", key, "url", u.String())
+
+		resp, err := ctxhttp.Get(ctx, http.DefaultClient, u.String())


Can you replace this with

res, err := http.NewRequest("GET", u.String(), nil) if err != nil { return true, err } resp, err := http.DefaulClient.Do(req.WithContext(ctx))

I want to start removing the ctxhttp library :)

stuartnelson3 · 2018-02-08T14:13:29Z

notify/impl_test.go

 	"io/ioutil"
+	"net/url"


Both "io/ioutil" and "net/url" should be in the first block of imports in this list, with the other stdlib imports.

stuartnelson3 · 2018-02-08T14:14:32Z

Thanks for the contribution! Few easy changes and then this should be good to be merged

songjiayang · 2018-02-08T20:21:59Z

demo to show:

@stuartnelson3 review again please.

stuartnelson3 · 2018-02-09T09:23:37Z

config/notifiers.go

-		ToUser:    `{{ template "wechat.default.to_user" . }}`,
-		ToParty:   `{{ template "wechat.default.to_party" . }}`,
-		ToTag:     `{{ template "wechat.default.to_tag" . }}`,
-		AgentID:   `{{ template "wechat.default.agent_id" . }}`,


Why were all these fields removed?

I think these fields should be a determined value.

stuartnelson3 · 2018-02-09T09:26:16Z

config/notifiers.go

-		AgentID:   `{{ template "wechat.default.agent_id" . }}`,
-		// TODO: Add a details field with all the alerts.
+		Message: `{{ template "wechat.default.message" . }}`,
+		APIURL:  DefaultGlobalConfig.WeChatAPIURL,


It would be good to leave this as a templated value for users. There is the check in https://github.com/prometheus/alertmanager/pull/1229/files#diff-3baf47c64847a8fb8aaa8cc2e088513bR250 that will set the api url to the global default.

ok, I will check it again.

songjiayang · 2018-02-11T09:24:03Z

fixed

…theus#1229) Adds a new label called "type" systemd_unit_state which contains the Type field from the unit file. This applies only to the .service and .mount unit types. The other unit types do not include the optional type field. Fixes prometheus#1210 Signed-off-by: Paul Gier <[email protected]>

songjiayang force-pushed the master branch 2 times, most recently from 552bce8 to 0054ed5 Compare February 7, 2018 12:23

fix wechat issue

48b8b8e

songjiayang force-pushed the master branch from 0054ed5 to 48b8b8e Compare February 8, 2018 01:58

stuartnelson3 suggested changes Feb 8, 2018

View reviewed changes

songjiayang force-pushed the master branch from 0f1fb0a to 9063500 Compare February 8, 2018 20:20

songjiayang force-pushed the master branch 2 times, most recently from 5e20d85 to 0da153d Compare February 8, 2018 21:00

stuartnelson3 reviewed Feb 9, 2018

View reviewed changes

wechat issue code review

68dea87

songjiayang force-pushed the master branch from 0da153d to 68dea87 Compare February 11, 2018 08:19

stuartnelson3 approved these changes Feb 11, 2018

View reviewed changes

stuartnelson3 merged commit d07a072 into prometheus:master Feb 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix WeChat issue #1229

Fix WeChat issue #1229

songjiayang commented Feb 7, 2018 •

edited

Loading

stuartnelson3 Feb 8, 2018

stuartnelson3 Feb 8, 2018

stuartnelson3 Feb 8, 2018

stuartnelson3 Feb 8, 2018

stuartnelson3 Feb 8, 2018

stuartnelson3 commented Feb 8, 2018

songjiayang commented Feb 8, 2018 •

edited

Loading

stuartnelson3 Feb 9, 2018

songjiayang Feb 9, 2018

stuartnelson3 Feb 9, 2018

songjiayang Feb 9, 2018

songjiayang commented Feb 11, 2018


		u.RawQuery = parameters.Encode()

		level.Debug(n.logger).Log("msg", "Sending Wechat message", "incident", key, "url", u.String())


		level.Debug(n.logger).Log("msg", "Sending Wechat message", "incident", key, "url", u.String())

		resp, err := ctxhttp.Get(ctx, http.DefaultClient, u.String())

Fix WeChat issue #1229

Fix WeChat issue #1229

Conversation

songjiayang commented Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartnelson3 commented Feb 8, 2018

songjiayang commented Feb 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

songjiayang commented Feb 11, 2018

songjiayang commented Feb 7, 2018 •

edited

Loading

songjiayang commented Feb 8, 2018 •

edited

Loading