Re: Re: [DISCUSS] Add UDF Metrics

2025-07-31 Thread Weiqing Yang
Hi Shengkai, Alan and Xuyang, Just checking in - do you have any concerns or feedback? If there are no further objections from anyone, I’ll mark the FLIP as ready for voting. Best, Weiqing On Mon, Jul 14, 2025 at 9:10 PM Weiqing Yang wrote: > Hi Xuyang, > > Thank you for reviewing the propo

Re: Re: [DISCUSS] Add UDF Metrics

2025-07-14 Thread Weiqing Yang
Hi Xuyang, Thank you for reviewing the proposal! I’m planning to use: *udf.metrics.process-time* and *udf.metrics.exception-count*. These follow the naming convention used in Flink (e.g., RocksDB native metrics

Re: [DISCUSS] Add UDF Metrics

2025-07-12 Thread Weiqing Yang
Hi Alan, Thanks for reviewing the proposal and for highlighting the ASYNC_TABLE work. Yes, I’ve updated the proposal to cover both ASYNC_SCALAR and ASYNC_TABLE. For async UDFs, the plan is to instrument both the invokeAsync() call and the async callback handler to measure the full end-to-end late

Re: [DISCUSS] Add UDF Metrics

2025-07-12 Thread Weiqing Yang
Hi Shengkai, Thanks for reviewing the proposal and for the thoughtful feedback. 1. Metric hierarchy - Makes sense. I’ve updated the proposal to scope the UDF metric group under OperatorMetricGroup. 2. Naming convention - UDF metrics will follow this pattern: ... For example, a

Re: [DISCUSS] Add UDF Metrics

2025-07-10 Thread Alan Sheinberg
Hi Weiqing, >From your doc, the entrypoint for UDF calls in the codegen is ExprCodeGenerator which should invoke BridgingSqlFunctionCallGen, which could be instrumented with metrics. This works well for synchronous calls, but what about ASYNC_SCALAR and the soon to be merged ASYNC_TABLE ( https:/

Re: [DISCUSS] Add UDF Metrics

2025-07-09 Thread Shengkai Fang
I just have some questions: 1. The current metrics hierarchy shows that the UDF metric group belongs to the TaskMetricGroup. I think it would be better for the UDF metric group to belong to the OperatorMetricGroup instead, because a UDF might be used by multiple operators. 2. What are the naming c