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

Support dynamically configure alarm settings #3557

Merged
merged 13 commits into from Oct 5, 2019
Merged

Support dynamically configure alarm settings #3557

merged 13 commits into from Oct 5, 2019

Conversation

kezhenxu94
Copy link
Member

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • Bug fix

  • New feature provided

  • Improve performance

  • Related issues

closes #3467

@kezhenxu94 kezhenxu94 added core feature Core and important feature. Sometimes, break backwards compatibility. backend OAP backend related. feature New feature labels Oct 2, 2019
@kezhenxu94 kezhenxu94 added this to the 6.5.0 milestone Oct 2, 2019
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

One core requirement for this, the alarm running context should be updated only if it exists before. The key is, the running context should be changed rule by rule.

@wu-sheng
Copy link
Member

wu-sheng commented Oct 4, 2019

CI failing? I am going to review this tomorrow.

Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

According to the document, I would suggest we need to update document

  1. New dynamic settings for alarm.
  2. Alarm cached data will be lost if alarm rule changes. Such as changing period from 5 to 10, will make last 5 mins cache data lost. This makes the alarm cache data started now.

@kezhenxu94 kezhenxu94 closed this Oct 5, 2019
@kezhenxu94 kezhenxu94 reopened this Oct 5, 2019
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

LGTM. I think you should send a notification mail to dev mail list about this new and important feature in 6.5.0.

@wu-sheng wu-sheng merged commit 2801de7 into apache:master Oct 5, 2019
@kezhenxu94 kezhenxu94 deleted the feature/dynamic-alarm-setting branch October 5, 2019 14:51
@SpursJiang
Copy link

Thank you very much!We really need this feature!

@SpursJiang
Copy link

Thank you very much! @kezhenxu94 @wu-sheng

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend OAP backend related. core feature Core and important feature. Sometimes, break backwards compatibility. feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Dynamic config in alarm
3 participants