feat: kq commitinorder support trace #75
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Added tracing support to the CommitInOrder functionality in the Kafka queue implementation, enhancing message processing traceability.
- Introduced
extractCtxFromMsgfunction to consistently extract context from Kafka messages - Applied
extractCtxFromMsgin both CommitInOrder and regular consumer cases - Improved traceability of message processing in the queue system
- Ensured consistent context extraction across different consumption modes
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
|
|
||
| if err := q.consumeOne(ctx, string(msg.Key), string(msg.Value)); err != nil { | ||
| if q.errorHandler != nil { | ||
| q.errorHandler(context.Background(), msg, err) |
There was a problem hiding this comment.
logic: using context.Background() here loses trace context - should use ctx instead
There was a problem hiding this comment.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| defer func() { | ||
| if err := recover(); err != nil { | ||
| logc.Errorf(ctx, "consumeOne failed recover, error: %v", err) | ||
| } | ||
| }() |
There was a problem hiding this comment.
style: recover handler should re-panic on system errors like out of memory that shouldn't be caught
There was a problem hiding this comment.
1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
go.mod
Outdated
| @@ -1,4 +1,4 @@ | |||
| module github.com/zeromicro/go-queue | |||
| module github.com/zhuud/go-queue | |||
There was a problem hiding this comment.
logic: Module path should remain 'github.com/zeromicro/go-queue' as this is a PR to the original repository. This change would break all existing imports.
There was a problem hiding this comment.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
- 新增 WithBatchTimeout、WithBatchSize、WithBatchBytes 三个 PushOption - 在 NewPusher 中应用这些配置到 kafka.Writer - 补充完整的单元测试覆盖 涉及文件： - kq/pusher.go: 添加配置选项和应用逻辑 - kq/pusher_test.go: 添加集成测试 - kq/queue_test.go: 添加选项函数单元测试
1 participant
Greptile Overview
Updated On: 2025-11-07 07:37:05 UTC
Greptile Summary
This review covers only the changes made since the last review, not the entire PR. The changes add new batch configuration options to the Kafka pusher functionality, providing users with more granular control over message batching behavior. Three new options are introduced:
batchTimeout,batchSize, andbatchBytes, which directly correspond to the underlying kafka-go library's batch configuration parameters. The implementation follows the existing pattern of configuration options in the codebase, with proper initialization logic and comprehensive test coverage. These additions enhance the pusher's performance tuning capabilities without affecting existing functionality.Important Files Changed
Confidence score: 5/5