backend: add alerting for stuck async operations#5952
Conversation
There was a problem hiding this comment.
Pull request overview
Adds backend-driven alerting for async operations that appear “stuck” in non-terminal phases, complementing existing SLO/terminal-phase alerting and improving visibility when operations don’t make progress.
Changes:
- Registers a new backend async-operations PrometheusRule file in the shared service alert bundle.
- Introduces backend async-operation “stuck” alert rules (plus promtool tests) for cluster, node pool, credential, and external auth operations.
- Regenerates the Azure Monitor Prometheus rule-group Bicep output to deploy the new alerts.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| observability/alerts-sl-services.yaml | Adds the new backend async-operations PrometheusRule YAML to the shared alert set. |
| dev-infrastructure/modules/metrics/rules/generatedPrometheusAlertingRules.bicep | Generated Azure Monitor rule group for the new backend async-operation stuck alerts. |
| backend/alerts/backend-async-operations-prometheusRule.yaml | Defines the new stuck-operation alert rules and alert metadata. |
| backend/alerts/backend-async-operations-prometheusRule_test.yaml | Adds promtool tests validating the new alert rules. |
machi1990
left a comment
machi1990
left a comment
There was a problem hiding this comment.
lgtm, intentionally not tagging it to give a member of SL team to see if the wiring is done correctly and wether the missing runbook is going to be a problem.
|
Will rebase on #5955 |
SudoBrendan
left a comment
SudoBrendan
left a comment
There was a problem hiding this comment.
Thanks for getting this Steve - been on my list of top alerts for a while; lots of value here.
| summary: 'Node pool create operation stuck' | ||
| description: 'Node pool create operation for {{ $labels.resource_id }} has been in {{ $labels.phase }} phase for over 20 minutes (e2e timeout: NodePoolCreationTimeout).' | ||
| runbook_url: TBD | ||
| # Node pool update: e2e timeout = 45 minutes (NodePoolVersionUpgradeTimeout) |
There was a problem hiding this comment.
You pulled this straight from the test case, but I wonder if that test itself is wrong and conflating various kinds of "update", maybe it was from the before-times when we hadn't decided to fire-and-forget on OCP version upgrades? As I understand from a backend Operation perspective, the actual LRO should go basically from Accepted -> Succeeded/Failed immediately on an OCP upgrade, right? We should maybe consider a shorter timeout, so that when customers bump their autoscale max from 8 -> 9 and that gets locked it doesn't take 45m to fire (here, and in the test, honestly)
However... while investigating this - I did discover that there are some LROs that we could expect to go longer than 45m or even an hour - internal discussion here.
There was a problem hiding this comment.
Yeah, if/when we change the e2e, we can change this - and you're 100% right that there are possible updates that could take a long time. If and unless we update the metric to classify the 'type' of update LRO, we can't subdivide. Time will tell if it is meaningful or not.
stevekuznetsov
force-pushed
the
skuznets/alert-async-operations
branch
from
c85f794 to
6a0b4a6
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bennerv, stevekuznetsov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
We have end-to-end test assertions that our async operations never take longer than $timeout. We have seen a few cases where customer interactions with the ARM API ended up stuck, and had no visibility into this, even though our end-to-end test suite, had it run on that region at that time, would have clearly caught this. This new set of alerts is an assertion from the backend that async operations never spend more than a certain amount of time in a particular phase. We still benefit from having our async operations not having a clear timeout in the backend, as fixing the underlying problem will cause them to get un-stuck and finish, but we want to know if we're failing to progress. Existing SLO alerting is for failed/terminal phases of async operations, which do not trigger for 'stuck' operations which never exit 'progressing.' Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com>
stevekuznetsov
force-pushed
the
skuznets/alert-async-operations
branch
from
6a0b4a6 to
ab53a97
Compare
|
New changes are detected. LGTM label has been removed. |
|
/label lgtm |
| # These alerts fire when an async operation has been running longer than | ||
| # the corresponding e2e test timeout. Thresholds are anchored to the | ||
| # canonical timeout constants in test/util/framework/constants.go. |
|
/retest |
1 similar comment
|
/retest |
|
/test all |
2 similar comments
|
/test all |
|
/test all |
|
@stevekuznetsov: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
We have end-to-end test assertions that our async operations never take longer than $timeout. We have seen a few cases where customer interactions with the ARM API ended up stuck, and had no visibility into this, even though our end-to-end test suite, had it run on that region at that time, would have clearly caught this.
This new set of alerts is an assertion from the backend that async operations never spend more than a certain amount of time in a particular phase. We still benefit from having our async operations not having a clear timeout in the backend, as fixing the underlying problem will cause them to get un-stuck and finish, but we want to know if we're failing to progress.
Existing SLO alerting is for failed/terminal phases of async operations, which do not trigger for 'stuck' operations which never exit 'progressing.'