[GitHub] [samza] mynameborat commented on a change in pull request #1368: SAMZA-2532: Refactor MetricsSnapshotReporter and MetricsSnapshotReporterFactory

[GitBox <gi...@apache.org>]

mynameborat commented on a change in pull request #1368:
URL: https://github.com/apache/samza/pull/1368#discussion_r431440392



##########
File path: samza-core/src/main/scala/org/apache/samza/metrics/reporter/MetricsSnapshotReporterFactory.scala
##########
@@ -77,15 +71,48 @@ class MetricsSnapshotReporterFactory extends MetricsReporterFactory with Logging
     } else {
       new MetricsSnapshotSerdeV2
     }
-
     info("Got serde %s." format serde)
+    serde
+  }
+
 
-    val pollingInterval: Int = metricsConfig.getMetricsSnapshotReporterInterval(name)
+  def getBlacklist(reporterName: String, config: Config): Option[String] = {
+    val metricsConfig = new MetricsConfig(config)
+    val blacklist = JavaOptionals.toRichOptional(metricsConfig.getMetricsSnapshotReporterBlacklist(reporterName)).toOption
+    info("Got blacklist as: %s" format blacklist)
+    blacklist
+  }
 
-    info("Setting polling interval to %d" format pollingInterval)
+  def getPollingInterval(reporterName: String, config: Config): Int = {
+    val metricsConfig = new MetricsConfig(config)
+    val pollingInterval = metricsConfig.getMetricsSnapshotReporterInterval(reporterName)

Review comment:
       why is this `pollingInterval` instead of `reportingInterval`? makes me think we poll for metrics at this interval but then the error statement above which says "Error reporting ... `pollingInterval`" is confusing.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org