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

Allow text/plain emails #926

Closed
awaragi opened this issue Jul 24, 2017 · 15 comments
Closed

Allow text/plain emails #926

awaragi opened this issue Jul 24, 2017 · 15 comments

Comments

@awaragi
Copy link
Contributor

awaragi commented Jul 24, 2017

Currently the email notifier is hard coded for html emails only. Considering that support for text/plain can be achieved by checking if a Content-Type header is already specified or not, is that a good solution that I can create a pull request for? or is such support is completely out of question?

Snippet + pseudo code from impl.go

	for header, t := range n.conf.Headers {
		value, err := n.tmpl.ExecuteTextString(t, data)
		if err != nil {
			return false, fmt.Errorf("executing %q header template: %s", header, err)
		}
		fmt.Fprintf(wc, "%s: %s\r\n", header, mime.QEncoding.Encode("utf-8", value))
	}

     // CHECK IF CONTENT IS ALREADY SPECIFIED BEFORE FORCING HTML
     if(n.conf.Headers["Content-Type"] == nil) {
	fmt.Fprintf(wc, "Content-Type: text/html; charset=UTF-8\r\n")
     }
	fmt.Fprintf(wc, "Date: %s\r\n", time.Now().Format(time.RFC1123Z))
@brian-brazil
Copy link
Contributor

There's more to this, as the templating applied is for HTML.

@awaragi
Copy link
Contributor Author

awaragi commented Jul 24, 2017

I understand. My code simply means that if the Content-Type is specified, don't hardcode it as in "I know what I am doing". The HTML template is evaluate as normal but its output is not interpreted as HTML by the email client which is what my purpose is.

@brian-brazil
Copy link
Contributor

It'll be escaped as though it is HTML though, which is not what you want.

@awaragi
Copy link
Contributor Author

awaragi commented Jul 24, 2017

I tried it and plain new lines were outputted as they are. My text "template" came out correctly but the text/html content-type was forcing the email client to read it as html so the new lines were ignored but the source was correct.

@awaragi
Copy link
Contributor Author

awaragi commented Jul 26, 2017

If you are okay with the following logic, I will submit a pull request.

if the user sets a custom header for email notification of type Content-Type, assume the user knows what they are doing and skip setting Content-Type: text/html to avoid conflict.

This patch will not break existing systems nor will make any assumptions on the content of the email template.

@brian-brazil
Copy link
Contributor

As noted above, that change would be incorrect as the content would still be treated as html by the templating system. There's more to changing the content type than the header.

@matthiasr
Copy link

@brian-brazil On what basis should the correct templating be selected? How do you want text emails to be selected and templated?

@brian-brazil
Copy link
Contributor

Either we'd have separate text and html fields (which I believe we've had requests for in the past), or a bool to select whether it's text or html.

@awaragi
Copy link
Contributor Author

awaragi commented Jul 27, 2017

Fair enough.

What's the consensus if I can provide some coding time? I am personally for two fields as they are not mutually exclusive and actually I've always included both in any application I develop with email notifications.

So basically if HTML input is provided, it will be used. If TEXT input is provided it will be also included. Default behavior is maintained with HTML templates.

@matthiasr
Copy link

I also think it should be two fields. they are not mutually exclusive.

@doctorlector
Copy link

doctorlector commented Jul 28, 2017

Hi All,

I still cannot realize how I can apply my own custom html template for e-mail alert in alert-manager config. Currently, I receive an empty e-mail alert.

Can you help me with that?

My current config alert-manager config:
html: '{{ define "email.tmpl" }} {{ end }}
{{ template "__email.tmpl" . }}'

I receive the following error although email.tmpl exists in that folder.

ERRO[16307] Notify for 1 alerts failed: Cancelling notify retry for "email" due to unrecoverable error: executing email html template: html/template::1:48: no such template "/etc/alertmanager/template/email.tmpl" source="dispatch.go:261"

Thanks in advance.

@mxinden
Copy link
Member

mxinden commented Jul 29, 2017

@doctorlector Please redirect your question to https://groups.google.com/forum/#!forum/prometheus-users if it is a usage question. In case you think you found a bug in Alertmanager, please open up a new issue on this repository. We are happy to help you in either one. Thanks.

@awaragi
Copy link
Contributor Author

awaragi commented Jul 31, 2017

I took a stab at adding text email notification support. I never developed with Go before so I kept my changes as simple as possible. I couldn't find any tests to update nor documentation in README.md so your online documentation need to be updated by someone else if the pull request is accepted.

The pull request is #934 . Looking forward to any feedback.

@awaragi
Copy link
Contributor Author

awaragi commented Jul 31, 2017

I just noticed that my pull request has failed due to Github not being available! I am not sure if there is a way to re-execute the validation or simply push an empty commit will trigger it.

@stuartnelson3
Copy link
Contributor

closed by #934

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants