louislam / uptime-kuma

A fancy self-hosted monitoring tool
https://uptime.kuma.pet
MIT License
54.97k stars 4.95k forks source link

Implement Templates for Notifications #646

Open deefdragon opened 2 years ago

deefdragon commented 2 years ago

Is it a duplicate question?

This is a potential solution to a number of other issues, consolidating them into one, and has been discussed briefly in those other issues.

Requests to have different information in the notification

Potential long term bonuses/considerations

Having templates built up would allow for custom reports to be written like requested in #366 (requires a reports task system to be built up) Would allow for inserting custom data into notifications like requested in #202 (would require saving of custom information for monitors/tags/groups/global etc.)

Is your feature request related to a problem? Please describe.

The messages sent in notifications have several different components which many have requested the ability to modify parts of or all of, and each of which requires translation.

Describe the solution you'd like

The implementation of a template engine such as liquidjs to allow users to customize the message sent as they see fit if they wish to do so. This would also condense the messages into as few formats as possible for translation.

Some implementations such as Discord require an object to use the API. In this case, the template would be used to generate a JSON object that would then be parsed and used as the required object.

Describe alternatives you've considered

627 currently proposes adding regex-bassed key replacement in the email subject. It would also possible to leave the components individual and just customize/translate each one, but that would likely be a messy, hard to update solution.

Additional context

I can see 4 major steps to this.

  1. Create the system to lookup, parse and return the templates
  2. Alter notifications to use common, simple templates in their message
  3. Alter front end & db to allow using custom templates
  4. Implement global template setting

2 and 3 would have to done for the simple notifications, as well as for the JSON object implementations. 1 would basically be re-usable, however a fallback will be required in the case that the data returned from the template is not valid JSON.

I am willing to work on this, however I would like some input & other eyes before I do so. I want to make sure I am not going in the completely wrong direction. This is also a large enough change that, as has been mentioned, I want the sign off of @louislam before I go any further.

louislam commented 2 years ago

It would be really difficult in my opinion. I would suggest that it should be implemented once we have a whole picture of it.

I have a similar thoughts before, but I can't think of a good approach, because there are a lot of formats here, for example:

  1. Implement global template setting

With this situation, how could global template be implemented?

  1. Alter front end

For notification like Discord, how is it look like in frontend? Textarea with json?

Current implementation

For your information, currently, there are two usages of Notification.send(...); https://github.com/louislam/uptime-kuma/blob/2a1fd93444d082e9d473f6548861d4aacd50dea6/server/notification.js#L74

  1. General messages

    Notification.send(notification, "HelloπŸ‘‹");
  2. Sending important up/down notifications

    Notification.send(notification, "[Test][πŸ”΄ Down] 404 Not Found", monitor, heartbeat);
bakeDong1 commented 2 years ago

how about global variable like statping

deefdragon commented 2 years ago

I would suggest that it should be implemented once we have a whole picture of it.

By this, do you mean we need to the proof of concept to then iterate on? If so I will get started on it.

... there are a lot of formats here, for example:

  • Line, Discord, Slack, Rocket.chat and more: All different JSON formats / payloads, also it is conditional (e.g. up = green, down = red).
  • Email: subject and body

One of the things that came to mind would be to have different detail levels of templates. (Minimal, small, medium, large, full etc.) The defaults for emails could then have minimal in the subject and large or full in the body, Appraise::Twitter could use the small template, etc.. When Selecting what is displayed in the message, you can then choose "custom" which exposes a textarea where you can input a custom template.

As for the the conditional status, we could either make up/down templates for each, or, because its a template, we can include that in the template itself. Something to be examined in the Proof of Concept.

{% if monitor.status == "up" %}
  {{ monitor.name }} is now up.
{% else %}
  {{ monitor.name }} is now down. 
{% endif %}
  1. Implement global template setting

With this situation, how could global template be implemented?

This was a combination of scope creep & crossed wires on my part. I was thinking about how many monitors some people had, not notifications, and reconsidering it with notifications, it would be a VERY rare situation where someone has that many notifications requiring the same template. A large number of notifications, IE on a language by language basis is possible. But not requiring the same template for many notifications.

  1. Alter front end

For notification like Discord, how is it look like in frontend? Textarea with json?

Expanding what I have here. I see adding a "simple/advanced" toggle to the JSON endpoints.

Selecting "custom" for the advanced mode would then have the user generate the JSON parsed into an object which is then sent to the service.

The default advanced template would have to be added for each of the different services for translation, but I don't think that would be much more complicated than the current system, and it would centralize them.

Current implementation

[...]

I am going to have to dig into how the different .sends are called. I might start with Templates only applying for the case where monitor & heartbeat are not null.

louislam commented 2 years ago

So it is really a large structural change.

One very important thing that I forgot to mention is, there is no auto tests of notifications. Also I believe that it is almost impossible to implement those tests.

All of them had already been tested well by hand. I am afraid that touching them would have some unpredictable situations that make notifications unstable as a result.

