ryucc commented on code in PR #25068:
URL: https://github.com/apache/beam/pull/25068#discussion_r1082962347
##########
runners/samza/src/main/java/org/apache/beam/runners/samza/runtime/SamzaDoFnRunners.java:
##########
@@ -388,6 +407,7 @@ private void emitMetrics() {
final long finishBundleTime = System.nanoTime();
final long averageProcessTime = (finishBundleTime - startBundleTime) /
count;
+ String metricName = "ExecutableStage-" + stepName + "-process-ns";
Review Comment:
I prefer not to use instance variables. We already have a large pool of it,
and would like to keep only primitive data in there.
On the same note, I want to keep data as local as possible. This
`metricName` seems only useful for the emitMetrics method at the moment (it's
specific to Executable stage time) , and may conflict with other metric names
if the future.
I think putting the computation here improves the code cleanness, and out
weights the cost to compute it every time.
--
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]
