Skip to content

PMM-14956 HA alert templates - #5730

Draft
4nte wants to merge 5 commits into
mainfrom
PMM-14956-ha-alert-templates
Draft

PMM-14956 HA alert templates#5730
4nte wants to merge 5 commits into
mainfrom
PMM-14956-ha-alert-templates

Conversation

@4nte

@4nte 4nte commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

PMM-14956

Feature build

What this adds

Five built-in alert templates for a PMM Server High Availability cluster:

Template Fires when Severity
pmm_ha_no_leader No node holds the Raft leader lease critical
pmm_ha_split_brain More than one node claims leadership critical
pmm_ha_leader_flapping Raft term changes more than N times in 10m warning
pmm_ha_node_unreachable Fewer nodes report than PMM_HA_PEERS declares warning
pmm_ha_quorum_at_risk Live voters at or below the smallest quorum majority critical

Documentation

  • documentation/docs/alert/templates_list.md - new "PMM High Availability
    templates" section with the table, the steps to create rules from the templates,
    and the coverage limitations.
  • documentation/docs/install-pmm/install-HA-clustered.md - short pointer to that
    section from the HA install page.

Implementation details

  • last_over_time(...[2m]) on every query, not a bare selector. The metrics
    database's own staleness window is longer than the alert needs. Without the
    bounded lookback a departed leader's final sample keeps counting, which delays
    pmm_ha_no_leader and can make an ordinary failover look like a split brain.
    The 2m window is deliberately shorter than every for:.
  • max by(node_id) before aggregating. Collapses duplicate series for one node,
    so extra scrape labels cannot make a single node count twice and raise a split
    brain on its own.
  • < bool / > bool rather than a bare comparison. A bare comparison filters
    the series away and Grafana evaluates the empty result as a label-less NoData
    instance, which breaks alert identity and grouping. The bool form keeps one
    instance carrying its labels.
  • pmm_ha_quorum_at_risk suppresses itself below three nodes via
    and on() (... > 2). On one- and two-node clusters the condition is permanently
    true, so it would be noise. pmm_ha_node_unreachable covers those sizes.
  • pmm_ha_node_unreachable uses unless plus an absent()-guarded fallback.
    A node that is down emits nothing, so it cannot be named from a live series. The
    first term names nodes seen in the last 6h but not the last 2m; the fallback
    catches a node that never reported or has been gone longer than 6h, and reports it
    without a name. The guard stops both terms firing for the same node.
  • A node that is down stops emitting pmm_ha_up rather than reporting 0. That
    is why the counting alerts compare against pmm_ha_expected_nodes instead of
    summing a health gauge.
  • Nothing is emitted at all when HA is disabled, which is what keeps these
    templates silent on standalone PMM.

Summary by CodeRabbit

  • New Features

    • Added five PMM High Availability alert templates covering leader loss, leader instability, unreachable nodes, quorum risks, and split-brain conditions.
    • Added the pmm_ha_expected_nodes metric to report configured cluster size.
    • Improved HA alert guidance with setup instructions, diagnostics, and remediation steps.
  • Bug Fixes

    • Improved HA peer configuration handling by trimming entries, ignoring blanks, and removing duplicates.
    • Corrected expected-node handling across HA metrics and cluster status reporting.
  • Documentation

    • Added the new HA alert templates to the catalog and documented how to configure them.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

PMM HA now normalizes peer configuration, exposes the expected node count through HA metrics, and adds five alert templates for cluster health conditions. Documentation describes template setup, alert-rule creation, remediation, and detection limitations.

Changes

PMM HA alerting and metrics

Layer / File(s) Summary
HA peer normalization
managed/cmd/pmm-managed/main.go, managed/cmd/pmm-managed/main_test.go
PMM_HA_PEERS entries are trimmed, empty entries are ignored, and duplicates are removed with warnings.
Expected-node metrics
managed/services/ha/haservice.go, managed/services/ha/ha.go, managed/services/ha/ha_metrics.go, managed/services/ha/ha_metrics_test.go
HA metrics now expose the configured expected node count. Tests cover disabled HA, startup values, cluster sizes, and metric descriptors.
HA alert templates
managed/data/alerting-templates/ha_*.yml
Added alerts for missing leaders, split-brain, leader flapping, unreachable nodes, and quorum risk.
HA alert documentation
documentation/docs/alert/templates_list.md, documentation/docs/install-pmm/install-HA-clustered.md
Documented the alert catalog, setup steps, alert-rule creation, remediation guidance, and detection limitations.

Possibly related PRs

  • percona/pmm#5704: Both changes update HA startup configuration parsing in managed/cmd/pmm-managed/main.go.
  • percona/pmm#5718: Both changes extend PMM HA alert-template documentation and coverage.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: adding PMM High Availability alert templates.
