-
Notifications
You must be signed in to change notification settings - Fork 330
rotor: migrate to confluent kafka library WIP #1222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
# Conflicts: # pnpm-lock.yaml # services/rotor/src/lib/kafka-config.ts # services/rotor/src/lib/metrics.ts # services/rotor/src/lib/rotor.ts
There was a problem hiding this 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 migrates the rotor service from kafkajs to Confluent's Kafka JavaScript client to improve stability and add native compression support. Key changes include updating the producer/consumer/admin APIs, enabling LZ4 compression, and simplifying configuration while maintaining backward compatibility.
- Replace kafkajs dependency with @confluentinc/kafka-javascript
- Update API configuration to use nested kafkaJS config structure
- Enable native LZ4 compression support in addition to existing compression types
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| webapps/console/lib/server/sync.ts | Minor typo fix changing "to low" to "too low" |
| services/rotor/webpack.config.js | Add external mapping for new Confluent Kafka library |
| services/rotor/src/lib/rotor.ts | Core migration to new Kafka client with updated consumer/producer configuration |
| services/rotor/src/lib/metrics.ts | Update type definitions and remove per-send compression |
| services/rotor/src/lib/kafka-config.ts | Complete rewrite of connection logic for new Kafka client |
| services/rotor/package.json | Replace kafkajs dependencies with Confluent client |
| services/rotor/dist_package.json | Update distribution dependencies |
| services/rotor/tests/kafka.test.ts | Update test configuration for new API |
| services/profiles/package.json | Bump Confluent client version to 1.4.1 |
| services/profiles/dist_package.json | Update distribution dependency version |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 11 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="services/rotor/src/lib/rotor.ts">
<violation number="1" location="services/rotor/src/lib/rotor.ts:96">
admin.connect() is not awaited, risking race conditions and unhandled rejections.</violation>
<violation number="2" location="services/rotor/src/lib/rotor.ts:123">
DoS via unbounded parallelism from attacker-controlled CONNECTION_IDS_HEADER (CWE-400)</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
* rotor: migrate to confluent kafka library
Summary by cubic
Migrate rotor from kafkajs to Confluent’s Kafka JavaScript client to improve stability and add native compression support. This updates consumer/producer/admin APIs, simplifies config, and enables LZ4 in addition to gzip/snappy/zstd.
Dependencies
Refactors