Conversation
in favor of sugar
| let sf = Arc::clone(&self.stats_flusher); | ||
| self.handles | ||
| .stats_flush_handles | ||
| .push(tokio::spawn(async move { sf.flush(false).await })); |
also remove sending interval to flush_blocking
There was a problem hiding this comment.
Pull request overview
This PR refactors flushing logic into a dedicated service module to centralize and standardize flush operations across different data types (logs, traces, metrics, stats, and proxy requests).
Changes:
- Introduces a new
FlushingServicethat coordinates non-blocking and blocking flush operations across multiple flusher types - Adds
FlushHandlesfor tracking in-flight flush operations with retry logic - Exposes the new flushing module in the public API
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| bottlecap/src/lib.rs | Exposes the new flushing module in the library's public API |
| bottlecap/src/flushing/mod.rs | Defines the module structure and exports the main types |
| bottlecap/src/flushing/handles.rs | Implements handle tracking for in-flight flush operations with retry support |
| bottlecap/src/flushing/service.rs | Implements the core FlushingService with spawn, await, and blocking flush capabilities |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Err(e) = flush_task_handle.await { | ||
| error!("Error waiting for background flush task: {e:?}"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Is there a particular reason we no longer flush on shutdown? If so, would you mind adding a brief comment in the commit to capture the rationale?
There was a problem hiding this comment.
It got moved, once we await the flush_task_handle, since it's the last task we wait for, inside of it, it just does its final flush on token cancellation
easier and cleaner than having it after we already awaited for our flushing task
litianningdatadog
left a comment
litianningdatadog
left a comment
There was a problem hiding this comment.
Left some comments
2 participants
Overview
Refactors flushing to become its own service
Motivation
Making it easier for us to handle the flushing code in one place and ensure consistency
SVLS-8505
Notes
Not using dynamic typings for the flushers we accepts so far, in another PR I will standardize it and ensure we can have capacity-based flushing too.