-
Couldn't load subscription status.
- Fork 147
feat: kq commitinorder support trace #75
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
base: master
Are you sure you want to change the base?
Conversation
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: using context.Background() here loses trace context - should use ctx instead
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.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
Greptile Summary
Added OpenTelemetry tracing support to Kafka queue implementation, enabling distributed tracing across message processing with standardized context propagation.
kq/internal/trace.goimplementingMessageCarrierinterface for OpenTelemetry context handlingPushWithKeymethod using OpenTelemetry propagatorextractCtxFromMsgfunction for consistent tracingconsumeOnemethodgo.modto support OpenTelemetry integration💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!