Skip to content

refactor: remove analytics channel, use outlet batch dispatch directly#867

Open
fergusfinn wants to merge 1 commit into
mainfrom
refactor/analytics-direct-batch
Open

refactor: remove analytics channel, use outlet batch dispatch directly#867
fergusfinn wants to merge 1 commit into
mainfrom
refactor/analytics-direct-batch

Conversation

@fergusfinn
Copy link
Copy Markdown
Contributor

Summary

  • Removes the intermediate channel + accumulation loop between outlet and analytics writes
  • Renames AnalyticsBatcherAnalyticsWriter (no longer batches, just enriches + writes)
  • Overrides handle_response_batch on AnalyticsHandler for direct batch processing
  • Removes the analytics-batcher background task from BackgroundServices
  • Increases outlet channel_capacity from 4096 → 16384 since outlet is now the sole buffer for billing data

Before

outlet → handle_response_batch (default, calls per-item)
  → AnalyticsHandler.handle_response × N
    → channel.send(RawAnalyticsRecord) × N
      → AnalyticsBatcher background task
        → re-accumulate from channel
        → enrich batch
        → transactional write

After

outlet → handle_response_batch (override)
  → AnalyticsHandler.handle_response_batch
    → extract RawAnalyticsRecords from slice
    → AnalyticsWriter.flush
      → enrich batch
      → transactional write

Net result: -161 lines, one fewer background task, one fewer channel, same enrichment/write/retry logic.

Test plan

  • just lint rust passes
  • just test rust passes (1053 tests, 0 failures)
  • CI passes

The analytics pipeline previously had two batching layers:
1. outlet accumulated responses and dispatched batches
2. AnalyticsHandler scattered items into a 10k channel
3. AnalyticsBatcher re-accumulated from the channel and flushed

Now that outlet handles batching natively (0.8.0), this simplifies to:
1. outlet accumulates and dispatches batches
2. AnalyticsHandler.handle_response_batch extracts records
3. AnalyticsWriter.flush does enrichment + transactional write directly

Changes:
- Rename AnalyticsBatcher → AnalyticsWriter, remove channel/accumulation loop
- Override handle_response_batch on AnalyticsHandler for direct batch processing
- Remove analytics background task from BackgroundServices
- Increase outlet channel_capacity to 16384 (sole buffer for billing data)
Copilot AI review requested due to automatic review settings March 12, 2026 14:21
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying control-layer with  Cloudflare Pages  Cloudflare Pages

Latest commit: 199b9df
Status: ✅  Deploy successful!
Preview URL: https://14fcb0df.control-layer.pages.dev
Branch Preview URL: https://refactor-analytics-direct-ba.control-layer.pages.dev

View logs

Copy link
Copy Markdown
Contributor

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 refactors the Rust backend analytics/billing pipeline to remove the intermediate analytics channel + background batcher task, and instead process outlet’s response batches directly via an AnalyticsWriter that enriches and writes records transactionally.

Changes:

  • Remove the analytics channel and background “analytics-batcher” task; rely on outlet batch dispatch.
  • Rename AnalyticsBatcherAnalyticsWriter and expose a flush(&[RawAnalyticsRecord]) API for batched writes.
  • Update router/background service wiring to enable analytics without passing around an analytics sender; increase outlet buffer capacity.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
dwctl/src/test/mod.rs Updates build_router call sites to match the new signature.
dwctl/src/request_logging/mod.rs Stops re-exporting the removed AnalyticsBatcher.
dwctl/src/request_logging/batcher.rs Refactors the former batcher into AnalyticsWriter and updates integration tests to call flush.
dwctl/src/request_logging/analytics_handler.rs Implements direct handle_response_batch processing and delegates to AnalyticsWriter.
dwctl/src/lib.rs Wires analytics handler directly in build_router, removes batcher background task setup, and increases outlet channel_capacity.
Comments suppressed due to low confidence (2)

dwctl/src/request_logging/batcher.rs:55

  • The doc comment for RawAnalyticsRecord still says the record is "sent through the channel" and that enrichment happens in the "batcher", but this PR removes the channel and renames the component to AnalyticsWriter. Updating this comment will prevent confusion when reading the new architecture.
/// Raw analytics record sent through the channel (unenriched).
///
/// This contains only data that can be extracted from the request/response
/// without any database lookups. Enrichment happens in the batcher.
#[derive(Debug, Clone)]

dwctl/src/request_logging/mod.rs:10

  • request_logging no longer re-exports the renamed writer type. Previously AnalyticsBatcher was exported from this module; after the rename, consider re-exporting AnalyticsWriter (or documenting the intended access path) to avoid an accidental public API breaking change.
pub use analytics_handler::AnalyticsHandler;
pub use models::{AiRequest, AiResponse, ParsedAIRequest};


You can also share your feedback on Copilot code review. Take the survey.

Comment thread dwctl/src/request_logging/analytics_handler.rs
Comment thread dwctl/src/request_logging/batcher.rs
Comment thread dwctl/src/request_logging/batcher.rs
Comment thread dwctl/src/request_logging/batcher.rs
Comment thread dwctl/src/request_logging/batcher.rs
Comment thread dwctl/src/request_logging/batcher.rs
Comment thread dwctl/src/request_logging/batcher.rs
Comment thread dwctl/src/request_logging/batcher.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants