-
Notifications
You must be signed in to change notification settings - Fork 1
cloud_run_alerts: add flexible json-based log indicators and policies #79
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This PR enhances the cloud_run_service_alerts module by adding support for flexible JSON-based log indicators and alert policies:
|
6fa61af
to
69df9b3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never looked at this code before today but on the whole this change looks good to me. I reviewed it to try to understand if / how it would affect existing users of the module and for the most part it seems to preserve the existing behavior, though I did have questions in one or two parts.
I know that @shankiyani has started to review this and is perhaps testing it out in his environment, I'll defer approval to him.
69df9b3
to
4e92aff
Compare
4e92aff
to
03898be
Compare
@shankiyani @pjh PR was updated |
@shankiyani please re-review, I'm deferring approval to you. |
Thanks @andriipetruk setting up a local test will take some time and I want to think through if this pattern should be adopted for the text payload variant as well. In general multi-condition alert policy hinders the ability to use Cloud Monitoring Dashboards with no indicator that the feature will be supported anytime soon. I've been wanting to create a more "flattened" approach at the cost of increased number of APs but the draw being that dashboards can be used for centralized on-call viewing. Its not blocking for this PR but I wanted to share my intentions for this module going forward. |
03898be
to
d7a562e
Compare
91276da
to
1487cb3
Compare
1487cb3
to
94e3d9b
Compare
94e3d9b
to
eec5337
Compare
This PR enhances the cloud_run_service_alerts module by adding support for flexible JSON-based log indicators and alert policies: