Skip to content

MON-4059: split TelemeterClientFailures into send/retrieve alerts - #550

Open
slashpai wants to merge 1 commit into
openshift:mainfrom
slashpai:MON-4059
Open

MON-4059: split TelemeterClientFailures into send/retrieve alerts#550
slashpai wants to merge 1 commit into
openshift:mainfrom
slashpai:MON-4059

Conversation

@slashpai

@slashpai slashpai commented Nov 19, 2024

Copy link
Copy Markdown
Member

Replace TelemeterClientFailures with two alerts on metricsclient_http_requests_total:

  • TelemeterClientSendErrors (federate_to) — upload failures to telemeter
  • TelemeterClientRetrieveErrors (federate_from) — federation failures from Prometheus
    Both fire when non-2xx rate > 20% for 1h. Descriptions distinguish 4xx (cluster-admin actionable) from 5xx (Red Hat service).

Ref: https://github.com/openshift/runbooks/blob/master/alerts/cluster-monitoring-operator/TelemeterClientFailures.md#diagnosis

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 19, 2024
@openshift-ci-robot

openshift-ci-robot commented Nov 19, 2024

Copy link
Copy Markdown
Contributor

@slashpai: This pull request references MON-4059 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

Details

In response to this:

Use the new metricsclient_http_requests_total metric which would tell the difference between 4xx errors (e.g. bad pull secret) and
5xx (issue on Red Hat side).

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 19, 2024
@openshift-ci-robot

openshift-ci-robot commented Nov 19, 2024

Copy link
Copy Markdown
Contributor

@slashpai: This pull request references MON-4059 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

Details

In response to this:

Use the new metricsclient_http_requests_total metric which would tell the difference between 4xx errors (e.g. bad pull secret) and
5xx (issue on Red Hat side).

Ref: https://github.com/openshift/runbooks/blob/master/alerts/cluster-monitoring-operator/TelemeterClientFailures.md#diagnosis

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

@simonpasquier simonpasquier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd recommend to create 2 alerting rules: one for the federate_from client and another one for the federate_to client. They can have the same name but different descriptions.

sum by (namespace) (
rate(federate_requests_total{job="telemeter-client"}[15m])
) > 0.2
sum by(client, status_code) (rate(metricsclient_http_requests_total{status_code!~"200"}[15m])) > 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(suggestion) we need to preserve the namespace label. Let's also evaluate the rate of failed requests.

Suggested change
sum by(client, status_code) (rate(metricsclient_http_requests_total{status_code!~"200"}[15m])) > 0
sum by(client, status_code,namespace) (rate(metricsclient_http_requests_total{status_code!~"2..",job="telemeter-client"}[15m]))
/
on(client, namespace) group_left() sum by(client, namespace) (rate(metricsclient_http_requests_total{job="telemeter-client"}[15m])) > 0.2

@openshift-bot

Copy link
Copy Markdown
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 22, 2025
@openshift-bot

Copy link
Copy Markdown
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci Bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 31, 2025
@openshift-bot

Copy link
Copy Markdown
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci Bot closed this Jun 30, 2025
@openshift-ci

openshift-ci Bot commented Jun 30, 2025

Copy link
Copy Markdown
Contributor

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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

@simonpasquier

Copy link
Copy Markdown
Contributor

/reopen

@openshift-ci openshift-ci Bot reopened this Jun 30, 2025
@openshift-ci

openshift-ci Bot commented Jun 30, 2025

Copy link
Copy Markdown
Contributor

@simonpasquier: Reopened this PR.

Details

In response to this:

/reopen

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

@openshift-ci-robot

openshift-ci-robot commented Jun 30, 2025

Copy link
Copy Markdown
Contributor

@slashpai: This pull request references MON-4059 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set.

Details

In response to this:

Use the new metricsclient_http_requests_total metric which would tell the difference between 4xx errors (e.g. bad pull secret) and
5xx (issue on Red Hat side).

Ref: https://github.com/openshift/runbooks/blob/master/alerts/cluster-monitoring-operator/TelemeterClientFailures.md#diagnosis

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

@simonpasquier

Copy link
Copy Markdown
Contributor

remove-lifecycle rotten

@simonpasquier

Copy link
Copy Markdown
Contributor

