2018-07-05 20:35:28 UTC - Rajan Dhabalia: hi @Sijie Guo @Jerry Peng @Sanjeev Kulkarni @Matteo Merli
I have a qq to understand how stats/metrics works in functions. 1. I can see that JavaInstanceRunnable has a method to generate and reset metrics: <https://github.com/apache/incubator-pulsar/blob/master/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L364> 2. It is being triggered by FunctionsStatsGenerator <https://github.com/apache/incubator-pulsar/blob/master/pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/FunctionsStatsGenerator.java#L52> Questions: 1. Who triggers stats-generation at FunctionsStatsGenerator which can generate and reset stats every 1 minute to capture rate-per-second for every function-stats? as a user, if I don't want to use prometheous then do I have to schedule a task which can generate-and-reset the stats every x (1) minute? 2. Right now, user-metrics is being collected for function-processing: <https://github.com/apache/incubator-pulsar/blob/master/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstance.java#L60> as a user, if I also want to capture user-metrics at Sink then is if fine if I can pass same context to sink to capture user-metrics? ---- 2018-07-05 21:34:17 UTC - Jerry Peng: @Rajan Dhabalia responding inline 1. Who triggers stats-generation at FunctionsStatsGenerator which can generate and reset stats every 1 minute to capture rate-per-second for every function-stats? as a user, if I don’t want to use prometheous then do I have to schedule a task which can generate-and-reset the stats every x (1) minute? correct, currently the implementation expects an external source to trigger get metrics and reset periodically like the rest of the metrics from the broker. Prometheus will do this automatically for you but if you want to get it yourself, you will have to schedule a task to do that. ---- 2018-07-05 21:38:00 UTC - Jerry Peng: 2. Right now, user-metrics is being collected for function-processing: <https://github.com/apache/incubator-pulsar/blob/master/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstance.java#L60> as a user, if I also want to capture user-metrics at Sink then is if fine if I can pass same context to sink to capture user-metrics? How do you intend to pass the context to the sink? We cannot easily add the same context to the sink interface since the context passed into a function assumes its getting data from a topic and writing it to another topic. Which might not be the case for a sink. We can potentially expose a modified interface to record metrics ---- 2018-07-05 21:43:16 UTC - Rajan Dhabalia: > How do you intend to pass the context to the sink? I mean, function is capturing user-metrics from the context: <https://github.com/apache/incubator-pulsar/blob/master/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L377> if we can pass same context to sink eg: KinesisSink then KinesisSink can add its user-metrics in it such as eg: kinesisPublishLatency, kinesisSuccess, kinesisFailures .. so, function doesn't have to maintain multiple contexts and all user-metrics can be captured by one context.. ---- 2018-07-05 21:49:40 UTC - Rajan Dhabalia: > We cannot easily add the same context to the sink interface since the context passed into a function assumes its getting data from a topic and writing it to another topic. Which might not be the case for a sink. That's true, we can't pass same context to sink.. > We can potentially expose a modified interface to record metrics correct..we mainly require to pass `ContextImpl.accumulatedMetrics` to sink which can be used to capture metrics ---- 2018-07-05 21:52:04 UTC - Rajan Dhabalia: so, would you like to create a PR or I can create a PR to accommodate this change.? ---- 2018-07-05 21:53:43 UTC - Jerry Peng: actually we can probably use the same context in sink as well since with a sink, the source is always PulsarSource and the function is set to the identity function ---- 2018-07-05 21:55:37 UTC - Jerry Peng: but for the sake of keeping things separate/modular, lets have different interfaces ---- 2018-07-05 21:55:57 UTC - Jerry Peng: The “Context” interface will be for functions and we can create a “SinkContext” for the sink ---- 2018-07-05 21:56:10 UTC - Jerry Peng: ContextImpl can implement both ---- 2018-07-05 21:56:45 UTC - Jerry Peng: maybe the SinkContext can just expose a subset of the functionality in ContextImpl ---- 2018-07-05 21:57:37 UTC - Jerry Peng: @Rajan Dhabalia how does that sound? ---- 2018-07-05 22:05:27 UTC - Rajan Dhabalia: yes, it seems we can can't break existing sink-api: `void write(RecordContext inputRecordContext, T value)` So, SinkContext = `RecordContext` can implement `Context` and it can implement two more apis from context 1. `void recordMetric(String metricName, double value)` 2. `MetricsData getAndResetMetrics()` ---- 2018-07-05 22:11:08 UTC - Rajan Dhabalia: will that be fine? or any thoughts ? ---- 2018-07-05 22:20:24 UTC - Sijie Guo: sink-api is not released yet. so we can break the api now +1 : Rajan Dhabalia ---- 2018-07-05 22:20:39 UTC - Jerry Peng: ^yup ---- 2018-07-05 22:20:53 UTC - Sijie Guo: if you can work on the change in the next few days, we can include that for 2.1 release. ---- 2018-07-05 22:21:15 UTC - Sijie Guo: since 2.1 release is anyway delayed by connector packaging issue ---- 2018-07-05 22:22:32 UTC - Rajan Dhabalia: I see.. ok.. @Jerry Peng can you create a PR or I can make the change for it? ---- 2018-07-05 22:27:22 UTC - Jerry Peng: @Rajan Dhabalia what is the use case for adding getAndResetMetrics? Do you need to do that in the code of the source and sink? ---- 2018-07-05 22:28:43 UTC - Rajan Dhabalia: not in source/sink.. but it can be called by JavaInstance to getandReset metrics.. ---- 2018-07-05 22:28:53 UTC - Rajan Dhabalia: source/sink will only require recordMetric ---- 2018-07-05 22:29:52 UTC - Jerry Peng: oh ok I will work on a PR +1 : Rajan Dhabalia ----
