chore(metrics): even better rate limit strategy #3500
Conversation
mdelapenya
added
chore
mdelapenya
added
chore
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe PR modifies the usage metrics collection timing and retry logic. It introduces a 7-second pre-query delay when metrics are already collected, removes the 2-second per-request delay, and reworks the retry backoff strategy to use uniform 60-second intervals instead of the previous exponential ramp. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Collector as collect-metrics
participant GitHub as GitHub API
rect rgb(220, 240, 255)
note over Collector: Metrics already collected
Collector->>Collector: Wait 7 seconds
end
rect rgb(200, 255, 200)
note over Client,Collector: Query sequence
loop Until success or max retries
Collector->>GitHub: Query usage metrics
alt Success
GitHub-->>Collector: Return metrics
Collector->>Client: Process metrics
else Failure (retry needed)
Collector->>Collector: Wait 60 seconds
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
usage-metrics/collect-metrics.go (1)
108-112: Consider differentiating backoff strategies for rate-limit vs. other errors.The uniform 60-second backoff is well-suited for rate limit errors, as it allows the rolling window to reset. However, for non-rate-limit errors (detected at lines 141-142), waiting 60 seconds might be excessive. These could be transient network issues, temporary GitHub API hiccups, or other recoverable errors that might succeed sooner.
While the current approach is simple and conservative (aligning with the PR's goal of resilience), you might consider implementing a shorter backoff sequence for non-rate-limit errors to reduce total execution time when encountering transient failures.
Example approach:
func queryGitHubUsageWithRetry(version string) (int, error) { var lastErr error - // Backoff intervals: wait longer for rate limit to reset (rolling window) - backoffIntervals := []time.Duration{ + // Backoff intervals for rate limit errors: wait for rolling window to reset + rateLimitBackoffs := []time.Duration{ 60 * time.Second, // Wait for rolling window 60 * time.Second, 60 * time.Second, } + // Shorter backoff for other transient errors + otherBackoffs := []time.Duration{ + 10 * time.Second, + 20 * time.Second, + 30 * time.Second, + } - // maxRetries includes the initial attempt plus one retry per backoff interval - maxRetries := len(backoffIntervals) + 1 + maxRetries := len(rateLimitBackoffs) + 1 for attempt := 0; attempt < maxRetries; attempt++ { if attempt > 0 { - // Use predefined backoff intervals - waitTime := backoffIntervals[attempt-1] + // Determine which backoff to use based on last error + var waitTime time.Duration + if strings.Contains(lastErr.Error(), "rate limit") || + strings.Contains(lastErr.Error(), "403") || + strings.Contains(lastErr.Error(), "429") { + waitTime = rateLimitBackoffs[attempt-1] + } else { + waitTime = otherBackoffs[attempt-1] + } log.Printf("Retrying version %s in %v (attempt %d/%d)", version, waitTime, attempt+1, maxRetries) time.Sleep(waitTime) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usage-metrics/collect-metrics.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
usage-metrics/collect-metrics.go (1)
log/logger.go (1)
Printf(47-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint (usage-metrics) / lint: usage-metrics
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
usage-metrics/collect-metrics.go (1)
67-71: Rate limit assumption verified as correct.Authenticated requests to the GitHub Code Search API are limited to 10 requests per minute, confirming the math in the comment (10 requests per 60 seconds = 6 seconds minimum). The implementation using 7 seconds is appropriately conservative and correct.
1 participant
What does this PR do?
Moves the delay before any call, instead of after the call. It also sets a bigger backoff intervals.
Why is it important?
Have a more resilient build
Related issues