Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces OpenTelemetry with an in-process pipeline: adds anonymizer, user-agent parser, ClickHouse writer, consumer fetch/batching controls, ClickHouse connection/batching settings, migrates logging from slog to zerolog, removes readiness probe and OTLP endpoint, and wires consumer to the new pipeline. Changes
Sequence Diagram(s)sequenceDiagram
participant K as Kafka Consumer
participant C as Consumer (batch)
participant P as Pipeline
participant A as Anonymizer
participant U as UserAgentParser
participant CH as ClickHouseWriter
K->>C: Poll messages
C->>C: accumulate pending (size/time)
alt batch ready
C->>P: Process(ctx, events)
loop per event
P->>A: Anonymize(rawIP)
A-->>P: ipHash, country, region
P->>U: Parse(userAgent)
U-->>P: browser, os, deviceType
P->>P: build ProcessedEvent (UUID, clamp fields)
end
P->>CH: WriteBatch(ctx, processedEvents)
CH-->>P: success / error
alt success
P-->>C: signal commit offsets
else error
P-->>C: do not commit (retry/err handling)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 800575ea0c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69d5ff58c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52a3deb793
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR introduces a pipeline-based event processing flow that anonymizes IPs, parses user agents, batches events, and writes them to ClickHouse, alongside updates to runtime configuration, logging, and health endpoints.
Changes:
- Added ClickHouse writer with batch insert support and corresponding tests.
- Introduced processors (IP anonymizer, user-agent parser) and a pipeline that transforms
InternalEvent→ProcessedEvent. - Refactored the Kafka/Redpanda consumer to support configurable fetch/batching options and integrate with the pipeline; migrated logging to zerolog and updated healthcheck endpoints.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mise.toml | Adds a lint task for golangci-lint. |
| internal/storage/clickhouse.go | Implements ClickHouse connection + batch insert writer. |
| internal/storage/clickhouse_test.go | Unit tests for ClickHouse writer option parsing/validation. |
| internal/server/server.go | Switches health endpoint wiring to /livez. |
| internal/server/health.go | Removes readiness probe and readiness state. |
| internal/processors/useragent.go | Adds UA parsing (browser/OS/device/bot detection). |
| internal/processors/useragent_test.go | Tests UA parsing behavior. |
| internal/processors/anonymizer.go | Adds IP truncation + salted hashing for anonymization. |
| internal/processors/anonymizer_test.go | Tests IP truncation and hashing determinism/salt usage. |
| internal/pipeline/pipeline.go | Adds processing pipeline and ClickHouse batch write step. |
| internal/pipeline/pipeline_test.go | Tests UUID derivation and screen-width clamping via pipeline processing. |
| internal/observability/tracing.go | Removes OpenTelemetry tracing initialization. |
| internal/observability/logger.go | Replaces slog with zerolog structured logging. |
| internal/consumer/consumer.go | Adds consumer options, batching, and pipeline integration; adjusts polling/commit logic. |
| internal/consumer/consumer_test.go | Updates tests for new consumer API/options and batching helpers. |
| internal/config/config.go | Expands env-config parsing for consumer/ClickHouse/anonymization settings. |
| go.mod | Adds ClickHouse + zerolog deps; removes direct OTel tracer deps. |
| go.sum | Updates dependency checksums. |
| Dockerfile | Updates container healthcheck to /livez. |
| cmd/processor/main.go | Wires config → pipeline → consumer → ClickHouse writer; updates logging/shutdown flow. |
| .env.example | Documents new required env vars for consumer + ClickHouse + anonymization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := c.flushPending(ctx, batch.pending); err != nil { | ||
| c.logger.Error().Err(err).Int("batch_size", len(batch.pending)).Msg("pipeline processing failed") | ||
| return true | ||
| } |
There was a problem hiding this comment.
On flushPending error, this path immediately returns true without resetting the batch, and the outer loop will retry again immediately (no backoff), potentially creating a tight CPU/log loop during sustained downstream failures. Consider returning an error to break Run (so supervision can restart), or introduce a bounded backoff/sleep before retrying.
Summary by CodeRabbit
New Features
Improvements
Tests