fix: drop apiserver_request_total watch/connect verb series from SRE metric set

[Jan-Hendrik Boll (janboll)]

Summary

• Drop apiserver_request_total timeseries with verb=WATCH|CONNECT from the SRE metric set
• These verbs account for ~29% of raw apiserver_request_total timeseries (~76,700 out of 261,732 across 44 HCPs) but are never consumed by any recording rule or alert — all upstream rules explicitly filter to verb=~"LIST|GET|POST|PUT|PATCH|DELETE"
• Fix verb regex case: Kubernetes exports verb labels in uppercase, so the original (watch|connect) regex never matched — changed to (WATCH|CONNECT)
• Add an E2E test that parses the configmap template, extracts all drop rules, and queries Azure Monitor Workspace to verify each dropped series is absent (using up{job=...} as a positive control to confirm metrics are flowing for all six components)

Test plan

• Verify helm fixture files are consistent (cd config && make materialize)
• Lint passes (make lint)
• Test compiles (go vet -tags E2Etests ./e2e/...)
• Confirm no recording rule or alert references verb="watch" or verb="connect" for apiserver_request_total
• After rollout, validate timeseries count reduction in Grafana
• Run E2E test in dev: make -C test -f E2ELocal.mk e2e-local/run-test TEST_NAME="SRE Metrics Set"

References: https://redhat.atlassian.net/browse/AROSLSRE-1550

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the Hypershift Operator SRE metric set relabeling to reduce Prometheus timeseries cardinality by dropping apiserver_request_total series for long-running verbs (watch/connect), which are high-cost and (per PR description) not consumed by existing alerts/recording rules.

Changes:

• Add a metric relabel drop rule for apiserver_request_total with verb=watch|connect in the SRE metric set ConfigMap template.
• Update rendered Helm fixtures to include the new drop rule for both the default and performance-metrics fixture outputs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
hypershiftoperator/deploy/templates/sre-metrics-set.configmap.yaml	Adds a relabel drop rule intended to remove apiserver_request_total watch/connect series from the SRE metric set.
hypershiftoperator/zz_fixture_TestHelmTemplate_dev_westus3_mgmt_1_hypershift.yaml	Updates the rendered fixture output to reflect the new drop rule.
hypershiftoperator/testdata/zz_fixture_TestHelmTemplate_hypershift_performance_metrics.yaml	Updates the performance-metrics rendered fixture output to reflect the new drop rule.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

hypershiftoperator/deploy/templates/sre-metrics-set.configmap.yaml:34

• The new drop rule only exists under the kubeAPIServer component. openshiftAPIServer still keeps apiserver_request_total (see its keep regex) without a corresponding verb=WATCH|CONNECT drop, so WATCH/CONNECT series from the OpenShift API Server will still be ingested and this won’t fully achieve “drop apiserver_request_total WATCH/CONNECT” for the overall SRE metrics set.

Add the same drop rule block under openshiftAPIServer (between the optional performanceMetrics drops and the keep rule), then re-run materialize to update the generated helm fixtures.

      - action:       "drop"
        regex:        "apiserver_request_total;(WATCH|CONNECT)"
        sourceLabels: ["__name__", "verb"]

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

test/e2e/metrics_drop_rules.go:92

• parseDropRules renders the Helm template with performanceMetrics hard-coded to false, which will generate a different drop-rule set than environments where hypershift.metricsSet.performanceMetrics=true (e.g. dev/perf). This can make the E2E check assert drops that are not actually configured for that environment. Consider reading the value from an env var populated by the runner (similar to AMW_REGION_RG) and using it when rendering the template.

	data := map[string]any{
		"Values": map[string]any{
			"metricsSet": map[string]any{
				"performanceMetrics": false,
			},
		},
		"Release": map[string]any{
			"Namespace": "test",
		},
	}

test/E2ELocal.mk:43

• metrics_drop_rules.go now expects AMW_* env vars; for correctness across environments, it should also receive whether performance metrics are enabled so it can render the configmap template consistently with the actual deployment values (hypershift.metricsSet.performanceMetrics). Export that value here so the test can use it.

	export ARTIFACT_DIR="$(E2E_ARTIFACT_DIR)"; \
	export AMW_REGION_RG="$$(yq .regionRG < $(AMW_RENDERED_CONFIG))"; \
	export AMW_HCP_WORKSPACE_NAME="$$(yq .monitoring.hcpWorkspaceName < $(AMW_RENDERED_CONFIG))"; \

test/e2e/metrics_drop_rules.go:267

• The absence query is not scoped to the component/job the drop rule applies to. Since metrics like apiserver_request_total can be emitted by multiple jobs (e.g. kube-apiserver vs openshift-apiserver), querying without a job matcher can fail even when the specific component’s drop rule is working. Consider adding job=... to the generated PromQL when a job label mapping exists.

				By(fmt.Sprintf("asserting dropped series are absent: %s", dropRuleToPromQL(rule)))
				promQL := dropRuleToPromQL(rule)
				resp, err := amwQuery(ctx, httpClient, cred, endpoint, promQL)

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

test/E2ELocal.mk:43

• The new metrics-drop-rules E2E test needs the same hypershift.metricsSet.performanceMetrics value used by the deployment to render/validate the correct drop-rule set. e2e-local/run-test already renders config into $(AMW_RENDERED_CONFIG); export the performanceMetrics flag alongside the AMW env vars so the test can consume it.

	export ARTIFACT_DIR="$(E2E_ARTIFACT_DIR)"; \
	export AMW_REGION_RG="$$(yq .regionRG < $(AMW_RENDERED_CONFIG))"; \
	export AMW_HCP_WORKSPACE_NAME="$$(yq .monitoring.hcpWorkspaceName < $(AMW_RENDERED_CONFIG))"; \

test/e2e/metrics_drop_rules.go:92

• parseDropRules() hard-codes Values.metricsSet.performanceMetrics=false when rendering the Helm template. This makes the test validate the non-performance drop-rule set even if the deployment is configured with performanceMetrics=true (e.g., dev/perf), which can cause false failures or miss issues in the active config. Read the intended value from an env var (plumbed from rendered config in the test harness) and render the template with that value.

	data := map[string]any{
		"Values": map[string]any{
			"metricsSet": map[string]any{
				"performanceMetrics": false,
			},
		},
		"Release": map[string]any{
			"Namespace": "test",
		},
	}

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: janboll

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:

• OWNERS [janboll]

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

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

test/e2e/metrics_drop_rules.go:263

• The positive-control up{job=...} wait runs once per drop rule, so components with multiple drop rules will repeat the same 10-minute Eventually block and extra AMW queries. Caching the per-component positive control keeps the test fast and reduces AMW load without changing coverage.

				if hasJob {
					By(fmt.Sprintf("waiting for up{job=%q} from %s", jobLabel, rule.Component))
					Eventually(func(g Gomega) {
						controlQuery := fmt.Sprintf(`up{job="%s"}`, jobLabel)
						resp, err := amwQuery(ctx, httpClient, cred, endpoint, controlQuery)

[Jan-Hendrik Boll (janboll)]

/retest-required

[Jan-Hendrik Boll (janboll)]

/hold
need to think about this again, simply discarding by label can be confusing