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.

Sign up for GitHub

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

Jump to bottom

Alerting: Makes timeouts and retries configurable #16259

Merged
merged 10 commits into from
Mar 29, 2019
Merged

Alerting: Makes timeouts and retries configurable #16259

merged 10 commits into from
Mar 29, 2019

Conversation

kobehaha
Copy link
Contributor

What this PR does / why we need it:

Make alertTimeout and alertMaxAttempts configurable in the config file

Which issue(s) this PR fixes:

Fixes #16240

Release note:

Make alertTimeout and alertMaxAttempts configurable in the config file

@CLAassistant
Copy link

CLAassistant commented Mar 27, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

marefr
marefr suggested changes Mar 27, 2019
View reviewed changes
Copy link
Contributor

@marefr marefr left a comment

Choose a reason for hiding this comment

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

This is a good start. What's left here is naming and descriptions of configuration settings.

My suggestions, but not sure about them because naming is hard 😄 :
s/alertingExcuteCaculateTimeout/alertingEvaluationTimeoutSeconds
s/execute_caculate_rule_timeout/evaluation_timeout_seconds
s/alertingResultHandleTimeout/alertingNotificationTimeoutSeconds
s/result_handle_timeout/notification_timeout_seconds

What you think?

@marefr marefr changed the title Make alertTimeout and alertMaxAttempts configurable in the config file Alerting: Makes timeouts and retries configurable Mar 27, 2019
@marefr
Copy link
Contributor

marefr commented Mar 27, 2019

You also have some go formatting issue and that's why build failed. Please run go fmt to fix issues (https://circleci.com/gh/grafana/grafana/81504?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link).

@kobehaha
Copy link
Contributor Author

kobehaha commented Mar 28, 2019 •
edited
Loading

Yes, you are right, changing the definition of variables will better understand. And I found that there are some test cases that need to be fixed, etc.

@kobehaha
Copy link
Contributor Author

kobehaha commented Mar 28, 2019 •
edited
Loading

It is my fault , I forgot the test case,Now I have solved all the problems, you can review it again. 😄

marefr
marefr suggested changes Mar 28, 2019
View reviewed changes
Copy link
Contributor

@marefr marefr left a comment

Choose a reason for hiding this comment

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

We're getting there, but still have some requests for changes and questions.

conf/defaults.ini Outdated Show resolved Hide resolved
conf/sample.ini Outdated Show resolved Hide resolved
docs/sources/installation/configuration.md Outdated Show resolved Hide resolved
pkg/services/alerting/engine.go Outdated Show resolved Hide resolved
pkg/setting/setting.go Outdated Show resolved Hide resolved
@kobehaha
Copy link
Contributor Author

I handled the problem mentioned above, you can review it again, thanks

marefr
marefr approved these changes Mar 29, 2019
View reviewed changes
Copy link
Contributor

@marefr marefr left a comment

Choose a reason for hiding this comment

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

This looks great! Awesome work +1:

@marefr marefr merged commit 1b84a92 into grafana:master Mar 29, 2019
@marefr
Copy link
Contributor

marefr commented Mar 29, 2019

Thank you for contributing to Grafana!

@marefr marefr added this to the 6.2 milestone Mar 29, 2019
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into react-streaming
3806da8 
* grafana/master:
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
  Fix: Correct SnapshotData typing (grafana#16279)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into table-datasource
896f9b4 
* grafana/master:
  Fix: Bring back styles on Switch components when checked
  Update CHANGELOG.md
  Chore: breaks build if certain FrontEnd limits are exceeded (grafana#16301)
  Fix: Graphite query ast to string fix (grafana#16297)
  Fix: Template query editor this bind exception fix (grafana#16299)
  Fix: Alerting Notification channel http api fixes (grafana#16288)
  Refactor: Move LogLevel and Labels utils to @grafana/ui (grafana#16285)
  Refactor: Rename Tags to Labels in SeriesData (simple) (grafana#16284)
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into panel-in-view-from…
10d819f 
…-scroll-again

* grafana/master:
  Fix: Bring back styles on Switch components when checked
  Update CHANGELOG.md
  Chore: breaks build if certain FrontEnd limits are exceeded (grafana#16301)
  Fix: Graphite query ast to string fix (grafana#16297)
  Fix: Template query editor this bind exception fix (grafana#16299)
  Fix: Alerting Notification channel http api fixes (grafana#16288)
  Refactor: Move LogLevel and Labels utils to @grafana/ui (grafana#16285)
  Refactor: Rename Tags to Labels in SeriesData (simple) (grafana#16284)
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
  Fix: Correct SnapshotData typing (grafana#16279)
  Feat: Angular panels & SeriesData to Table/TimeSeries (grafana#16266)
  Fix: React Graph & Show message on no data (grafana#16278)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into clone-reference-panel
cb55609 
* grafana/master:
  Fix: Bring back styles on Switch components when checked
  Update CHANGELOG.md
  Chore: breaks build if certain FrontEnd limits are exceeded (grafana#16301)
  Fix: Graphite query ast to string fix (grafana#16297)
  Fix: Template query editor this bind exception fix (grafana#16299)
  Fix: Alerting Notification channel http api fixes (grafana#16288)
  Refactor: Move LogLevel and Labels utils to @grafana/ui (grafana#16285)
  Refactor: Rename Tags to Labels in SeriesData (simple) (grafana#16284)
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
  Fix: Correct SnapshotData typing (grafana#16279)
  Feat: Angular panels & SeriesData to Table/TimeSeries (grafana#16266)
  Fix: React Graph & Show message on no data (grafana#16278)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into dynamic-units
5e5d374 
* grafana/master: (102 commits)
  Fix: Bring back styles on Switch components when checked
  Update CHANGELOG.md
  Chore: breaks build if certain FrontEnd limits are exceeded (grafana#16301)
  Fix: Graphite query ast to string fix (grafana#16297)
  Fix: Template query editor this bind exception fix (grafana#16299)
  Fix: Alerting Notification channel http api fixes (grafana#16288)
  Refactor: Move LogLevel and Labels utils to @grafana/ui (grafana#16285)
  Refactor: Rename Tags to Labels in SeriesData (simple) (grafana#16284)
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
  Fix: Correct SnapshotData typing (grafana#16279)
  Feat: Angular panels & SeriesData to Table/TimeSeries (grafana#16266)
  Fix: React Graph & Show message on no data (grafana#16278)
  Feature: added actionable message in Explore when no datasource configured (grafana#16252)
  Feature: Case insensitive Loki search (grafana#15948)
  Feat: Singlestat panel react progress & refactorings (grafana#16039)
  Chore: Implement gosec (grafana#16261)
  Fix: Updated snapshot unit test that was failing
  Refactor: Theme & Removed the last rems (grafana#16245)
  ...
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into stats-filter
783cc20 
* grafana/master:
  Fix: Bring back styles on Switch components when checked
  Update CHANGELOG.md
  Chore: breaks build if certain FrontEnd limits are exceeded (grafana#16301)
  Fix: Graphite query ast to string fix (grafana#16297)
  Fix: Template query editor this bind exception fix (grafana#16299)
  Fix: Alerting Notification channel http api fixes (grafana#16288)
  Refactor: Move LogLevel and Labels utils to @grafana/ui (grafana#16285)
  Refactor: Rename Tags to Labels in SeriesData (simple) (grafana#16284)
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
  Fix: Correct SnapshotData typing (grafana#16279)
  Feat: Angular panels & SeriesData to Table/TimeSeries (grafana#16266)
  Fix: React Graph & Show message on no data (grafana#16278)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into expose-get-backend…
8e17d98 
…-srv

* grafana/master:
  Fix: Bring back styles on Switch components when checked
  Update CHANGELOG.md
  Chore: breaks build if certain FrontEnd limits are exceeded (grafana#16301)
  Fix: Graphite query ast to string fix (grafana#16297)
  Fix: Template query editor this bind exception fix (grafana#16299)
  Fix: Alerting Notification channel http api fixes (grafana#16288)
  Refactor: Move LogLevel and Labels utils to @grafana/ui (grafana#16285)
  Refactor: Rename Tags to Labels in SeriesData (simple) (grafana#16284)
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
  Fix: Correct SnapshotData typing (grafana#16279)
  Feat: Angular panels & SeriesData to Table/TimeSeries (grafana#16266)
  Fix: React Graph & Show message on no data (grafana#16278)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into jwt
c80f2b9 
* grafana/master:
  Fix: Bring back styles on Switch components when checked
  Update CHANGELOG.md
  Chore: breaks build if certain FrontEnd limits are exceeded (grafana#16301)
  Fix: Graphite query ast to string fix (grafana#16297)
  Fix: Template query editor this bind exception fix (grafana#16299)
  Fix: Alerting Notification channel http api fixes (grafana#16288)
  Refactor: Move LogLevel and Labels utils to @grafana/ui (grafana#16285)
  Refactor: Rename Tags to Labels in SeriesData (simple) (grafana#16284)
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
  Fix: Correct SnapshotData typing (grafana#16279)
  Feat: Angular panels & SeriesData to Table/TimeSeries (grafana#16266)
  Fix: React Graph & Show message on no data (grafana#16278)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Mar 29, 2019
@ryantxu
Merge remote-tracking branch 'grafana/master' into timepicker-tooltip
aaf0a18 
* grafana/master: (608 commits)
  Fix: Bring back styles on Switch components when checked
  Update CHANGELOG.md
  Chore: breaks build if certain FrontEnd limits are exceeded (grafana#16301)
  Fix: Graphite query ast to string fix (grafana#16297)
  Fix: Template query editor this bind exception fix (grafana#16299)
  Fix: Alerting Notification channel http api fixes (grafana#16288)
  Refactor: Move LogLevel and Labels utils to @grafana/ui (grafana#16285)
  Refactor: Rename Tags to Labels in SeriesData (simple) (grafana#16284)
  Elasticsearch: Fix view percentiles metric in table without date histogram (grafana#15686)
  Configuration: Improve session_lifetime comments (grafana#16238)
  Alerting: Makes timeouts and retries configurable (grafana#16259)
  Fix: Correct SnapshotData typing (grafana#16279)
  Feat: Angular panels & SeriesData to Table/TimeSeries (grafana#16266)
  Fix: React Graph & Show message on no data (grafana#16278)
  Feature: added actionable message in Explore when no datasource configured (grafana#16252)
  Feature: Case insensitive Loki search (grafana#15948)
  Feat: Singlestat panel react progress & refactorings (grafana#16039)
  Chore: Implement gosec (grafana#16261)
  Fix: Updated snapshot unit test that was failing
  Refactor: Theme & Removed the last rems (grafana#16245)
  ...
@kobehaha kobehaha deleted the make-some-alert-parameter-configurable branch March 31, 2019 01:37
@ying-jeanne ying-jeanne added the pr/external This PR is from external contributor label Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marefr marefr marefr approved these changes

Assignees
No one assigned
Labels
add to changelog pr/external This PR is from external contributor
Projects
None yet
Milestone
6.2-beta1
Development

Successfully merging this pull request may close these issues.

Make alertTimeout and alertMaxAttempts configurable in the config file
4 participants
@kobehaha @CLAassistant @marefr @ying-jeanne