Skip to content

feat(capture): wire v1 sink router in production setup#59977

Merged
eli-r-ph merged 4 commits into
eli.r/capture-v1-response-3from
eli.r/capture-v1-response-4
May 26, 2026
Merged

feat(capture): wire v1 sink router in production setup#59977
eli-r-ph merged 4 commits into
eli.r/capture-v1-response-3from
eli.r/capture-v1-response-4

Conversation

@eli-r-ph
Copy link
Copy Markdown
Contributor

@eli-r-ph eli-r-ph commented May 25, 2026

Problem

This PR wires up the real production router when configured.

Changes

  • Add v1_sink lifecycle handle to LifecycleHandles, registered conditionally when CAPTURE_V1_SINKS is non-empty
  • Extract create_v1_sink_router() helper in setup.rs that builds KafkaProducer + KafkaSink + Router with rich anyhow::Context error propagation on each step
  • Add v1_sink_router: Option<Arc<v1::sinks::Router>> to CaptureComponents
  • Pass v1_sink_router as a parameter to router::router() instead of hardcoding None
  • Flush the V1 sink router during graceful shutdown in server.rs (same pattern as legacy sink)
  • Unit test for error propagation in create_v1_sink_router

The CAPTURE_V1_SINKS env var defaults to "" — no deployment behavior changes until a deploy explicitly sets the var. No consumer config is added.

How did you test this code?

Locally and in CI.

Publish to changelog?

N/A

🤖 Agent context

Eli planned, Claude coded, Eli collected paycheck ✅

Conditionally build and register the V1 sink router when
CAPTURE_V1_SINKS is non-empty. Pass it through the HTTP router state
and flush it during graceful shutdown alongside the legacy sink.

The env var defaults to "" so no deployment behavior changes until a
deploy explicitly configures v1 sinks.
@eli-r-ph eli-r-ph self-assigned this May 25, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 25, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
rust/capture/src/setup.rs:69-73
The final registration of `v1_sink` is the last use of `sink_opts`, so the `.clone()` is superfluous — `sink_opts` can be moved directly into `manager.register`. The clone allocates and drops immediately, which is unnecessary.

```suggestion
    let v1_sink = if !config.capture_v1_sinks.is_empty() {
        Some(manager.register("v1-sink", sink_opts))
    } else {
        None
    };
```

Reviews (1): Last reviewed commit: "feat(capture): wire v1 sink router in pr..." | Re-trigger Greptile

Comment thread rust/capture/src/setup.rs Outdated
Comment thread rust/capture/src/server.rs Outdated
Comment thread rust/capture/src/setup.rs
@eli-r-ph eli-r-ph merged commit 4bdba80 into eli.r/capture-v1-response-3 May 26, 2026
144 checks passed
@eli-r-ph eli-r-ph deleted the eli.r/capture-v1-response-4 branch May 26, 2026 19:41
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