feat(cli): Add query lifecycle callbacks to Console (#1057)#1057
feat(cli): Add query lifecycle callbacks to Console (#1057)#1057amitkdutta wants to merge 1 commit intofacebookincubator:mainfrom
Conversation
|
@amitkdutta has exported this pull request. If you are a Meta employee, you can view the originating Diff in D96492425. |
…r#1057) Summary: Add optional start and completion callbacks to Console so callers can hook into the query lifecycle for logging, metrics, or other processing. - startCallback: invoked before parse for every query. - completionCallback: invoked after execution with query timing, row counts, and success/failure status. Differential Revision: D96492425
6678f16 to
1a73fb6
Compare
axiom/cli/Console.h
Outdated
| std::string queryId; | ||
| std::string_view query; | ||
| bool succeeded{true}; | ||
| std::string errorMessage; |
There was a problem hiding this comment.
Should this be an std::optional?
| std::string errorMessage; | ||
| /// Serialized query plan for downstream logging. | ||
| std::string planString; | ||
| uint64_t parseMicros{0}; |
There was a problem hiding this comment.
Would you document all the fields?
I think we need timing for parsing, optimizing and execution. optimizeMicros is missing.
| uint64_t executionMicros{0}; | ||
| int64_t numOutputRows{0}; | ||
| std::chrono::system_clock::time_point createTime; | ||
| std::chrono::system_clock::time_point endTime; |
There was a problem hiding this comment.
Let's change the order of fields to match QueryCompletionInfo.
- queryId
- query
- createTime
- endTime
- succeeded
- optional errorMessage
- parseMicros
- optimizeMicros
- executeMicros
- numOutputRows
- planString
Let's discuss whether we want a flat struct or perhaps group related fields into their own structs.
For example, I expect we'll want more execution stats beyond numOutputRows. Similarly, we may want more info about the plan shape. Maybe it doesn't make sense to duplicate QueryStartInfo fields, but rather add a field of type QueryStartInfo. Let's discuss.
1a73fb6 to
574791f
Compare
…r#1057) Summary: Add optional start and completion callbacks to Console so callers can hook into the query lifecycle for logging, metrics, or other processing. - startCallback: invoked before parse for every query. - completionCallback: invoked after execution with query timing, row counts, and success/failure status. Reviewed By: hdikeman Differential Revision: D96492425
574791f to
f7343db
Compare
…r#1057) Summary: Pull Request resolved: facebookincubator#1057 Add optional start and completion callbacks to Console so callers can hook into the query lifecycle for logging, metrics, or other processing. - startCallback: invoked before parse for every query. - completionCallback: invoked after execution with query timing, row counts, and success/failure status. Reviewed By: hdikeman Differential Revision: D96492425
…r#1057) Summary: Add optional start and completion callbacks to Console so callers can hook into the query lifecycle for logging, metrics, or other processing. - startCallback: invoked before parse for every query. - completionCallback: invoked after execution with query timing, row counts, and success/failure status. Reviewed By: hdikeman Differential Revision: D96492425
f7343db to
2522857
Compare
…r#1057) Summary: Add optional start and completion callbacks to Console so callers can hook into the query lifecycle for logging, metrics, or other processing. - startCallback: invoked before parse for every query. - completionCallback: invoked after execution with query timing, row counts, and success/failure status. Reviewed By: hdikeman Differential Revision: D96492425
2522857 to
ba9ab15
Compare
Summary:
Add optional start and completion callbacks to Console so callers can
hook into the query lifecycle for logging, metrics, or other processing.
counts, and success/failure status.
Reviewed By: hdikeman
Differential Revision: D96492425