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

[ospp] support custom notice template #1233

Merged
merged 17 commits into from
Oct 15, 2023

Conversation

Eden4701
Copy link
Contributor

@Eden4701 Eden4701 commented Sep 7, 2023

What's changed?

Custom Notice Template Configuration

Checklist

  • I have read the Contributing Guide
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@tomsun28 tomsun28 added enhancement New feature or request good first pull request Good for newcomers new feature labels Sep 8, 2023
@tomsun28 tomsun28 changed the title Custom NoticeTemplate [ospp] support custom notice template Sep 8, 2023
@tomsun28
Copy link
Contributor

👍

# password: 123456
# url: jdbc:h2:./data/hertzbeat;MODE=MYSQL
# hikari:
# max-lifetime: 120000
datasource:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个配置修改本地测试可以的,但是不要提交远程

description = "模板名称",
example = "dispatch-1", accessMode = READ_WRITE)
@Length(max = 100)
@NotNull
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

针对String类型的非空校验,可以使用@NotBlank

@Schema(title = "Record the latest modification time (timestamp in milliseconds)",
description = "记录最新修改时间", accessMode = READ_ONLY)
@LastModifiedDate
private LocalDateTime gmtUpdate;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是否缺少一个字段,用于标识是否系统预设模版(系统预设模板不允许删除)?