Description check ✅ Passed The description provides the ticket, feature-build link, implementation details, documentation changes, alert coverage, and behavior when HA is disabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the documentation Documentation changes label Aug 4, 2026
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.21%. Comparing base (31318c7) to head (24a1e8f).
⚠️ Report is 91 commits behind head on main.

Files with missing lines Patch % Lines
managed/services/ha/haservice.go 44.44% 5 Missing ⚠️
managed/cmd/pmm-managed/main.go 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5730      +/-   ##
==========================================
+ Coverage   43.59%   45.21%   +1.61%     
==========================================
  Files         415      218     -197     
  Lines       43134    27867   -15267     
==========================================
- Hits        18804    12599    -6205     
+ Misses      22454    13915    -8539     
+ Partials     1876     1353     -523     
Flag Coverage Δ
admin ?
agent ?
managed 45.21% <82.85%> (+2.22%) ⬆️
vmproxy ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@theTibi

theTibi commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@managed/cmd/pmm-managed/main.go`:
- Around line 1245-1256: Update parseHAPeers to accept the existing
*logrus.Entry main logger, and replace the package-level logrus.Warnf duplicate
warning with a structured field for the peer while preserving the warning
message and duplicate-handling behavior. Update its caller to pass main.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 83f38b89-81fb-4540-b835-65783dc99e13

📥 Commits

Reviewing files that changed from the base of the PR and between b0e9283 and 24a1e8f.

📒 Files selected for processing (13)
  • documentation/docs/alert/templates_list.md
  • documentation/docs/install-pmm/install-HA-clustered.md
  • managed/cmd/pmm-managed/main.go
  • managed/cmd/pmm-managed/main_test.go
  • managed/data/alerting-templates/ha_leader_flapping.yml
  • managed/data/alerting-templates/ha_no_leader.yml
  • managed/data/alerting-templates/ha_node_unreachable.yml
  • managed/data/alerting-templates/ha_quorum_at_risk.yml
  • managed/data/alerting-templates/ha_split_brain.yml
  • managed/services/ha/ha.go
  • managed/services/ha/ha_metrics.go
  • managed/services/ha/ha_metrics_test.go
  • managed/services/ha/haservice.go

Comment on lines +1245 to +1256
func parseHAPeers(peers string) []string {
var nodes []string
seen := make(map[string]struct{})

for node := range strings.SplitSeq(peers, ",") {
node = strings.TrimSpace(node)
if node == "" {
continue
}
if _, ok := seen[node]; ok {
logrus.Warnf("Ignoring duplicate entry %q in PMM_HA_PEERS.", node)
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use a structured logrus.Entry for duplicate-peer warnings.

Line 1255 uses the package-level logrus.Warnf. Pass the existing main entry to parseHAPeers. Log the peer as a field so the message retains component context and supports structured filtering.

As per coding guidelines, use “structured logging, such as s.l.WithField("key", value).Error("message"), and pass *logrus.Entry rather than *logrus.Logger.”

Proposed fix
-	nodes := parseHAPeers(*haPeers)
+	nodes := parseHAPeers(l, *haPeers)

-func parseHAPeers(peers string) []string {
+func parseHAPeers(l *logrus.Entry, peers string) []string {
...
-			logrus.Warnf("Ignoring duplicate entry %q in PMM_HA_PEERS.", node)
+			l.WithField("peer", node).Warn("Ignoring duplicate entry in PMM_HA_PEERS.")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func parseHAPeers(peers string) []string {
var nodes []string
seen := make(map[string]struct{})
for node := range strings.SplitSeq(peers, ",") {
node = strings.TrimSpace(node)
if node == "" {
continue
}
if _, ok := seen[node]; ok {
logrus.Warnf("Ignoring duplicate entry %q in PMM_HA_PEERS.", node)
continue
func parseHAPeers(l *logrus.Entry, peers string) []string {
var nodes []string
seen := make(map[string]struct{})
for node := range strings.SplitSeq(peers, ",") {
node = strings.TrimSpace(node)
if node == "" {
continue
}
if _, ok := seen[node]; ok {
l.WithField("peer", node).Warn("Ignoring duplicate entry in PMM_HA_PEERS.")
continue
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@managed/cmd/pmm-managed/main.go` around lines 1245 - 1256, Update
parseHAPeers to accept the existing *logrus.Entry main logger, and replace the
package-level logrus.Warnf duplicate warning with a structured field for the
peer while preserving the warning message and duplicate-handling behavior.
Update its caller to pass main.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Documentation changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants