feat: adds logging to retry logic in provider-proxy#1741
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce structured logging to the HTTP retry mechanism used throughout the provider proxy service. A logger instance is conditionally created and passed through the service container, injected into the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Container
participant ProviderService
participant httpRetry
participant Logger
App->>Container: Initialize (with NODE_ENV)
Container->>Logger: Create appLogger (if test)
Container->>ProviderService: Instantiate (pass appLogger)
ProviderService->>httpRetry: Call with logger option
httpRetry->>Logger: Log attempt (if logger present)
httpRetry->>ProviderService: Execute callback
alt Retry needed
httpRetry->>Logger: Log retry event
httpRetry->>httpRetry: Retry with backoff (pass logger)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1741 +/- ##
==========================================
- Coverage 72.46% 72.32% -0.14%
==========================================
Files 627 622 -5
Lines 14867 14696 -171
Branches 2583 2568 -15
==========================================
- Hits 10773 10629 -144
+ Misses 3772 3744 -28
- Partials 322 323 +1
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
d3440a7 to
cb5ba22
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive logging to the retry logic in the provider-proxy application to improve observability and debugging capabilities. The changes enable tracking of retry attempts, failures, and callback execution patterns.
- Adds optional logger parameter to retry utilities and services
- Implements structured logging for retry attempts and failures
- Updates dependency injection to provide logger instances throughout the application
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/provider-proxy/src/utils/retry.ts | Adds logging support to retry functions with structured log events |
| apps/provider-proxy/src/services/ProviderService/ProviderService.ts | Updates constructor to accept logger and passes it to retry calls |
| apps/provider-proxy/src/routes/proxyProviderRequest.ts | Adds logger to retry options in proxy request handling |
| apps/provider-proxy/src/container.ts | Reorganizes logger creation to provide app logger to ProviderService |
Why
Probably this is a blind spot which is reported by OTEL but we don't have logs for. Eventually I will suppress metrics collection for this section but first need to prove that this is the blind spot we were looking for
Summary by CodeRabbit
New Features
Bug Fixes