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

Custom the save method of the Rule model to handle the annotation "rule" #556

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

hoangpn
Copy link
Contributor

@hoangpn hoangpn commented Dec 2, 2024

When creating or updating a rule on Promgen, the corresponding rule data on Prometheus will be given an annotation "rule" that is the link to that rule on Promgen. However, the annotation rule on Promgen always has the URI "/rule/0" because it has not been intentionally set up:

  • Previously, the value of annotation["rule"] on Promgen was accidentally set when loading data from the AlertRuleForm. Specifically, in the form's clean() method, there is a declaration of prometheus.check_rule(), which includes a serializer that automatically adds the annotation["rule"]. Since Python is pass-by-object-reference, annotation["rule"] of the rule will be automatically set whenever we create or update it.
  • After the commit 2761419, to handle the case of calling prometheus.check_rule() when creating a new rule, a temporary line of code setting rule.pk=0 was added. Since this is a shared form between creating and updating, the value of annotation["rule"] for both cases becomes /rule/0.
  • The actual value of annotation["rule"] on Prometheus is not affected because it will be serialized again before each time it is sent to Prometheus. https://github.com/line/promgen/blob/master/promgen/signals.py#L173

-->
To ensure data is synced between Promgen and Prometheus, I customize the save method of the Rule model.
This solution will help keep the data in the database synchronized and requires minimal changes while still affecting all processes such as displaying, creating, updating, cloning, or importing rules.

Before:
Data displayed on Rule page after creating or updating rule:
image

After:
Data displayed on Rule page after creating or updating rule:
image

@hoangpn hoangpn requested review from kfdm and a team as code owners December 2, 2024 09:56
@hoangpn hoangpn force-pushed the custom_save_rule_method branch 2 times, most recently from d1e211e to 40958ef Compare December 2, 2024 10:11
Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

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

I have reviewed and tested your code. It works fine :)

However, I have some minor suggestions I would like you to fix.

Apart from the suggestions I have made here in GitHub, could you please wrap the lines in your commit message at 100 characters?

Thank you very much!

promgen/models.py Outdated Show resolved Hide resolved
promgen/models.py Outdated Show resolved Hide resolved
promgen/tests/test_models.py Outdated Show resolved Hide resolved
promgen/tests/test_models.py Outdated Show resolved Hide resolved
promgen/tests/test_models.py Outdated Show resolved Hide resolved
promgen/tests/test_models.py Outdated Show resolved Hide resolved
promgen/tests/test_models.py Outdated Show resolved Hide resolved
@hoangpn
Copy link
Contributor Author

hoangpn commented Dec 3, 2024

@vincent-olivert-riera
Thank you for your comments. I have updated the source code according to your suggestion by a fix-up commit.

@vincent-olivert-riera
Copy link
Contributor

@vincent-olivert-riera Thank you for your comments. I have updated the source code according to your suggestion by a fix-up commit.

Looks good to me 👍

Please squash the fixup commits for final review. Don't forget to change the commit message to wrap all lines at 100 characters.

@hoangpn hoangpn force-pushed the custom_save_rule_method branch from 675e6dd to 3328fff Compare December 3, 2024 06:56
@hoangpn
Copy link
Contributor Author

hoangpn commented Dec 3, 2024

Thank you for your hard work. I squashed the fix-up commits and reformatted the commit message too. 👍

When creating or updating a rule on Promgen, the corresponding rule data on Prometheus will be given
an annotation rule that is the link to that rule on Promgen. However, the annotation rule on Promgen
always has the URI "/rule/0" because it has not been intentionally set up.

To ensure data is synced between the two applications, we customize the save method of the Rule
model.
@vincent-olivert-riera vincent-olivert-riera merged commit f4e6012 into line:master Dec 3, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants