sv2000 commented on a change in pull request #3065:
URL: https://github.com/apache/incubator-gobblin/pull/3065#discussion_r458190950
##########
File path:
gobblin-metrics-libs/gobblin-metrics/src/main/java/org/apache/gobblin/metrics/GobblinMetrics.java
##########
@@ -367,6 +374,20 @@ public void startMetricReportingWithFileSuffix(State
state, String metricsFileSu
oldMetricsFileSuffix += "." + metricsFileSuffix;
}
metricsReportingProps.setProperty(ConfigurationKeys.METRICS_FILE_SUFFIX,
oldMetricsFileSuffix);
+
+ if (!Strings.isNullOrEmpty(metricsFileSuffix)) {
Review comment:
I think GobblinMetrics should be agnostic to the execution mode of the
job, and shouldn't have to worry about extracting mapper/reducer num from the
task id. This block can be moved inside MRJobLauncherClass and the
configurations such as MAPPER_TASK_NUM_KEY can be set there and added into the
state object.
##########
File path:
gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java
##########
@@ -708,6 +708,11 @@
public static final String DEFAULT_METRICS_REPORTING_FILE_ENABLED =
Boolean.toString(false);
public static final String METRICS_LOG_DIR_KEY =
METRICS_CONFIGURATIONS_PREFIX + "log.dir";
public static final String METRICS_FILE_SUFFIX =
METRICS_CONFIGURATIONS_PREFIX + "reporting.file.suffix";
+ public static final String MR_TYPE_KEY = METRICS_CONFIGURATIONS_PREFIX +
"mr.type";
Review comment:
Since these configuration keys are dynamically set and not user
provided, its better to define them inside the MRJobLauncher class.
----------------------------------------------------------------
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:
us...@infra.apache.org
