Harden redis metrics collection against transient errors - #5131
Harden redis metrics collection against transient errors#5131nikola-jokic wants to merge 1 commit into
Conversation
|
Greptile SummaryThe PR hardens Redis metrics collection against transient failures while retaining the last successful business-metrics snapshot.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| internal/eventingester/metrics/redis/collector.go | Adds bounded scan retries and republishes updated failure telemetry while retaining the last successful business metrics. |
| internal/eventingester/configuration/validation.go | Validates effective retry-backoff bounds, including interactions between configured maxima and default initial values. |
| internal/common/config/validation.go | Safely formats nil, wrapped validator, and ordinary errors without an unchecked type assertion. |
| internal/eventingester/repository/scanner.go | Treats vanished Redis streams as expected scan races instead of aborting collection. |
| internal/eventingester/metrics/redis/collector_test.go | Covers stale-snapshot retention, failure telemetry, retry classification, retry exhaustion, and effective backoff capping. |
Sequence Diagram
sequenceDiagram
participant C as Metrics collector
participant R as Redis scanner
participant S as Atomic snapshot
C->>R: ScanAll with per-attempt timeout
alt Scan succeeds
R-->>C: Stream information
C->>C: Reset and rebuild business metrics
C->>S: Publish successful snapshot
else Retryable failure
R-->>C: Timeout or connection error
C->>C: Wait with bounded exponential backoff
C->>R: Retry ScanAll
else Final or non-retryable failure
R-->>C: Error
C->>C: Increment failure telemetry
C->>S: Republish retained business metrics plus current error telemetry
end
Reviews (11): Last reviewed commit: "Fix redis collection" | Re-trigger Greptile
| // Collect snapshot with error metrics | ||
| c.collectSnapshot() | ||
| c.collectionDuration.WithLabelValues(collectionStatusError).Observe(time.Since(start).Seconds()) | ||
| c.collectSnapshot() // Update snapshot with self-monitoring metrics even on error |
There was a problem hiding this comment.
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
570cedc to
fbfcfdf
Compare
db16182 to
d7be53e
Compare
What type of PR is this?
Fix
What this PR does / why we need it