weiqingy commented on code in PR #28692:
URL: https://github.com/apache/flink/pull/28692#discussion_r3592851558


##########
docs/content/docs/ops/metrics.md:
##########
@@ -2441,6 +2469,22 @@ It is recommended to only use them for debugging 
purposes.
 If state.ttl is enabled, the size of the value will include the size of the 
TTL-related timestamp.
 The value size of AggregatingState is not accounted for because 
AggregatingState returns a result processed by a user-defined 
AggregateFunction, whereas currently, only the actual stored data size in the 
state can be tracked.
 
+## UDF metrics
+
+Flink can track the processing time and exceptions of SQL/Table user-defined 
functions on a per-operator basis. This feature is disabled by default.
+To enable it you must set `table.exec.udf-metric-enabled` to `true`. When 
disabled, nothing is registered and there is no overhead.
+
+Once enabled, each user-defined function gets two metrics on the operator that 
runs it, scoped under `udf.<udf_name>`: a `udfProcessingTime` histogram and a 
`udfExceptionCount` counter.
+
+Flink samples the processing time every `N` invocations, in which `N` is 
defined by `table.exec.udf-metric.sample-interval`.
+This configuration has a default value of 100. A smaller value will get more 
accurate results but have a higher performance impact since it is sampled more 
frequently.
+Exceptions are counted on every invocation and are not sampled.
+
+The metrics cover synchronous and asynchronous scalar and table functions. 
Process table functions (`PROCESS_TABLE`), aggregate functions, and functions 
registered through the deprecated `registerFunction` API are not covered.
+
+<span class="label label-danger">Warning</span> Enabling UDF metrics may 
impact the performance, both from the sampled timing and from the metric 
reporter.
+It is recommended to only use them for debugging purposes.

Review Comment:
   Thanks for taking a look, David. Good questions on both.
   
   On the wording: the "…may impact performance / recommended only for 
debugging" note mirrors the existing State Access Latency and state-size metric 
sections a bit higher on this same page. It's the standard caution for opt-in, 
per-record operator metrics, not a signal that UDF metrics are unusually costly.
   
   On the actual overhead — it should be minimal, and that's the design intent, 
via two layers:
   
   - Off by default: when `table.exec.udf-metric-enabled` is `false`, nothing 
is registered and there is zero overhead.
   - When enabled, it uses the same counter-based sampling as state latency 
tracking (FLINK-21736): only every Nth invocation is timed (default 
`sample-interval` = 100) and the non-sampled fast path is a single integer 
increment. Exceptions are counted every invocation, but that's just a counter 
increment.
   
   So on the common path the added per-record cost is essentially a branch plus 
an int increment, with the `nanoTime()`/histogram work amortized across the 
sampling interval. In local micro-benchmarking the non-sampled path was 
sub-nanosecond; a rigorous JMH benchmark belongs in `flink-benchmarks` and I 
can add one.
   
   Happy to make the sampling context clearer in this note (e.g. that overhead 
is minimal at the default interval) when I make the PR review-ready.
   
   One heads-up: this PR is intentionally still a draft. I'm keeping it in 
draft until the FLIP-485 vote concludes, then I'll mark it review-ready. 
Appreciate the early look.
   



-- 
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]

Reply via email to