-
Notifications
You must be signed in to change notification settings - Fork 159
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
Investigation about tracing, logging, and metrics support. #482
Comments
I will suggestion the following set:
Why not
|
Personally I'd support |
Users who rely on Here is a decision matrix:
|
Hi, @sdd
Could you elaborate on this? Previously for logging capability I thought As with using I'm trying to summarize the proposed approaches here:
|
Hi all, I agree with @sdd and suggest to stick with tracing. It is the default choice for logging in async applications and has the most wide-spread adoption. Regarding performance arguments, I doubt that this matters in any real world benchmark of this crate, any tracing incurred overhead will be dwarfed by the network latencies of whatever storage cloud service serves the data files / roundtrips to DB catalog / API calls to a Rest catalog. I'm strongly in favor with sticking with a tried and tested implementation and put some real world benchmarks in place and only if those show tracing to be a bottleneck consider choosing a less active alternative. I also do not agree with the sentiment that it is a problem to address tracing & logging in the same crate. It rather provides very useful debugging utilities, e.g. when used in a server, some middleware can create a span containing a request id, now every (tracing-)log statement within that span will have the request id attached to it so your favorite log aggregator can filter by this request id to get all logs pertaining to it which is incredibly helpful when dealing with production issues. Regarding metrics, I'm not sure I understand the issue, there exists https://crates.io/crates/metrics-prometheus which provides integration of prometheus with metrics.rs. |
Hey guys, some thoughts from my side:
|
Thank you all for joining the discussion. It seems most people prefer using The left part from my side is Context from opendal. OpenDAL supports ALL. Regardless of the library chosen, we simply need to enable the corresponding layer for it.
|
Some additional context. Renjie mentioned:
This is not quite right. You can use the tracing_log crate to redirect any other crates logs that went to use tracing_log::LogTracer;
// redirects all output sent to log:* to their tracing::* equivalent
LogTracer::init().expect("Failed to set logger"); Regarding metrics, my preference would be either |
It sounds like Regarding telemetry, does anyone have objections to the choice of |
I'm fine with this. Also cc @liurenjie1024 for ideas.
I will support to use |
+1.
+1. I would also prefer to use |
I have a question: the LogTracer in tracing_log is used to convert the log record into a trace event. Does this mean that if the user uses log originally and after they introduce iceberg-rust, they have two choices:
For users, is it a more ideal behavior to redirect the trace to the original log? 🤔 |
As a library for production usage, it would be useful to add logging, metrics, and tracing support. We need to do some investigation about to fit into rust ecosystem as much as possible.
The text was updated successfully, but these errors were encountered: