Skip to content

Conversation

lalitb
Copy link
Member

@lalitb lalitb commented Sep 2, 2025

towards: #309

Summary

This PR adds retry logic to the Geneva Config Service client to improve resilience against transient network issues and server overload. The changes implement intelligent error handling with appropriate backoff strategies.

Key changes:

  • Adds retry mechanism with up to 3 attempts for Geneva Config Service API calls
  • Implements smart backoff strategy using server-provided Retry-After headers or fixed delays
  • Introduces new error type for rate limiting scenarios with retry information
  • No changes in upload API, as retry for upload failure is the responsibility of caller.

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@lalitb lalitb changed the title [Geneva Exporter] Add retries for ingestion info retrieval fix: [Geneva Exporter] Add retries for ingestion info retrieval Sep 2, 2025
Copy link

codecov bot commented Sep 2, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.2%. Comparing base (95b097a) to head (49fbb6d).

Files with missing lines Patch % Lines
...eneva/geneva-uploader/src/config_service/client.rs 66.6% 24 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #442   +/-   ##
=====================================
  Coverage   53.2%   53.2%           
=====================================
  Files         70      70           
  Lines      10648   10718   +70     
=====================================
+ Hits        5665    5711   +46     
- Misses      4983    5007   +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@utpilla
Copy link
Contributor

utpilla commented Sep 5, 2025

fixes: #309

This PR doesn't fix that issue. It's only working on add retries for the ingestion info calls not the actual upload calls. We should either create a dedicate issue for this PR or use a different phrase such as "Towards" or "Partially Fixes" so that merging this PR doesn't automatically close the issue.

Comment on lines 381 to 385
Err(e) => {
if attempt < MAX_GCS_RETRIES && is_retriable_error(&e) {
attempt += 1;
continue;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to respect the Retry-After header if present, before sending the request again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I wasn't actually aware that such rate limiting option exists in HTTP, Added now.

@lalitb
Copy link
Member Author

lalitb commented Sep 13, 2025

This PR doesn't fix that issue. It's only working on add retries for the ingestion info calls not the actual upload calls. We should either create a dedicate issue for this PR or use a different phrase such as "Towards" or "Partially Fixes" so that merging this PR doesn't automatically close the issue.

Agree, done.

@lalitb lalitb requested a review from Copilot September 14, 2025 00:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds robust retry logic to the Geneva Config Service client to improve resilience against transient network issues and server overload. The changes implement intelligent error handling with appropriate backoff strategies.

Key changes:

  • Adds retry mechanism with up to 3 attempts for Geneva Config Service API calls
  • Implements smart backoff strategy using server-provided Retry-After headers or fixed delays
  • Introduces new error type for rate limiting scenarios with retry information

Reviewed Changes

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

File Description
client.rs Implements retry logic, error handling, and backoff strategy for config service calls
Cargo.toml Adds tokio dependency for sleep functionality in retry implementation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants