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

Added alert for logic app workflow failed runs #304

Closed

Conversation

cdomansky
Copy link

@cdomansky cdomansky commented Aug 9, 2024

Overview/Summary

Added alert for logic app workflow failed runs

This PR fixes/adds/changes/removes

Added alert for logic app workflow failed runs

Breaking Changes

None

As part of this Pull Request I have

  • Read the Contribution Guide and ensured this PR is compliant with the guide
  • Checked for duplicate Pull Requests
  • Associated it with relevant GitHub Issues or ADO Work Items (Internal Only)
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Ensured PR tests are passing
  • Updated relevant and associated documentation (e.g. Contribution Guide, Docs etc.)

@cdomansky
Copy link
Author

Deployment through pipeline

image

Policy

image

Alerts

image

@JoeyBarnes JoeyBarnes added the Pattern: ALZ 🚁 Issues / PR's related to the ALZ Pattern label Aug 20, 2024
@arjenhuitema arjenhuitema self-assigned this Aug 23, 2024
@arjenhuitema
Copy link
Contributor

Hi @cdomansky, I appreciate your input. Your proposal is great, and it would be fantastic to integrate it into AMBA.

However, no further changes will be made to the Landing Zone Initiative, as the Landing Zone Initiative will be phased out soon and replaced with smaller initiatives divided by Service Category. The same Policies will apply, but they'll be distributed across several smaller initiatives. This is necessary because we're reaching the limits of the current initiative.

At the moment, we can proceed with these modifications only:

  1. services/Logic/workflows/Deploy-Workflow-RunsFailed-Alert.json
  2. services/Logic/workflows/alerts.yaml

Once the new initiatives are merged and the Landing Zone initiative is phased out, we can focus on developing a new initiative that incorporates Logic Apps.

@arjenhuitema arjenhuitema added the Area: Policy 📝 Issues / PR's related to Policy label Aug 23, 2024
@@ -2033,6 +2033,21 @@
},
"FDBackendRequestLatencyAlertState": {
"value": "true"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Please undo the modifications to the parameter file. No additional changes will be made to the Landing Zone Initiative, so no updates are necessary for the parameters until we launch a new initiative for Logic Apps.

@@ -4176,6 +4176,74 @@
"displayName": "FD Backend Request Latency Alert State",
"description": "Alert state for the alert"
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Please undo the modifications. No additional changes will be made to the Landing Zone Initiative.

@@ -1,6 +1,8 @@
targetScope = 'managementGroup'

@metadata({ message: 'The JSON version of this file is programatically generated from Bicep. PLEASE DO NOT UPDATE MANUALLY!!' })
@metadata({
Copy link
Contributor

Choose a reason for hiding this comment

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

Please reverse the changes. We will break this template up into smaller parts due to hitting ARM limits. Since the new policy isn't part of an initiative yet, it doesn't need to be deployed yet.

"displayName": "Deploy Logic App Workflow Runs Failed Alert",
"description": "Policy to audit/deploy Logic App Workflow Runs Failed Alert",
"metadata": {
"version": "1.1.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"version": "1.1.0",
"version": "1.0.0",

@@ -4,8 +4,8 @@
verified: false
visible: true
tags:
- auto-generated
- agc-62008
- auto-generated
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the format changes need to reverted. @JoeyBarnes can you check/approve the changes to the yaml? Suggest to check changes to format, GUID, Tags for the added alert.

@arjenhuitema arjenhuitema added the AMBA Core Issues / PR's related AMBA Core label Aug 23, 2024
@arjenhuitema arjenhuitema added the Needs: Author Feedback Needs the author to provide feedback label Aug 23, 2024
@arjenhuitema
Copy link
Contributor

@cdomansky, Feel free to ask if you need clarification on my comments or change requests.

If this PR remains inactive, I will proceed to close it.

@cdomansky
Copy link
Author

@arjenhuitema I saw the upcoming changes to the ALZ Pattern and decided to hold off implementing until after I pull in the major updates and test. You can close this PR and I'll reopen once I have the time to commit to pulling in the major update and test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMBA Core Issues / PR's related AMBA Core Area: Policy 📝 Issues / PR's related to Policy Needs: Author Feedback Needs the author to provide feedback Pattern: ALZ 🚁 Issues / PR's related to the ALZ Pattern
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants