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

Ees 5686 add public api performance metric alerts #5463

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

duncan-at-hiveit
Copy link
Collaborator

@duncan-at-hiveit duncan-at-hiveit commented Dec 12, 2024

This PR adds in various alerts related to performance to resources of the Public API.

This introduces the use of dynamic threshold alerts, which use machine learning to determine normal usage patterns for the resources they monitor and determine a dynamic threshold against which to test the performance of the metric they're tracking.

Alerts included in this PR are:

  • Response / end-to-end latency times for:
    • Application Gateway
    • Container Apps
    • File Services
    • Storage Accounts
  • Longest query and longest transaction for PostgreSQL Flexible Server

…ileShare.bicep, and correct setting up of alert flags for file shares in functionApp.bicep
…blic-api-performance-metric-alerts-alerts-with-components

EES-5686 - moving alert creation into individual component files rather than higher-level templates
@@ -0,0 +1,82 @@
import { EvaluationFrequency, MetricName, DynamicMetricOperator, ResourceType, TimeAggregation, WindowSize, Severity, Sensitivity, severityMapping } from 'types.bicep'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be formatted into a multi-line import i.e.

import {
  EvaluationFrequency,
  ...
} from 'types.bicep'

Note that the built-in Bicep formatter would normally do this automatically.

You can format directly from VS Code by running Format Document (using the command palette). It should prompt you to configure a formatter for Bicep, which I think it does automatically, but I you can also manually configure it in your settings.json.

Would suggest we create a new ticket to add a pre-commit hook to run the Bicep formatter (like how we run Prettier) and then format any other Bicep files so we have a good formatting baseline.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good - I've created the following ticket ta: https://dfedigital.atlassian.net/browse/EES-5748

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(and formatted the file using CS Code in the meantime)

Comment on lines 28 to 34
param sensitivity Sensitivity = 'High'

param minFailingPeriodsToAlert int = 1

param numberOfEvaluationPeriods int = 1

param ignoreDataBefore string?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these have descriptions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed they should! I've added some now - quite descriptive ones as I feel that these various parameters are a little mysterious unless you know alerts inside and out.

Copy link
Collaborator

Choose a reason for hiding this comment

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

flexibleServers for the directory maybe isn't the best name as it doesn't convey that these alerts are for the Postgres database.

Perhaps postgreSql or postgres would be better?

Or, if it's possible to abstract these alerts for any database type, maybe db could work? (probably needs resourceType to be configurable though)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was mostly just to stick with the folder name convention that matched the Azure type of the resources that the alerts were for (in this case Microsoft.DBforPostgreSQL/flexibleServers). For instance, sites rather than functionApps or appServices as an example. I've got no problem changing it though, and so have done to postgreSqlFlexibleServers. At some point, the postgresqlDatabase.bicep file could do with renaming to postgreSqlFlexibleServer.bicep too.

I think that these alerts will be specific to PostgreSQL FS btw rather than generic to all dbs.

@duncan-at-hiveit duncan-at-hiveit force-pushed the EES-5686-add-public-api-performance-metric-alerts branch from 960e4ed to aaaccaf Compare December 20, 2024 17:01
…micMetricAlert.bicep params that were missing descriptions. Formatted file for cleaner import block.
@duncan-at-hiveit duncan-at-hiveit force-pushed the EES-5686-add-public-api-performance-metric-alerts branch from aaaccaf to 6cefbfc Compare December 20, 2024 17:06
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