[GitHub] [gobblin] umustafi commented on a diff in pull request #3500: [GOBBLIN-1639] Prevent metrics reporting if configured, clean up workunit count metric

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

umustafi commented on code in PR #3500:
URL: https://github.com/apache/gobblin/pull/3500#discussion_r866144603


##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/AbstractJobLauncher.java:
##########
@@ -685,8 +674,23 @@ public void apply(JobListener jobListener, JobContext jobContext)
         }
       }
     } finally {
-      // Stop metrics reporting
+      // Register metrics then stop metrics reporting, metrics will flush and emit when the metricsContext is closed
       if (this.jobContext.getJobMetricsOptional().isPresent()) {
+        if (jobState.getPropAsBoolean(ConfigurationKeys.GOBBLIN_OUTPUT_JOB_LEVEL_METRICS, true)) {
+          String workunitCreationGaugeName = MetricRegistry.name(ServiceMetricNames.GOBBLIN_JOB_METRICS_PREFIX,
+              TimingEvent.LauncherTimings.WORK_UNITS_CREATION, jobState.getJobName());
+          long workUnitsCreationTime = workUnitsCreationTimer.getDuration() / TimeUnit.SECONDS.toMillis(1);

Review Comment:
   I agree that we should always emit a workunit count because this is most useful when the workflow does _not_ perform work. Wrapping it in a try can make this safer to avoid throwing an exception. 



-- 
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: dev-unsubscribe@gobblin.apache.org

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