Feat : Expose Degraded Condition as Prometheus Metric for external auth by m-mandhare · Pull Request #7583 · Azure/ARO-HCP

Mansi Mandhare (m-mandhare) · 2026-10-16T12:57:04Z

Expose Degraded Condition as Prometheus Metric

What

Added backend_externalauth_degraded{resource_id, controller} (0/1 gauge) to ExternalAuthUserFacingConditionsDegradedAggregator, registered via promauto.With(legacyregistry.Registerer()) at controller-construction time, following the existing metrics pattern.
Set to 1 when ExternalAuth.Status.UserFacingConditions[Degraded] is True, 0 otherwise — recorded on every sync regardless of whether the Cosmos write happens (no-op path included), so the metric never silently goes stale.

Why

For Grafana dashboard visibility

Testing

Manual testing -

Testing is required for feature completion and tests should be part of the pull
request along with the feature changes.

Describe the testing provided. If you did not add tests, provide a clear
justification.

Unit tests

openshift-merge-bot · 2026-10-16T12:57:08Z

Pipeline controller notification

This PR uses the pipeline controller for second-stage tests. Selection and triggering follow the repository configuration.

Use /test ? to list jobs and /pipeline help for commands.

openshift-ci · 2026-10-16T12:57:12Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: m-mandhare
Once this PR has been reviewed and has the lgtm label, please assign machi1990 for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

backend/OWNERS

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

openshift-ci · 2026-10-16T12:57:15Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copilot

🟡 Changes recommended

Duplicate registration can panic, and metric lifecycle handling can leave missing, stale, or inaccurate samples.

5 open findings

What changed in this PR

Adds a backend Prometheus gauge for external-auth degradation, supporting Grafana visibility for ARO-29559.

Changes:

Records degradation by resource ID and controller.
Allows syncers to handle missing external-auth resources.
Adds gauge-value and deletion-path unit tests.

File	Description
backend/pkg/utils/controllerutils/external_auth_watching_controller.go	Calls syncers for missing resources while skipping controller-document writes.
backend/pkg/controllers/externalauth/status/externalauth_userfacing_conditions_degraded_aggregator.go	Registers and updates the degradation gauge.
backend/pkg/controllers/externalauth/status/externalauth_userfacing_conditions_degraded_aggregator_test.go	Tests condition values and gauge reset on NotFound.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+	// Always call syncer.SyncOnce so syncers can handle NotFound themselves
+	// (e.g., to set Prometheus gauge to 0 for deleted resources).
+	syncErr := c.syncer.SyncOnce(ctx, key)


Simon Wiencki (swiencki) · 2026-10-20T16:28:10Z

 		resourcesDBClient:                 resourcesDBClient,
