Harden redis metrics collection against transient errors

[nikola-jokic]

What type of PR is this?

Fix

What this PR does / why we need it

• Skip streams that vanish between SCAN and XINFO ("no such key") instead of aborting the whole collection cycle
• Retry scan attempts on transient errors (timeouts, connection failures) with exponential backoff, bounded by MaxRetries, and a per-attempt CollectionTimeout
• Keep the last good metrics snapshot on failed cycles; stale metrics are preferable to missing metrics

[datadog-armadaproject]

⚠️ Warnings

⚠️ Your PR has warnings. Please review the issues below.

🚦 2 Pipeline jobs failed

CI | All jobs succeeded

View in Datadog · View in GitHub Actions

CI | test / Golang Unit Tests

View in Datadog · View in GitHub Actions

1 failed test. Error: Should be empty, but was [0xa4f02a64200 0xa4f02a64300 0xa4f02a64400 0xa4f02a64500 0xa4f02a948a0 0xa4f02a46080 0xa4f02a460c0] in collector_test.go:337.

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: db16182 | Docs | View more details | Give us feedback!}

[greptile-apps]

Greptile Summary

The PR makes Redis stream metrics collection resilient to vanished streams and transient scan failures while retaining the last successful data snapshot.

• Adds bounded retries with per-attempt timeouts and exponential backoff.
• Validates retry backoff configuration and safely formats ordinary validation errors.
• Skips keys that disappear between Redis SCAN and XINFO.

Confidence Score: 4/5

The PR is not yet safe to merge because the cancellation test remains incompatible with the collector’s failure-snapshot behavior.

A cancelled first collection now publishes error telemetry through the atomic snapshot, while TestCollect_ContextCancellation still requires the scrape to be empty, leaving the test suite with a deterministic failure.

Files Needing Attention: internal/eventingester/metrics/redis/collector.go, internal/eventingester/metrics/redis/collector_test.go

Important Files Changed

Filename	Overview
internal/eventingester/metrics/redis/collector.go	Adds retry handling and stale-snapshot publication, but its first-failure behavior remains inconsistent with the cancellation test.
internal/eventingester/metrics/redis/collector_test.go	Adds coverage for retries and stale metrics, while the cancellation assertion still conflicts with the collector’s published failure snapshot.
internal/eventingester/configuration/validation.go	Validates negative and inconsistent retry backoff values against the collector’s effective defaults.
internal/common/config/validation.go	Safely formats validator errors while passing through ordinary errors without a panic.
internal/eventingester/repository/scanner.go	Treats Redis missing-key responses as vanished streams and continues collecting remaining stream metrics.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Start collection cycle] --> B[Scan Redis with attempt timeout]
  B -->|Transient error| C{Retries remaining?}
  C -->|Yes| D[Wait with bounded exponential backoff]
  D --> B
  C -->|No| E[Increment failure telemetry]
  B -->|Success| F[Build current metrics]
  E --> G[Publish stale data with current failure telemetry]
  F --> H[Publish new atomic snapshot]
  B -->|Vanished stream| I[Skip missing key]
  I --> B

Loading

_{Reviews (10): Last reviewed commit: "trim down validations" | Re-trigger Greptile}

[greptile-apps]

Cancellation test now fails

When the first collection attempt is cancelled, this call publishes a non-empty snapshot containing errorsTotal and the error-labelled collectionDuration, but TestCollect_ContextCancellation requires the collected metrics to remain empty, causing the test suite to fail.

Knowledge Base Used: Event store and ingestion