Recently, I actually rejected a big pull request, because there is no way for me to verify the change completely. (#568)

I prefer not to implement this, just because of the testing problem.

Instead, at this moment, I would prefer the approach like #627. Each pull request just modify one notification. It is easy for me to check one by one.

But I agree that we should use the parser like liquidjs instead of regex.

deefdragon commented 2 years ago

I can think of a potential solution to the testing problem, but I understand the not wanting to take on such a large change until that is implemented.

Would you be willing to consider it if I limited it in scope to not include the complex JSON cases for now? The changes would be limited to around here https://github.com/louislam/uptime-kuma/blob/0550ceb6d42f6c9a2d6233d17230e79951953e04/server/model/monitor.js#L328 (I was not aware how literal you were being when you said that there were 2 calls to Notification.send. I thought you meant two types of calls)

PopcornPanda commented 2 years ago

I think, there is no need for changing notification system itself. It could be basically a template engine that generate body from arguments. Something like this:

msg = template(templateName,monitorJSON,heartbeatJSON,someExtraArgs...)

Where templateName is name of predefined template which we could create in dashboard. Maybe there should also be a way to pass template as a string like this:

msg = template("[{{NAME}}] {{$t(has changed status to)}} {{STATUS}}",monitorJSON,heartbeatJSON,someExtraArgs...)

For multilanguage support, text in $t() could be translated like in vue files.

If it only generates a text, then it should be safe for existing notifications (because it doesn't touch them) and simple to test. Just change ready msg to result of template engine in one notification provider at a time and check provider itself.

I'm not a nodejs dev, and I don't know that this has any sense in this language. In ruby ERB just handles generating content from templates and You just use result where You need it.

deefdragon commented 2 years ago

So, I have started on this, and gotten a quick and dirty example to work. While parsing the template in place of msg on line 328 can work, I am once again wondering if it would be better to put the template parsing inside Notification.send, and not 328.

I wonder this because I think that when a test notification is sent, it could include some mock objects for the monitor and heartbeat. Combined with parsing the template in Notification.send, the user will be able to not only test connection to the notification provider etc, but also be able to see what the notification will look like when received.

Hamitamaru commented 2 years ago

Was just about to cut a separate issue for the Slack particular notification improvement I would like to see but I found this one.

If I run this:

curl -X POST -H 'Content-type: application/json' --data '{"text": "[βœ… Up][plex]","username": "uptime-kuma","blocks": [{"type": "header","text": {"type": "plain_text","text": "Uptime Kuma Alert 2",},},{"type": "section","fields": [{"type": "mrkdwn","text": "*Message*\n la2"},{"type": "mrkdwn","text": "*Time (UTC)*\n la"}]}]}' https://hooks.slack.com/services/replaceThisLinkWithYourSlackAppIncomingWebHookUrl

I get a more useful notification on my android phone that shows [βœ… Up][plex] rather than less useful Uptime Kuma Alert so I don't need to open slack to understand what has changed (what went up or down).

Not sure what template variables we are adding support for, so far I've seen [{{NAME}}] I guess this is the name of the monitor stored in monitorJSON["name"] [{{STATUS}}] I guess this is the value in heartbeatJSON["status"]

Can we create template variables scheme that allow for arbitrary key access in both monitorJSON and heartbeatJSON e.g. [{{monitor-name}}] results in monitorJSON["name"] [{{heartbeat-status}}] results in heartbeatJSON["status"]

Aside from that arbitrary template variable scheme for access to those JSON variables if we can also find a way to accomplish: [βœ… Up] and the down equivalent via some template variable that would help my case.

deefdragon commented 2 years ago

@Hamitamaru unfortunately, the more complex notification types like slack/discord are going to take a bit to implement. The notification message can be changed using a template without much worry as it's just text. Discord/Slack etc.'s richer features on the other hand will each require wiring up and testing individually to account for their different formats.

The templates should have access to the entire monitor and heartbeat objects (minus some potential sanitization). I also suspect there will be a number of helpers included such as βœ… Up, But time will tell on the full details.

deefdragon commented 2 years ago

Question on translation: Do we have the a way to translate things server side? I'm not seeing any previous uses, and part of why I started this was so that the default templates could be translated.

tiakun commented 2 years ago

Would it be easier if this is implemented not globally but per notification type? It would also allow the customization per notification type, e.g. some users might want short message for SMS but longer messages for chat/message apps.

deefdragon commented 2 years ago

It would be easier, but it would make the notifications aspect of uptime kuma even more of a dis-congruous mess.

The ideal would be to have a consistent notification form/message for everything, and while it's ok now, it could be much better.

The thing is, there are other aspects of UK that should get worked on first (groups, notification controls/cascading etc). Louis only has so much time to test, and addressing the notifications system is rather low on the list.

I'm going to close this issue for now, as it is very likely not going to get addressed until a V2/notifications core code re-write.

chakflying commented 8 months ago

Re-opening this since I think this is a better place to keep track of all the duplicates.

Realistically it probably won't happen before 2.2.0 at the earliest, but fundamentally the issue still exists and this is the most practical way of solving it.

Qumans commented 3 months ago

Since Feishu notifications can only fix templates, and I wanted to customize the content, I used webhook, and the url was the Feishu bot's webhook url. After clicking the test, it shows that the delivery was successful, but there is no message for my flying book. I can receive the flying book after testing with postman. And many times will appear Error: Error: getaddrinfo ENOTFOUND open.feishu.cn. Whatever the form of notification

Qumans commented 3 months ago

Why did I not receive the message after kuma successfully sent it? Same url, same body, but I get messages through postman@

CommanderStorm commented 3 months ago

@Qumans You are posting this in the wrong place/issue. => Please open a new issue and include full context of what you tried and what works in Postman via screenshots.

There are a lot of cases that MIGHT be wrong: It might be because of your network, your DNS or because the notification provider had an outage. Debugging these cases is not something that this issue is a good place to do, this issue is about implementing notification-templates not using the webhook notification provider. Some basic troubleshooting steps are writtten down here: https://github.com/louislam/uptime-kuma/wiki/Troubleshooting

borrelan commented 2 weeks ago

Hello,

I would like to thank the team behind Uptime Kuma for such an awesome service that I have come to depend on. As this issue is ongoing, would you be willing to consider change the static entry of "[Uptime Kuma]" in the ntfy notification to include the "Friendly Name"? When you have more than one instance pushing to the same topic it makes it difficult to determine which instance is pushing the notification. One could also argue what's the point of the setting "Friendly Name" if it's not used?

Thanks again!!!