@@ -189,7 +344,9 @@ public boolean sendTestMsg(NoticeReceiver noticeReceiver) {
alert.setFirstAlarmTime(System.currentTimeMillis());
alert.setLastAlarmTime(System.currentTimeMillis());
alert.setPriority(CommonConstants.ALERT_PRIORITY_CODE_CRITICAL);
return dispatcherAlarm.sendNoticeMsg(noticeReceiver, alert);
Byte type=noticeReceiver.getType();
NoticeTemplate noticeTemplate=getNoticeTemplatesById(Long.valueOf(type));;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这边逻辑不太对,传参是type,但是查询是通过模板ID查询


@Override
public NoticeTemplate getNoticeTemplatesById(Long templateId) {
return noticeTemplateDao.getReferenceById(templateId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议使用findById,返回值用Optional

context.setVariable("monitorNameLabel", bundle.getString("alerter.notify.monitorName"));
context.setVariable("targetLabel", bundle.getString("alerter.notify.target"));
context.setVariable("target", alert.getTarget());
String monitorId=alert.getTags().get("monitorId");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

存在空指针风险

Signed-off-by: Eden4701 <Eden4701@163.com>
Signed-off-by: Eden4701 <Eden4701@163.com>
@tomsun28 tomsun28 added this to the v1.4.2 milestone Sep 26, 2023
@tomsun28
Copy link
Contributor

hi pls fix these confilcts

manager/pom.xml
web-app/src/assets/i18n/zh-CN.json

Signed-off-by: Eden4701 <Eden4701@163.com>
@tomsun28
Copy link
Contributor

  • 启动修改 hzb_notice_rule 表新增 template_id 字段报错不能为null, 这里是因为之前的老数据没有此字段导致,新增 alter table hzb_notice_rule add column template_id bigint not null 会导致之前的老数据异常
2023-10-11 11:14:23 [main] WARN  org.hibernate.tool.schema.internal.ExceptionHandlerLoggedImpl - GenerationTarget encountered exception accepting command : Error executing DDL "alter table hzb_notice_rule add column template_id bigint not null" via JDBC Statement
org.hibernate.tool.schema.spi.CommandAcceptanceException: Error executing DDL "alter table hzb_notice_rule add column template_id bigint not null" via JDBC Statement

建议不设置 template_id 约束null,然后考虑老数据问题,当template_id为null时应该有个对应的默认模版

  • 启动时报错 'classpath:sql/data.sql' 找不到资源,那是因为没有在manager模块的pom.xml里面配置资源路径,导致编译class时没有主动编译此资源到类路径下面去
    Caused by: java.lang.IllegalStateException: No data scripts found at location 'classpath:sql/data.sql'
    需要把 <include>sql/**</include> 添加到 manager pom.xml build.resources.resource.includes

@tomsun28
Copy link
Contributor

要不要把我们内置的默认消息模版像之前一样以文件的方式存储,用户自定义消息模版用现在的数据库方式存储。系统启动时从文件中加载内置模版到内存中,当 template_id 为null找不到时就使用我们内置的消息模版。页面配置用户不选择的情况下默认就使用内置消息模版。
这样能避免 template_id 为null老数据问题和data.sql重复初始化等问题,无需初始化数据库里面的消息模版数据。

@hertzbeat
Copy link
Contributor

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Should we store our built-in default message templates in files as before, and user-defined message templates in the current database method? When the system starts, the built-in template is loaded from the file into the memory. When template_id is null and cannot be found, our built-in message template is used. If the page configuration user does not select it, the built-in message template will be used by default.
This can avoid the problem of old data with template_id being null and repeated initialization of data.sql. There is no need to initialize the message template data in the database.

@Eden4701
Copy link
Contributor Author

@tomsun28 @Eden4701 可以换成用文件存储,这样就不用初始化数据和是否为默认模板这个字段了;但是缺点是需要额外维护一下从文件获取模板的代码,而且,如果要在页面显示默认模板的话,还需要额外编写代码(文件 + 数据库的分页感觉有点难做)

是的,如果这样做的话看能不能应用层的代码尽量不改动,主要是启动时加载内置的文件模版,还有数据层操作,因为内置模版只查询不修改,数据层分页改造的时候我们固定把内置模版排序到最前面,然后计算剩余pagesize 和pagenum查询数据库数据后合并 用 new PageImpl<>(list)来返回数据

新的代码已提交,预设模板文件存储,自定义模板在数据库,设置通知规则时模板是可选项,用户不选择模板时默认使用预设模板

@hertzbeat
Copy link
Contributor

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@tomsun28 @Eden4701 You can switch to file storage, so you don’t need to initialize the data and the fields of whether it is the default template; but the disadvantage is that you need to maintain the code to get the template from the file, and if you want to display the default template on the page If so, additional code needs to be written (file + database paging seems a bit difficult to do)

Yes, if you do this, see if you can keep the application layer code as unchanged as possible. The main thing is to load the built-in file template at startup and data layer operations, because the built-in template only queries and does not modify. When we transform the data layer paging Fixed sorting the built-in templates to the front, then calculating the remaining pagesize and pagenum, querying the database data, and then merging and using new PageImpl<>(list) to return the data.

The new code has been submitted, the preset template file is stored, and the custom template is in the database. The template is optional when setting notification rules. When the user does not select a template, the preset template is used by default.

Comment on lines 88 to 93

@EventListener(SystemConfigChangeEvent.class)
public void onEvent(SystemConfigChangeEvent event) {
log.info("{} receive system config change event: {}.", this.getClass().getName(), event.getSource());
this.bundle = ResourceBundleUtil.getBundle("alerter");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi 这里需要保留 当用户语言变更时 模版语言监听变更

Comment on lines 84 to 116
//获取sender
JavaMailSenderImpl sender = (JavaMailSenderImpl) javaMailSender;
String fromUsername = username;
try {
boolean useDatabase = false;
GeneralConfig emailConfig = generalConfigDao.findByType(TYPE);
if (emailConfig != null && emailConfig.getContent() != null) {
// 若启用数据库配置
String content = emailConfig.getContent();
EmailNoticeSender emailNoticeSenderConfig = objectMapper.readValue(content, EmailNoticeSender.class);
if (emailNoticeSenderConfig.isEnable()) {
sender.setHost(emailNoticeSenderConfig.getEmailHost());
sender.setPort(emailNoticeSenderConfig.getEmailPort());
sender.setUsername(emailNoticeSenderConfig.getEmailUsername());
sender.setPassword(emailNoticeSenderConfig.getEmailPassword());
Properties props = sender.getJavaMailProperties();
props.put("mail.smtp.ssl.enable", emailNoticeSenderConfig.isEmailSsl());
fromUsername = emailNoticeSenderConfig.getEmailUsername();
useDatabase = true;
}
}
if (!useDatabase) {
// 若数据库未配置则启用yml配置
sender.setHost(host);
sender.setPort(port);
sender.setUsername(username);
sender.setPassword(password);
Properties props = sender.getJavaMailProperties();
props.put("mail.smtp.ssl.enable", sslEnable);
}
} catch (Exception e) {
log.error("Type not found {}",e.getMessage());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi 数据库获取邮件服务器配置需要保留,我们现在支持用户在页面配置邮件服务器 或者 在 application.yml配置邮件服务器。优先级 页面数据库配置 高于 application.yml 文件配置。

Comment on lines 139 to 144

@EventListener(SystemConfigChangeEvent.class)
public void onEvent(SystemConfigChangeEvent event) {
log.info("{} receive system config change event: {}.", this.getClass().getName(), event.getSource());
this.bundle = ResourceBundleUtil.getBundle("alerter");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi 同上上,这里监听语言变更也需要保留

Comment on lines -90 to -93
@EventListener(SystemConfigChangeEvent.class)
public void onEvent(SystemConfigChangeEvent event) {
log.info("{} receive system config change event: {}.", this.getClass().getName(), event.getSource());
this.bundle = ResourceBundleUtil.getBundle("alerter");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi 同上,这里监听语言变更也需要保留

Copy link
Contributor

@tomsun28 tomsun28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍👍LGTM

@tomsun28
Copy link
Contributor

赞👍 完成度和质量都很高,这个PR合并了哦。
后续UI优化建议供参考:

  1. [通知模板] 菜单放到第三位即 [告警通知策略] 后面,因为通知模版在默认情况下用户不修改也能使用,对于多数没有修改模版需求的用户它只需前面两步即可。
  2. [告警通知策略]新增配置时建议[通知模版框]根据选中的接收人的通知类型自动渲染填充系统内置默认模版
  3. 编辑通知模版UI建议有展示当前所支持的模版环境变量,参考华为云消息通知模版
image

@hertzbeat
Copy link
Contributor

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Like 👍 The completion and quality are very high, this PR has been merged.
Follow-up UI optimization suggestions are for reference:

  1. The [Notification Template] menu is placed in the third position, that is, after [Alarm Notification Policy], because the notification template can be used by users without modification by default. For most users who do not need to modify the template, it only requires the first two steps. .
  2. [Alarm Notification Policy] When adding new configuration, it is recommended that [Notification Template Box] automatically render and fill the system's built-in default template according to the notification type of the selected recipient.
  3. It is recommended to display the currently supported template environment variables when editing the notification template UI. Please refer to Huawei Cloud Message Notification Template.
image

@tomsun28 tomsun28 merged commit 2883967 into apache:master Oct 15, 2023
2 checks passed
@tomsun28
Copy link
Contributor

@all-contributors please add @Eden4701 for code

@allcontributors
Copy link
Contributor

@tomsun28

I've put up a pull request to add @Eden4701! 🎉

@Eden4701
Copy link
Contributor Author

赞👍 完成度和质量都很高,这个PR合并了哦。 后续UI优化建议供参考:

  1. [通知模板] 菜单放到第三位即 [告警通知策略] 后面,因为通知模版在默认情况下用户不修改也能使用,对于多数没有修改模版需求的用户它只需前面两步即可。
  2. [告警通知策略]新增配置时建议[通知模版框]根据选中的接收人的通知类型自动渲染填充系统内置默认模版
  3. 编辑通知模版UI建议有展示当前所支持的模版环境变量,参考华为云消息通知模版
image

收到!

@hertzbeat
Copy link
Contributor

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Like 👍 The completion and quality are very high, this PR has been merged. Follow-up UI optimization suggestions are for reference:

  1. The [Notification Template] menu is placed in the third position, that is, after [Alarm Notification Policy], because the notification template can be used by users without modification by default. For most users who do not need to modify the template, it only requires the first two steps. Can.
  2. [Alarm Notification Policy] When adding new configurations, it is recommended that the [Notification Template Box] automatically render and fill the system's built-in default template according to the notification type of the selected recipient.
  3. It is recommended to display the currently supported template environment variables when editing the notification template UI. Please refer to Huawei Cloud Message Notification Template.
image

receive!

@tomsun28
Copy link
Contributor

@Eden4701 hi 打扰了 今天使用中有个建议反馈下 我们现在对默认模版每次是发告警时去打开文件,读取文件内容,关闭文件这样,在大量告警消息时会很耗性能。建议参考 org.dromara.hertzbeat.manager.service.impl.AppServiceImpl#run 在程序启动时统一读取文件加载到内存中,发告警消息时若使用内置默认模版,就到内存中取模版内容即可。

@hertzbeat
Copy link
Contributor

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@Eden4701 hi I’m sorry to bother you. I have a suggestion during use today. Please give me feedback. We now use the default template to open the file, read the file content, and close the file every time an alarm is sent. This will consume a lot of performance when there are a large number of alarm messages. It is recommended to refer to org.dromara.hertzbeat.manager.service.impl.AppServiceImpl#run to uniformly read the file and load it into the memory when the program starts. If you use the built-in default template when sending an alarm message, just get the template content from the memory.

@Eden4701
Copy link
Contributor Author

@Eden4701 hi 打扰了 今天使用中有个建议反馈下 我们现在对默认模版每次是发告警时去打开文件,读取文件内容,关闭文件这样,在大量告警消息时会很耗性能。建议参考 org.dromara.hertzbeat.manager.service.impl.AppServiceImpl#run 在程序启动时统一读取文件加载到内存中,发告警消息时若使用内置默认模版,就到内存中取模版内容即可。

ok

@hertzbeat
Copy link
Contributor

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@Eden4701 hi Sorry to bother you. I have a suggestion and feedback during use today. We now use the default template to open the file, read the file content, and close the file every time an alarm is sent. This will consume a lot of performance when there are a large number of alarm messages. It is recommended to refer to org.dromara.hertzbeat.manager.service.impl.AppServiceImpl#run to uniformly read the file and load it into the memory when the program starts. If you use the built-in default template when sending an alarm message, just get the template content from the memory.

OK

tomsun28 pushed a commit that referenced this pull request Jan 16, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
tomsun28 pushed a commit that referenced this pull request Mar 9, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
tomsun28 pushed a commit that referenced this pull request Mar 9, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
tomsun28 pushed a commit that referenced this pull request Mar 10, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
tomsun28 pushed a commit that referenced this pull request Mar 10, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
tomsun28 pushed a commit that referenced this pull request Mar 11, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
tomsun28 pushed a commit that referenced this pull request Mar 11, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
tomsun28 pushed a commit that referenced this pull request Mar 11, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
tomsun28 pushed a commit that referenced this pull request Mar 11, 2024
Signed-off-by: Eden4701 <Eden4701@163.com>
Co-authored-by: ulikehhh <2762242004@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first pull request Good for newcomers new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] <Development of custom template configuration for message notification>
5 participants