/remove-lifecycle rotten

@openshift-ci openshift-ci Bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 30, 2025
@openshift-ci

openshift-ci Bot commented Jun 30, 2025

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: slashpai

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot

Copy link
Copy Markdown
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 29, 2025
@openshift-bot

Copy link
Copy Markdown
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci Bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 29, 2025
@openshift-bot

Copy link
Copy Markdown
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci Bot closed this Nov 29, 2025
@openshift-ci openshift-ci Bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Dec 1, 2025
@openshift-bot

Copy link
Copy Markdown
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 2, 2026
@juzhao

juzhao commented Mar 9, 2026

Copy link
Copy Markdown

/remove-lifecycle stale

@openshift-ci openshift-ci Bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 9, 2026
@juzhao

juzhao commented Mar 9, 2026

Copy link
Copy Markdown

/test e2e-aws-ovn

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2026
@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: a8267599-871a-4e34-8425-980f5f3b3f2e

📥 Commits

Reviewing files that changed from the base of the PR and between 9582830 and 41b6b30.

📒 Files selected for processing (1)
  • test/rulestests.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The change replaces TelemeterClientFailures with separate TelemeterClientSendErrors and TelemeterClientRetrieveErrors alerts. Each alert monitors a direction-specific non-2xx request rate and uses direction-specific guidance. The Makefile generates and validates tmp/alert-rules.yaml. Tests cover thresholds, one-hour firing behavior, labels, annotations, and successful requests.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 41b6b

The directional Telemeter alerts are covered for threshold, duration, and successful-request behavior, with no current merge-blocking risk identified.

Suggested reviewers: jiri-fiala

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: splitting TelemeterClientFailures into separate send and retrieve alerts.
Description check ✅ Passed The description directly explains the alert replacement, metric, directions, threshold, duration, and failure categories covered by the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
Stable And Deterministic Test Names ✅ Passed PASS. The pull request changes Makefile, Jsonnet, PrometheusRule YAML, and promtool rule tests. It adds no Ginkgo test titles such as It(), Describe(), Context(), or When(). The added `alertna…
Test Structure And Quality ✅ Passed PASS: The pull request adds Prometheus promtool rule tests in test/rulestests.yaml, not Ginkgo tests. The exact diff contains no It, BeforeEach, AfterEach, Eventually, Consistently, reso…
Microshift Test Compatibility ✅ Passed PASS: The pull request adds Prometheus rule tests in test/rulestests.yaml, not Ginkgo tests. The changed files contain no new It(), Describe(), Context(), or When() test declarations. Theref…
Single Node Openshift (Sno) Test Compatibility ✅ Passed The pull request changes only Makefile, Jsonnet, PrometheusRule YAML, and promtool rule-test YAML. The diff adds no Go or Ginkgo e2e tests, and the changed files contain no It(), Describe(), Context()…
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The pull request does not introduce topology-dependent scheduling constraints. The diff changes Makefile rule validation, PrometheusRule alert expressions and annotations, and alert tests. The J…
Ote Binary Stdout Contract ✅ Passed PASS. The PR changes only Makefile, Jsonnet, PrometheusRule YAML, and rule-test YAML files. The diff contains no Go, OTE, Ginkgo suite, or process-entrypoint changes. It introduces no process-level st…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS. The PR adds no Ginkgo e2e tests and changes no Go or shell test files. The added tests are Prometheus promtool rule tests in test/rulestests.yaml; they use synthetic metric series and do not…
No-Weak-Crypto ✅ Passed PASS: The pull request changes alert expressions, rule generation, and rule tests. The added lines introduce no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, cryptographic implementation, or secret/token …
Container-Privileges ✅ Passed PASS: The pull request changes a PrometheusRule, Jsonnet alert definitions, Makefile rules, and Prometheus rule tests. The changed files introduce no privileged: true, hostPID, hostNetwork, `hos…
No-Sensitive-Data-In-Logs ✅ Passed PASS. The PR changes Makefile, Jsonnet, PrometheusRule YAML, and rule tests. It adds no runtime logging and no log message containing credentials, PII, session IDs, customer data, or internal hostname…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@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: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 7d316c0c-febf-473e-8265-7c5f5b2ef4e6

📥 Commits

