BlakeOrth commented on code in PR #18103:
URL: https://github.com/apache/datafusion/pull/18103#discussion_r2437229437
##########
datafusion-cli/src/object_storage/instrumented.rs:
##########
@@ -138,6 +143,26 @@ impl InstrumentedObjectStore {
Ok(ret)
}
+
+ fn instrumented_list(
+ &self,
+ prefix: Option<&Path>,
+ ) -> BoxStream<'static, Result<ObjectMeta>> {
+ let timestamp = Utc::now();
+ let ret = self.inner.list(prefix);
+
+ self.requests.lock().push(RequestDetails {
+ op: Operation::List,
+ path: prefix.cloned().unwrap_or_else(|| Path::from("")),
+ timestamp,
+ duration: None, // list returns a stream, so the duration isn't
meaningful
Review Comment:
Yes, not being able to easily evaluate a meaningful duration from this is a
pretty big bummer honestly. I think time to first response is probably the
ideal measurement to take here. I briefly looked into what it would take to
make that happen within this instrumented store and I think it ends up being
quite complex. I'm pretty sure we'd have to write a custom future to wrap the
elements within the stream since the duration is only meaningful once elements
in the stream start reporting `Poll::Ready`. Hopefully there's an easier way,
because that sounds pretty painful.
--
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]
