-
Notifications
You must be signed in to change notification settings - Fork 21
chore: use log over tracing #96
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
| dashmap = "6.1.0" | ||
| rust_decimal = "1" | ||
| ordered-float = { version = "4", features = ["serde"] } | ||
| ordered-float = { version = "5", features = ["serde"] } |
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.
Hash format changed. But I think we're yet to rely on it.
feae500 to
35b4b40
Compare
Signed-off-by: tison <wander4096@gmail.com>
35b4b40 to
eedb8ca
Compare
| Err(err) => { | ||
| log::warn!( | ||
| "Cannot read message header, ignoring message: {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.
err in kv is not good for text searching. I experienced this when sending logs to otel backend ..
| pub trait FileRead: Send + Unpin + 'static { | ||
| async fn read(&self, range: Range<u64>) -> Result<Bytes>; | ||
| fn read(&self, range: Range<u64>) -> impl Future<Output = Result<Bytes>> + Send; | ||
| } |
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.
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 logging infrastructure from the tracing crate to the log crate, motivated by performance considerations and compatibility with the fastrace distributed tracing approach. The change also includes dependency updates (prost 0.13→0.14, thiserror 1.0→2, ordered-float 4→5) and removes the async-trait dependency in favor of native Rust async trait syntax.
Key changes:
- Replaces all
tracing::warn!calls withlog::warn!and adds thelogcrate withkv_stdfeature - Removes
async-traitmacro and converts theFileReadtrait to use nativeimpl Futurereturn types - Updates several dependencies to their latest major versions
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/fluss/Cargo.toml | Adds log crate, removes tracing and async-trait, updates prost, thiserror, and ordered-float to latest versions |
| crates/fluss/src/util/mod.rs | Converts tracing::warn! to log::warn! in file deletion error handling |
| crates/fluss/src/rpc/server_connection.rs | Converts tracing::warn! calls to log::warn!, but contains invalid log macro syntax |
| crates/fluss/src/client/write/broadcast.rs | Converts tracing::warn! to log::warn! in BroadcastOnce drop handler |
| crates/fluss/src/io/file_io.rs | Removes #[async_trait::async_trait] and converts trait to use native async syntax, but missing Future import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
luoyuxia
left a comment
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.
@tisonkun Thanks for the pr. LGTM. Looking forward to more valuable inputs from you -Rust expert!
Ref -
And anyway tracing can be set to understand
log's output.