Reviewing files that changed from the base of the PR and between 0e78993 and 6d027de.

📒 Files selected for processing (1)
  • test/rulestests.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.

Comment thread test/rulestests.yaml
@slashpai
slashpai marked this pull request as draft September 8, 2026 13:57
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 8, 2026
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2026

@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


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: ca877c23-d779-4084-b583-6cfd716d2211

📥 Commits

Reviewing files that changed from the base of the PR and between 6d027de and 9582830.

📒 Files selected for processing (1)
  • test/rulestests.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Comment thread test/rulestests.yaml Outdated
@slashpai

slashpai commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

Tested the alert TelemeterClientSendErrors by patching telemeter client deployment in clusterbot cluster by replacing --to=$(TO) with separate endpoints

--to-auth
--to-upload

with wrong value set for --to-upload and reducing alert time to test firing

Screenshot 2026-09-09 at 12 03 51 PM

To test TelemeterClientRetrieveErrors, deleted role binding telemeter-client-view, reduced alert time to test firing and restarted telemeter deployment

Screenshot 2026-09-09 at 12 40 19 PM

@slashpai
slashpai marked this pull request as ready for review September 9, 2026 07:11
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 9, 2026
Replace deprecated federate_requests_* based alert with two focused
alerts using metricsclient_http_requests_total{status_code!~"2.."}:

- TelemeterClientSendErrors (client="federate_to")
- TelemeterClientRetrieveErrors (client="federate_from")

Each alert description distinguishes 4xx (user-actionable) from
5xx (Red Hat service issue). Threshold: >20% error rate for 1h.

Also extend check-rules to validate alert rules and add promtool
unit tests in rulestests.yaml.

Signed-off-by: Jayapriya Pai <[email protected]>
@openshift-ci-robot

openshift-ci-robot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@slashpai: This pull request references MON-4059 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Replace TelemeterClientFailures with two alerts on metricsclient_http_requests_total:

  • TelemeterClientSendErrors (federate_to) — upload failures to telemeter
  • TelemeterClientRetrieveErrors (federate_from) — federation failures from Prometheus
    Both fire when non-2xx rate > 20% for 1h. Descriptions distinguish 4xx (cluster-admin actionable) from 5xx (Red Hat service).

Ref: https://github.com/openshift/runbooks/blob/master/alerts/cluster-monitoring-operator/TelemeterClientFailures.md#diagnosis

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

@slashpai slashpai changed the title MON-4059: Update TelemeterClientFailures alert MON-4059: split TelemeterClientFailures into send/retrieve alerts Sep 9, 2026
@openshift-ci

openshift-ci Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@slashpai: all tests passed!

Full PR test history. Your PR dashboard.

Details

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

@slashpai

slashpai commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

@simonpasquier I have revived this PR and updated per suggestions. Included tests and locally tested as above.

Take a look when you get chance

The telemeter client in namespace {{ $labels.namespace }} has {{ $value | humanizePercentage }} error rate when sending metrics to the telemeter service.
4xx errors typically indicate an authentication or authorization issue — check that the global pull secret is valid and up to date:
oc get secret pull-secret -n openshift-config -o jsonpath='{.data.\.dockerconfigjson}' | base64 -d
5xx errors indicate a server-side issue on Red Hat telemeter service and are not actionable by the cluster administrator.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤔 do we even need to alert in this case? maybe we just need an info alert for 5xx responses.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

info is good.

@simonpasquier simonpasquier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what do we do with the runbook? obviously we need to keep it for supported OCP releases but we can either extend it to cover the new alerting rules or create dedicated runbooks for each (my preference would be option 2 as it's more obvious for customers and we could eventually remove the old runbook).

@slashpai

slashpai commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

what do we do with the runbook? obviously we need to keep it for supported OCP releases but we can either extend it to cover the new alerting rules or create dedicated runbooks for each (my preference would be option 2 as it's more obvious for customers and we could eventually remove the old runbook).

Ya. Since we can

what do we do with the runbook? obviously we need to keep it for supported OCP releases but we can either extend it to cover the new alerting rules or create dedicated runbooks for each (my preference would be option 2 as it's more obvious for customers and we could eventually remove the old runbook).

We can add new ones since we currently one runbook per alert

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants