[Improvement] Expose KyuubiOperation exec time metrics
Code of Conduct
- [X] I agree to follow this project's Code of Conduct
Search before asking
- [X] I have searched in the issues and found no similar issues.
What would you like to be improved?
Kyuubi already supports ExecuteStatement exec_time metrics. If KyuubiOperation (LaunchEngine, ExecutedCommandExec) supports exec_time metrics, it will be more convenient when troubleshooting.
How should we improve?
KyuubiOperation (LaunchEngine, ExecutedCommandExec) supports exec_time metrics
Are you willing to submit PR?
- [X] Yes. I would be willing to submit a PR with guidance from the Kyuubi community to improve.
- [ ] No. I cannot submit a PR at this time.
I also agree that these metrics would be very useful for troubleshooting.
I have noticed that in current implementation, metrics of ExecuteStatement exec_time is got within org.apache.kyuubi.operation.ExecuteStatement.
I wonder if we could move this kind of logic inside org.apache.kyuubi.operation.KyuubiOperation#setState to track all operation execution time.
override def setState(newState: OperationState): Unit = {
MetricsSystem.tracing { ms =>
if (!OperationState.isTerminal(state)) {
ms.markMeter(MetricRegistry.name(OPERATION_STATE, opType, state.toString.toLowerCase), -1)
// add operation exec time tracking logic here
val execTime = System.currentTimeMillis() - startTime
ms.updateHistogram(
MetricRegistry.name(OPERATION_EXEC_TIME, opType),
execTime)
}
ms.markMeter(MetricRegistry.name(OPERATION_STATE, opType, newState.toString.toLowerCase))
ms.markMeter(MetricRegistry.name(OPERATION_STATE, newState.toString.toLowerCase))
}
super.setState(newState)
if (eventEnabled) EventBus.post(KyuubiOperationEvent(this))
}
extending the metrics scope to all operations makes sense to me, please go head to prepare the PR.