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

Dev alz pattern #314

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

Dev alz pattern #314

wants to merge 26 commits into from

Conversation

ymehdimsft
Copy link
Contributor

@ymehdimsft ymehdimsft commented Aug 28, 2024

Overview/Summary

Replace this with a brief description of what this Pull Request fixes, changes, etc.

This PR fixes/adds/changes/removes

Adds 2 alerts (App Insights Throttling limit reached, App Insights Delete

Breaking Changes

  1. Replace me
  2. Replace me

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.)

ymehdimsft and others added 26 commits August 1, 2024 16:17
troubleshooting failing remediation
Update Deploy-AppInsightsThrottlingLimit-Alert.json
Update Deploy-ActivityLog-AppInsights-Del.json
Update Deploy-AppInsightsThrottlingLimit-Alert.json
Refining query to return only records for the selected AppInsights resource
Update Deploy-AppInsightsThrottlingLimit-Alert.json
@arjenhuitema arjenhuitema self-assigned this Aug 29, 2024
@arjenhuitema arjenhuitema added Area: Policy 📝 Issues / PR's related to Policy Pattern: ALZ 🚁 Issues / PR's related to the ALZ Pattern labels Aug 29, 2024
Copy link
Contributor

@arjenhuitema arjenhuitema left a comment

Choose a reason for hiding this comment

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

Thanks @ymehdimsft

Let's work on making the following changes:

  1. These resource types typically dont exist under the Management management group. Therefore we should include these policies not in the Management Initiative. As this regards App Insights it makes sense to add them to the Web Initiative.
  2. The policies.bicep will no longer be used, and shouldn't be modified anymore and it will be removed in the near future.

@@ -1660,6 +1660,45 @@
},
"LAWDailyCapLimitAlertState": {
"value": "true"
},
"AppInsightsThrottlingLimitSeverity": {
Copy link
Contributor

Choose a reason for hiding this comment

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

The new paramaters should go to the policyAssignmentParametersWeb section.

@@ -446,6 +446,163 @@
"displayName": "ALog Analytics Workspace Daily Cap Limit Reached 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.

Move the new policies references and paramaters to the Deploy-Web-Alerts.json

@@ -71,6 +71,8 @@ var loadPolicyDefinitions = {
loadTextContent('../../../services/OperationalInsights/workspaces/Deploy-ActivityLog-LAWorkspace-Del.json')
loadTextContent('../../../services/OperationalInsights/workspaces/Deploy-ActivityLog-LAWorkspace-KeyRegen.json')
loadTextContent('../../../services/OperationalInsights/workspaces/Deploy-LAWorkspace-DailyCapLimitReached-Alert.json')
loadTextContent('../../../services/Insights/components/Deploy-AppInsightsThrottlingLimit-Alert.json')
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the new lines from here and move them to policies-Web.bicep

@@ -167,6 +167,7 @@ var loadPolicyDefinitions = {
loadTextContent('../../../services/OperationalInsights/workspaces/Deploy-ActivityLog-LAWorkspace-Del.json')
loadTextContent('../../../services/OperationalInsights/workspaces/Deploy-ActivityLog-LAWorkspace-KeyRegen.json')
loadTextContent('../../../services/OperationalInsights/workspaces/Deploy-LAWorkspace-DailyCapLimitReached-Alert.json')
loadTextContent('../../../services/Insights/components/Deploy-AppInsightsThrottlingLimit-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 this change. This file is no longer required.

@arjenhuitema arjenhuitema changed the base branch from dev-alz-pattern to main September 2, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Policy 📝 Issues / PR's related to Policy Pattern: ALZ 🚁 Issues / PR's related to the ALZ Pattern
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants