adriangb commented on PR #27110: URL: https://github.com/apache/datafusion/pull/27110#issuecomment-6685764396
I understand the hesitation to bring in tracing. As you already pointed our right now this is a dev only dep so there is zero impact / direct decision being made about what event / tracing API Datafusion exposes more generally. I think we both agree that it is somewhat of a slippery slope, but only in the sense that it's easy to say "hey all of these operations are instrumented, why not just expose them to users!". That said, if we do want to discuss what should Datafusion offer as it's event / tracing implementation I see 3 options: 1. Pick a single crate (which IMO should be `tracing` or `opentelemetry` given https://lib.rs/crates/ numbers). You can bridge between them, e.g. we use opentelemetry but have plenty of crates that offer only tracing instrumentation. For example using https://github.com/tokio-rs/tracing-opentelemetry. 2. Offer multiple instrumentation. We offer feature flags for various instrumentations. You can turn on one or many. It'd be pretty straightforward to tell an LLM to add a new instrumentation. This is mechanical code that would not be hard to maintain and doesn't need to be perfect. Realistically I think this is 2-3 instrumenations. Companies that use custom instrumentations would need to build a bridge between a standard open source instrumenation and their internal implementation (which IMO is a reasonable ask). 3. Create our own events API. IMO this requires design work, compatibility guarantees, etc. And every single consumer needs to build some sort of bridge into whatever they use for tracing. I find this to be the worst of all worlds: more work for us, more work for users. Thus my vote would be to: 1. If we find this approach useful for testing (and maybe we don't, that should be discussed more on it's own merits) make changes to it / touch it up and adopt it. 2. Gradually expand to planning, other areas of the codebase like you indicated & add utilities like filtering events, selecting which attributes to show, other views of the data that we can snapshot in tests. 3. If it makes sense we can later decide to make this a feature flagged feature exposed to users. 4. If users request other integrations e.g. `opentelemetry` instead of `tracing` (and there are good arguments for not just using a bridge) we can add a second feature flag. -- 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]
