fix(test): attempt at fixing metrics profile e2e test flake

[Clee2691]

Description

This PR attempts to fix the metrics profile e2e flake. It replaces vector_started_total with vector_uptime_seconds because the former metric expires.

This also updates the QueryPrometheus function to surface errors instead of silently swallowing them.

No associated JIRA

/cc @vparfonov
/assign @jcantrill

Summary by CodeRabbit

• Bug Fixes
  • Improved Prometheus query handling by surfacing query/request/response failures instead of masking them.
  • Made metrics polling more resilient by treating Prometheus errors as transient during retries, while reporting the last error if the wait ultimately times out.
• Tests
  • Updated end-to-end metrics assertions to verify the correct metric name/value for profile and flow-control scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a95883f3-01d0-4bc8-8db7-b2875c5adfc6

📥 Commits

Reviewing files that changed from the base of the PR and between c19ef11 and 758eb98.

📒 Files selected for processing (3)

• test/e2e/collection/metrics/profile_test.go
• test/e2e/flowcontrol/utils.go
• test/helpers/prometheus/prometheus.go

🚧 Files skipped from review as they are similar to previous changes (2)

• test/e2e/collection/metrics/profile_test.go
• test/helpers/prometheus/prometheus.go

---

📝 Walkthrough

Walkthrough

Prometheus test queries now return explicit errors instead of empty maps, and the e2e metric checks were updated to use vector_uptime_seconds. The flowcontrol polling helper now keeps retrying through query failures and reports the last error on timeout.

Changes

Prometheus Query Error Handling

Layer / File(s)	Summary
QueryPrometheus error propagation test/helpers/prometheus/prometheus.go	QueryPrometheus returns (map[string]interface{}, error) and returns wrapped errors for request creation, request execution, non-200 responses, response body reads, and JSON unmarshalling. Query returns that result directly.
Metric name and polling updates test/e2e/flowcontrol/utils.go, test/e2e/collection/metrics/profile_test.go	VectorUpTotal and the full-profile scraping test now use vector_uptime_seconds, and WaitForMetricsToShow records the last Prometheus query error while continuing to poll until timeout.

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

Related PRs: None identified.

Suggested labels: test, refactor

Suggested reviewers: None identified.

Poem

A query once failed and stayed unseen,
Now errors return, clear and clean.
A metric renamed, the tests align,
And polling waits a little more time.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing a flaky metrics profile e2e test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)

---

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

[qodo-for-rh-openshift]

PR Summary by Qodo

Fix metrics profile e2e flake by using stable metric and surfacing Prometheus errors

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Replace expiring vector_started_total check with stable vector_uptime_seconds in e2e metrics
 profile test.
• Change Prometheus query helper to return errors on request/response/parsing failures.
• Propagate query errors to callers instead of silently returning empty results.

Diagram

graph TD
A["metrics/profile_test.go"] --> B["prometheus.Query()"] --> C["prometheus.QueryPrometheus()"] --> D[("Thanos / Prometheus API")]
C -- "returns error" --> B

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Query a canonical always-present metric (e.g., `up` or `vector_build_info`)

• ➕ Less dependent on Vector-specific counter lifetimes or resets
• ➕ Often simpler semantics for presence/availability checks
• ➖ May not validate the specific non-allowlisted metric behavior the test intends to cover
• ➖ May require adjusting the test’s assertions/intent

2. Keep `vector_started_total` but make the test resilient to expiration

• ➕ Minimal semantic change to what’s being asserted today
• ➖ Continues to rely on a metric known to expire, likely preserving flake risk
• ➖ Adds complexity (time-windowing/extra retries) without improving observability of failures

Recommendation: The PR’s approach is sound: switching to vector_uptime_seconds directly targets the flake root cause (metric expiration) while improving debuggability by surfacing Thanos/Prometheus errors. If this test’s intent is only “a non-allowlisted metric exists,” consider up/vector_build_info later, but vector_uptime_seconds is a good stable Vector-specific choice.

Files changed (2) +9 / -16

Bug fix (1) +8 / -15

prometheus.goReturn and propagate Prometheus query errors +8/-15

Return and propagate Prometheus query errors

• Updates 'QueryPrometheus' to return '(map, error)' and to fail fast on request creation, network errors, non-200 responses, body read failures, and JSON unmarshal failures. Updates 'Query' to propagate 'QueryPrometheus' errors instead of always returning nil error.

test/helpers/prometheus/prometheus.go

Tests (1) +1 / -1

profile_test.goUse non-expiring metric for non-allowlisted assertion +1/-1

Use non-expiring metric for non-allowlisted assertion

• Replaces the non-allowlisted metric from 'vector_started_total' to 'vector_uptime_seconds' to avoid flakes caused by metric expiration.

test/e2e/collection/metrics/profile_test.go

[qodo-for-rh-openshift]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 9 rules

1. Polling aborts on query error ✓ Resolved 🐞 Bug ☼ Reliability

Description

prometheus.Query now propagates QueryPrometheus errors, and WaitForMetricsToShow returns those
errors from the PollUntilContextTimeout condition function, which can prematurely stop polling on
transient Thanos failures instead of retrying until metrics appear.

Code

test/helpers/prometheus/prometheus.go[R87-90]

	if err != nil {
		return nil, err
	}
-	return QueryPrometheus(host, token, query), nil
+	return QueryPrometheus(host, token, query)

Relevance

⭐⭐ Medium

Repo sometimes prefers surfacing errors/early-exit in waits (PR #3301, #3304); no clear precedent
for Thanos transient retries.

PR-#3301
PR-#3304

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR changes Query() to return QueryPrometheus(host, token, query) directly (including its
errors). Flowcontrol’s WaitForMetricsToShow returns any Query error from the PollUntilContextTimeout
callback, while other polling helpers in the repo typically return (false, nil) on transient
errors to keep polling.

test/helpers/prometheus/prometheus.go[81-91]
test/e2e/flowcontrol/utils.go[22-33]
test/framework/e2e/wait.go[37-52]
test/framework/e2e/wait.go[58-63]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`test/helpers/prometheus.Query()` now returns errors from `QueryPrometheus()`. `WaitForMetricsToShow()` (flowcontrol e2e) currently returns those errors from the `wait.PollUntilContextTimeout` condition function, which can terminate polling early on transient query failures.

### Issue Context
Other polling helpers in this repo intentionally swallow transient errors by returning `(false, nil)` so polling continues until success/timeout.

### Fix Focus Areas
- test/e2e/flowcontrol/utils.go[22-33]
- test/helpers/prometheus/prometheus.go[81-91]

### Suggested fix
In `WaitForMetricsToShow()`, treat `prometheus.Query(...)` errors as retryable during the poll window:
- On query error: log at a low verbosity and return `(false, nil)` so the poll keeps retrying.
- Optionally track the last error and include it in the final failure log if the poll times out.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[jcantrill]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Clee2691, jcantrill

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 [jcantrill]

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

[Clee2691]

/retest

[vparfonov]

/lgtm

[openshift-ci]

@Clee2691: 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/e2e-using-bundle	758eb98	link	false	/test e2e-using-bundle

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.