+		degradedGauge: promauto.With(legacyregistry.Registerer()).NewGaugeVec(
+			prometheus.GaugeOpts{
+				Name: "backend_externalauth_degraded",


Nothing in observability reads backend_externalauth_degraded yet, including the dashboard in #7267, so can you link the panel or alert that will use it and the good/total query you have in mind?

Simon Wiencki (swiencki) · 2026-10-20T17:21:14Z

+		value = 1.0
+	}
+
+	c.degradedGauge.With(prometheus.Labels{


A missing client secret or a bad issuer URL from the customer sets this to 1 just like a service failure, so could we add a small reason label (customer config vs service) so the SLI can keep customer mistakes out of the error budget?

Simon Wiencki (swiencki) · 2026-10-20T17:21:15Z

 	replacement := existing.DeepCopy()
 	apimeta.SetStatusCondition(&replacement.Status.UserFacingConditions, aggregated)
+
+	// Record the gauge regardless of whether we write to Cosmos


The OIDC clients controller keeps the last condition when the HostedCluster is missing or ActiveVersions is empty, so a last-evaluated timestamp gauge would stop a stuck pipeline from looking healthy.

Raised ticket for this changes - https://redhat.atlassian.net/browse/ARO-30411

Simon Wiencki (swiencki) · 2026-10-20T17:21:15Z

 	return nil
 }

+// recordDegradedGauge sets backend_externalauth_degraded to 1 if the


Right after a create or update a nil Status.Configuration flips this to 1, so how would the SLI tell normal rollout lag from a real failure, maybe with a grace window on operation phase?

Copilot

🟡 Changes recommended

Per-component metrics persist after resource deletion, and their values and cleanup lack metric assertions.

3 open issues

4 resolved since last review

_{Review effort: Balanced}


+	// Emit per-component degradation gauge. Clear all previous component
+	// series for this resource first so removed/renamed clients don't leak.
+	c.recordPerComponentGauges(externalAuth, degradedSet)


+
+// clearPerComponentGauges removes all per-component series for the given
+// external auth (e.g. when clients are empty or the version is unsupported).
+func (c *externalAuthOIDCClientsDegradedController) clearPerComponentGauges(externalAuth *coreapi.ExternalAuth) {


openshift-merge-bot · 2026-10-21T08:56:04Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-parallel

Simon Wiencki (swiencki) · 2026-10-21T17:10:21Z

 			"Unix timestamp when the external auth was created.",
 		),
+		degraded: prometheus.NewGaugeVec(prometheus.GaugeOpts{
+			Name: "backend_externalauth_degraded",


#7267 builds a 95 percent non-degraded SLI on this gauge, so could you add a reason label (customer config vs service) so a missing client secret or bad issuer URL stays out of the error budget?

Currently degradation controller is not providing any reason label as customer config or service. Hence it is not possible to directly add it on metrics. First it has to add it in external auth userfacing condition controller. And this is also not correct according to current design

Simon Wiencki (@swiencki) To get this information RFE ticker has been raised for Hypershift team. This changes will be needed from them. Hence cannot be added in this scope.

Thanks, we can drop the scope from the pr but can we card a follow up in https://redhat.atlassian.net/browse/ARO-28393

Simon Wiencki (swiencki) · 2026-10-21T17:10:21Z

+	}
+
+	value := 0.0
+	if cond.Status == metav1.ConditionTrue {


A nil Status.Configuration right after a create or update shows up as degraded, so how will the SLI tell normal rollout lag apart from a real failure?

The nil-Configuration window only opens after four SyncOnce guards pass (ReadDesire observed, HC exists, ActiveVersions populated, version ≥ 4.20.41), so provision_state is still provisioning at that point. The SLI excludes it at the PromQL level with unless on (resource_id) backend_externalauth_provision_state{phase=~"accepted|provisioning"}, keeping the gauge as ground truth and the grace policy tunable without code changes. Updates to already-provisioned ExternalAuths don't hit this path at all since Configuration is already populated.

That makes sense for the gauge, but I can't find the unless on (resource_id) backend_externalauth_provision_state{phase=~"accepted|provisioning"} exclusion in this PR, 7723 or main, so can you point me at where it lives or note it as a requirement for whichever PR adds the SLI?

Added a provisioning-state guard that skips emitting the backend_externalauth_degraded gauge when the ExternalAuth's provisioning state is non-terminal (Accepted, Provisioning, Updating, Deleting, AwaitingSecret). The gauge is only emitted once the state reaches Succeeded, Failed, or Canceled.

Copilot

🔵 Needs a closer look

Per-client metric cleanup does not reliably remove deleted resources, leaving stale health values exposed.

3 open issues

_{Review effort: Balanced}

openshift-merge-bot · 2026-10-26T07:41:52Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-parallel

…ient status conditions

Copilot

🟡 Changes recommended

Production deletion bypasses ExternalAuth gauge cleanup, leaving stale metrics exported, while nonterminal test fixtures prevent expected degraded series from being created.

4 open issues

_{Review effort: Balanced}

Mansi Mandhare (m-mandhare) · 2026-10-28T15:52:36Z

/retest

1. Remove Oldest Degraded Eval panel — metric (backend_externalauth_degraded_last_evaluation_seconds) does not exist on main or in Azure#7583 2. Remove or vector(0) from True Availability — show No data when backend_externalauth_degraded is absent instead of misleading 0% 3. Remove Error Budget Remaining, Error Budget Burn Rate, and Degraded Health Budget panels — these are point-in-time ratios with no window and count AwaitingSecret as bad; hold until windowed good/total rules from ARO-25935 exist 4. Add or vector(0) to Timeliness SLO numerator — show 0% when every create runs late, instead of No data 5. Apply exclude_subs filter to Create >15m, Update >10m, Delete >15m stuck stats, Active Operations table, and In-Flight Operation Duration — matches the stuck alerts which exclude internal subs Co-authored-by: Cursor <cursoragent@cursor.com>

1. Remove Oldest Degraded Eval panel — metric (backend_externalauth_degraded_last_evaluation_seconds) does not exist on main or in Azure#7583 2. Remove or vector(0) from True Availability — show No data when backend_externalauth_degraded is absent instead of misleading 0% 3. Remove Error Budget Remaining, Error Budget Burn Rate, and Degraded Health Budget panels — these are point-in-time ratios with no window and count AwaitingSecret as bad; hold until windowed good/total rules from ARO-25935 exist 4. Add or vector(0) to Timeliness SLO numerator — show 0% when every create runs late, instead of No data 5. Apply exclude_subs filter to Create >15m, Update >10m, Delete >15m stuck stats, Active Operations table, and In-Flight Operation Duration — matches the stuck alerts which exclude internal subs

Simon Wiencki (swiencki) · 2026-10-28T20:28:34Z

+	// transient Degraded=True that does not represent a real failure. Only
+	// emit the gauge once the provisioning state is terminal (Succeeded,
+	// Failed, or Canceled) so the SLI is not polluted by normal rollout lag.
+	if !externalAuth.Properties.ProvisioningState.IsTerminal() {


IsTerminal also skips Updating, so an already degraded ExternalAuth drops out of the gauge for the whole update even though updates never hit the nil Configuration case; could the skip be limited to Accepted, AwaitingSecret and Provisioning?

AwaitingSecret is one of the degraded states. In the future, it will be labeled as a configuration issue, but explicitly skipping AwaitingSecret would not be a consistent behavior. Added only Accepted and provisioning

Copilot

🟡 Changes recommended

Production deletion bypasses ExternalAuth metric cleanup, leaving stale series exported, while provisioning-state filtering hides explicitly reported degraded failures.

4 open issues

1 resolved since last review

Nonterminal fixtures prevent degraded gauge series from being emitted

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The production controller bypasses metric cleanup when an ExternalAuth is deleted, leaving stale component degradation metrics exported until restart, while the deletion test calls the inner syncer directly and misses this path.

4 open issues

_{Review effort: Balanced}

openshift-ci · 2026-10-29T05:47:25Z

Mansi Mandhare (@m-mandhare): The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/alert-gate	`08d348f`	link	false	`/test alert-gate`

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.

openshift-merge-bot · 2026-10-29T06:08:15Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-parallel

Copilot AI balanced review requested due to automatic review settings October 16, 2026 12:57

openshift-ci Bot added the do-not-merge/work-in-progress label Oct 16, 2026

openshift-ci Bot added the area/backend Automatically applied by owners-label for area/backend label Oct 16, 2026

Mansi Mandhare (m-mandhare) marked this pull request as ready for review October 16, 2026 12:57

openshift-ci Bot removed the do-not-merge/work-in-progress label Oct 16, 2026

openshift-ci Bot requested review from Gerd Oberlechner (geoberle) and Matthew Barnes (mbarnes) October 16, 2026 12:57

Copilot started reviewing on behalf of Mansi Mandhare (m-mandhare) October 16, 2026 12:57

View session

Mansi Mandhare (m-mandhare) changed the title ~~Feat : Expose Degraded Condition as Prometheus Metric for external auth- #5~~ Feat : Expose Degraded Condition as Prometheus Metric for external auth Oct 16, 2026

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Simon Wiencki (swiencki) mentioned this pull request Oct 20, 2026

feat: add ExternalAuth Lifecycle Overview Grafana dashboard #7267

Open

Simon Wiencki (swiencki) reviewed Oct 20, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 21, 2026 08:08

Copilot started reviewing on behalf of Mansi Mandhare (m-mandhare) October 21, 2026 08:09

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Simon Wiencki (swiencki) reviewed Oct 21, 2026

View reviewed changes

Mansi Mandhare (m-mandhare) force-pushed the ARO-29559 branch from 51cc443 to a98dfac Compare October 26, 2026 07:12

Copilot AI balanced review requested due to automatic review settings October 26, 2026 07:12

Copilot started reviewing on behalf of Mansi Mandhare (m-mandhare) October 26, 2026 07:13

View session

Copilot AI reviewed Oct 26, 2026

View reviewed changes

Simon Wiencki (swiencki) reviewed Oct 26, 2026

View reviewed changes

Comment thread backend/pkg/controllers/externalauth/status/externalauth_oidc_clients_degraded_controller.go Outdated

feat(backend): add ExternalAuthAvailableController to surface OIDC cl…

f9e65e4

…ient status conditions

Mansi Mandhare (m-mandhare) force-pushed the ARO-29559 branch from a98dfac to 8bc3ed4 Compare October 28, 2026 11:41

Copilot AI balanced review requested due to automatic review settings October 28, 2026 11:41

Copilot started reviewing on behalf of Mansi Mandhare (m-mandhare) October 28, 2026 11:42

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Comment thread backend/pkg/controllers/metrics/resource_metrics_controller_test.go

Simon Wiencki (swiencki) reviewed Oct 28, 2026

View reviewed changes

Mansi Mandhare (m-mandhare) force-pushed the ARO-29559 branch from 8bc3ed4 to 85ee984 Compare October 29, 2026 05:14

Copilot AI balanced review requested due to automatic review settings October 29, 2026 05:14

Copilot started reviewing on behalf of Mansi Mandhare (m-mandhare) October 29, 2026 05:15

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Comment thread backend/pkg/controllers/metrics/resource_metrics_controller.go Outdated

Expose Degraded Condition as Prometheus Metric

08d348f

Mansi Mandhare (m-mandhare) force-pushed the ARO-29559 branch from 85ee984 to 08d348f Compare October 29, 2026 05:31

Copilot AI balanced review requested due to automatic review settings October 29, 2026 05:31

Copilot started reviewing on behalf of Mansi Mandhare (m-mandhare) October 29, 2026 05:32

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Conversation

Mansi Mandhare (m-mandhare) commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Testing

Uh oh!

openshift-merge-bot Bot commented Oct 16, 2026

Uh oh!

openshift-ci Bot commented Oct 16, 2026

Uh oh!

openshift-ci Bot commented Oct 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

openshift-merge-bot Bot commented Oct 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

openshift-merge-bot Bot commented Oct 26, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Mansi Mandhare (m-mandhare) commented Oct 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Mansi Mandhare (m-mandhare) commented Oct 16, 2026 •

edited

Loading