Skip to content

Add Azure Monitoring Module to enable default alert criteria and notifications #132

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
Merged
merged 8 commits into from
May 30, 2019
Merged

Add Azure Monitoring Module to enable default alert criteria and notifications #132

merged 8 commits into from
May 30, 2019

Conversation

TechnicallyWilliams
Copy link
Contributor

@TechnicallyWilliams TechnicallyWilliams commented May 22, 2019 •
edited
Loading

All Submissions:

  • [YES] Have you followed the guidelines in our Contributing document?
  • [YES] Have you added an explanation of what your changes do and why you'd like us to include them?
  • [YES] I have updated the documentation accordingly.
  • [NO] I have added tests to cover my changes.
  • [NA] All new and existing tests passed.
  • [YES] My code follows the code style of this project.
  • [YES] I ran lint checks locally prior to submission.
  • [YES] Have you checked to ensure there aren't other open Pull Requests for the same update/change?

What is the current behavior?

  • Cobalt templates currently have the option to monitor metrics at the individual app service level by leveraging the App-Insights module. However, this new module introduces a starting point for monitoring metrics emitted by other Azure resources. (ex. App Service Plan, App Gateway, Traffic Manager, etc.)

Issue Number: #86
Fixes #86

What is the new behavior?

  • The new module introduced in this PR deploys the Azure Monitor Service. It is a custom Terraform module called App-Monitoring that Cobalt Templates can use in order to assign configurable metric alert criteria to individual Azure Resources for monitoring.

  • In addition to the metric alert criteria, this module introduces configurable action groups that when paired with metric alert criteria can trigger event types like e-mail notifications.

  • In conclusion, the App-Monitoring module is now being referenced by the Azure Simple Template which now adds the following default behavior to the template:

    • The deployment of the Azure Monitor Service depends on if a template provides an e-mail address to the App-Monitoring* module. Otherwise, the App-Monitoring module deploys nothing.
    • Azure Simple Template default is now set up to deploy CPU resource monitoring of the App Service Plan Service.

Does this introduce a breaking change?

  • [NO]

Any relevant logs, error output, etc?

image

Other information

Mapping metric alert criteria to a resource:
image

@TechnicallyWilliams TechnicallyWilliams changed the title Add Azure Monitoring Module to enable default alert criteria and notifications Add Azure Monitoring Module to enable default alert criteria May 23, 2019
@TechnicallyWilliams TechnicallyWilliams changed the title Add Azure Monitoring Module to enable default alert criteria Add Azure Monitoring Module to enable default alert criteria and notifications May 23, 2019
@TechnicallyWilliams TechnicallyWilliams marked this pull request as ready for review May 24, 2019 02:37
nmiodice
nmiodice reviewed May 24, 2019
View reviewed changes
Copy link

@nmiodice nmiodice left a comment

Choose a reason for hiding this comment

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

Looks great! A few nits/comments and questions but overall it looks awesome.

erikschlegel
erikschlegel requested changes May 28, 2019
View reviewed changes
Copy link
Contributor

@erikschlegel erikschlegel 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 looking great @TechnicallyWilliams . Nice work. Just left some minor nits below.

@TechnicallyWilliams
init app monitoring module; add module ref to azure simple template
664bb83 
added app monitoring readme; updated variable names

Added and integrated app monitoring into azure simple template
@TechnicallyWilliams
update readme; update app monitoring variables
d3ae86b
@TechnicallyWilliams
removed default e-mail alerts
e554e4d
@TechnicallyWilliams
update readme; restored default e-mail configuration
fd11826
@TechnicallyWilliams
removed line from azure simple readme; removed tfplan binary file
6a8921c
@TechnicallyWilliams
format variables.tf
42b7b97 
update readme

Update Readme.md

Update README.md

Update README.md

Redesigned README.md

provide clarity regarding resourceIds and scaling dimensions

update README; remove comments

Update README.md and azure simple variables

added attributes section to README.md
@TechnicallyWilliams TechnicallyWilliams force-pushed the dexterw/features/app-monitoring branch from 3f14349 to 42b7b97 Compare May 29, 2019 00:25
erikschlegel
erikschlegel approved these changes May 30, 2019
View reviewed changes
Copy link
Contributor

@erikschlegel erikschlegel left a comment

Choose a reason for hiding this comment

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

Woot! Great work @TechnicallyWilliams. Happy to see this flexible solution to add monitoring to a fleet of app service containers.

@TechnicallyWilliams TechnicallyWilliams merged commit 6aedc0f into master May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmiodice nmiodice nmiodice left review comments

@erikschlegel erikschlegel erikschlegel approved these changes

@hxlnt hxlnt Awaiting requested review from hxlnt

@ianphil ianphil Awaiting requested review from ianphil

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Azure monitoring module
3 participants
@TechnicallyWilliams @nmiodice @erikschlegel