blackmwk commented on PR #2467: URL: https://github.com/apache/iceberg-rust/pull/2467#issuecomment-4573736945
Thanks @xanderbailey for this pr, but I'm not convinced that this is the right direction to go. The motivation you mentioned have two parts: 1. Not relying on `tracing` for observability. This is only a benefit in theory, in practice what user are mostly concerned about are collecting the spans with different sink rather than the library used to generate the spans. `tracing` is widely used and has a large ecosystem, what's more we are already relying on `tracing` for logging and method tracking. 2. Library consumers decide what to instrument. This is a good point, but I don't know what kinds of information library consumer could get from a `BoxFuture`. In fact, tokio runtime has provided some instrument point: see [on_after_task_poll](https://docs.rs/tokio/latest/tokio/runtime/struct.Builder.html#method.on_after_task_poll), [on_before_task_poll](https://docs.rs/tokio/latest/tokio/runtime/struct.Builder.html#method.on_before_task_poll), etc. I think the context provided by tokio's instrument point is more useful than a `BoxFuture`, and that's the right direction to go since iceberg crate's Runtime is built one tokio Runtime, which is created and maintained by user. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
