Addressing comments about reporter configs
Project: http://git-wip-us.apache.org/repos/asf/storm/repo Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/6811c9b8 Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/6811c9b8 Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/6811c9b8 Branch: refs/heads/1.x-branch Commit: 6811c9b82873ca48efbc337e5493c7aba85d5731 Parents: d519815 Author: Kishor Patil <[email protected]> Authored: Thu Feb 4 14:19:02 2016 -0600 Committer: Kishor Patil <[email protected]> Committed: Fri Feb 5 19:30:06 2016 +0000 ---------------------------------------------------------------------- storm-core/src/jvm/org/apache/storm/Config.java | 10 ++++- .../storm/daemon/metrics/MetricsUtils.java | 39 ++++++++++++++++---- .../reporters/ConsolePreparableReporter.java | 8 ++-- .../reporters/CsvPreparableReporter.java | 23 ++---------- .../reporters/JmxPreparableReporter.java | 8 ++-- 5 files changed, 52 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/storm/blob/6811c9b8/storm-core/src/jvm/org/apache/storm/Config.java ---------------------------------------------------------------------- diff --git a/storm-core/src/jvm/org/apache/storm/Config.java b/storm-core/src/jvm/org/apache/storm/Config.java index 49306eb..a456bb2 100644 --- a/storm-core/src/jvm/org/apache/storm/Config.java +++ b/storm-core/src/jvm/org/apache/storm/Config.java @@ -150,7 +150,7 @@ public class Config extends HashMap<String, Object> { * Use the specified IETF BCP 47 language tag string for a Locale. */ @isString - public static final String STORM_DAEMON_METRICS_REPORTER_PLUGIN_LOCALE = "storm.daemon.metrics.reporter.plugin.local"; + public static final String STORM_DAEMON_METRICS_REPORTER_PLUGIN_LOCALE = "storm.daemon.metrics.reporter.plugin.locale"; /** * A specify domain for daemon metrics reporter plugin to limit reporting to specific domain. @@ -169,6 +169,14 @@ public class Config extends HashMap<String, Object> { */ @isString public static final String STORM_DAEMON_METRICS_REPORTER_PLUGIN_DURATION_UNIT = "storm.daemon.metrics.reporter.plugin.duration.unit"; + + + /** + * A specify csv reporter directory for CvsPreparableReporter daemon metrics reporter. + */ + @isString + public static final String STORM_DAEMON_METRICS_REPORTER_CSV_LOG_DIR = "storm.daemon.metrics.reporter.csv.log.dir"; + /** * A list of hosts of ZooKeeper servers used to manage the cluster. */ http://git-wip-us.apache.org/repos/asf/storm/blob/6811c9b8/storm-core/src/jvm/org/apache/storm/daemon/metrics/MetricsUtils.java ---------------------------------------------------------------------- diff --git a/storm-core/src/jvm/org/apache/storm/daemon/metrics/MetricsUtils.java b/storm-core/src/jvm/org/apache/storm/daemon/metrics/MetricsUtils.java index 4425f59..aa5ce28 100644 --- a/storm-core/src/jvm/org/apache/storm/daemon/metrics/MetricsUtils.java +++ b/storm-core/src/jvm/org/apache/storm/daemon/metrics/MetricsUtils.java @@ -6,9 +6,9 @@ * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at - * + * <p/> * http://www.apache.org/licenses/LICENSE-2.0 - * + * <p/> * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -20,10 +20,12 @@ package org.apache.storm.daemon.metrics; import org.apache.storm.Config; import org.apache.storm.daemon.metrics.reporters.JmxPreparableReporter; import org.apache.storm.daemon.metrics.reporters.PreparableReporter; +import org.apache.storm.utils.ConfigUtils; import org.apache.storm.utils.Utils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; import java.util.ArrayList; import java.util.List; import java.util.Locale; @@ -38,11 +40,11 @@ public class MetricsUtils { List<PreparableReporter> reporterList = new ArrayList<>(); if (clazzes != null) { - for(String clazz: clazzes ) { + for (String clazz : clazzes) { reporterList.add(getPreparableReporter(clazz)); } } - if(reporterList.isEmpty()) { + if (reporterList.isEmpty()) { reporterList.add(new JmxPreparableReporter()); } return reporterList; @@ -51,7 +53,7 @@ public class MetricsUtils { private static PreparableReporter getPreparableReporter(String clazz) { PreparableReporter reporter = null; LOG.info("Using statistics reporter plugin:" + clazz); - if(clazz != null) { + if (clazz != null) { reporter = (PreparableReporter) Utils.newInstance(clazz); } return reporter; @@ -59,7 +61,7 @@ public class MetricsUtils { public static Locale getMetricsReporterLocale(Map stormConf) { String languageTag = Utils.getString(stormConf.get(Config.STORM_DAEMON_METRICS_REPORTER_PLUGIN_LOCALE), null); - if(languageTag != null) { + if (languageTag != null) { return Locale.forLanguageTag(languageTag); } return null; @@ -75,9 +77,32 @@ public class MetricsUtils { private static TimeUnit getTimeUnitForCofig(Map stormConf, String configName) { String rateUnitString = Utils.getString(stormConf.get(configName), null); - if(rateUnitString != null) { + if (rateUnitString != null) { return TimeUnit.valueOf(rateUnitString); } return null; } + + public static File getCsvLogDir(Map stormConf) { + String csvMetricsLogDirectory = Utils.getString(stormConf.get(Config.STORM_DAEMON_METRICS_REPORTER_CSV_LOG_DIR), null); + if (csvMetricsLogDirectory == null) { + csvMetricsLogDirectory = ConfigUtils.absoluteHealthCheckDir(stormConf); + csvMetricsLogDirectory = csvMetricsLogDirectory + ConfigUtils.FILE_SEPARATOR + "csvmetrics"; + } + File csvMetricsDir = new File(csvMetricsLogDirectory); + validateCreateOutputDir(csvMetricsDir); + return csvMetricsDir; + } + + private static void validateCreateOutputDir(File dir) { + if (!dir.exists()) { + dir.mkdirs(); + } + if (!dir.canWrite()) { + throw new IllegalStateException(dir.getName() + " does not have write permissions."); + } + if (!dir.isDirectory()) { + throw new IllegalStateException(dir.getName() + " is not a directory."); + } + } } http://git-wip-us.apache.org/repos/asf/storm/blob/6811c9b8/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/ConsolePreparableReporter.java ---------------------------------------------------------------------- diff --git a/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/ConsolePreparableReporter.java b/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/ConsolePreparableReporter.java index 2f466ce..3ef4237 100644 --- a/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/ConsolePreparableReporter.java +++ b/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/ConsolePreparableReporter.java @@ -6,9 +6,9 @@ * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at - * + * <p/> * http://www.apache.org/licenses/LICENSE-2.0 - * + * <p/> * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -61,7 +61,7 @@ public class ConsolePreparableReporter implements PreparableReporter<ConsoleRepo @Override public void start() { - if (reporter != null ) { + if (reporter != null) { LOG.debug("Starting..."); reporter.start(10, TimeUnit.SECONDS); } else { @@ -71,7 +71,7 @@ public class ConsolePreparableReporter implements PreparableReporter<ConsoleRepo @Override public void stop() { - if (reporter !=null) { + if (reporter != null) { LOG.debug("Stopping..."); reporter.stop(); } else { http://git-wip-us.apache.org/repos/asf/storm/blob/6811c9b8/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/CsvPreparableReporter.java ---------------------------------------------------------------------- diff --git a/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/CsvPreparableReporter.java b/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/CsvPreparableReporter.java index 28fd605..605f389 100644 --- a/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/CsvPreparableReporter.java +++ b/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/CsvPreparableReporter.java @@ -6,9 +6,9 @@ * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at - * + * <p/> * http://www.apache.org/licenses/LICENSE-2.0 - * + * <p/> * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -19,9 +19,7 @@ package org.apache.storm.daemon.metrics.reporters; import com.codahale.metrics.CsvReporter; import com.codahale.metrics.MetricRegistry; -import org.apache.storm.Config; import org.apache.storm.daemon.metrics.MetricsUtils; -import org.apache.storm.utils.Utils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,10 +52,7 @@ public class CsvPreparableReporter implements PreparableReporter<CsvReporter> { builder.convertDurationsTo(durationUnit); } - String localStormDirLocation = Utils.getString(stormConf.get(Config.STORM_LOCAL_DIR), "."); - File csvMetricsDir = new File(localStormDirLocation + System.getProperty("file.separator") + "csvmetrics" ); - validateCreateOutputDir(csvMetricsDir); - + File csvMetricsDir = MetricsUtils.getCsvLogDir(stormConf); reporter = builder.build(csvMetricsDir); } @@ -81,17 +76,5 @@ public class CsvPreparableReporter implements PreparableReporter<CsvReporter> { } } - - private void validateCreateOutputDir(File dir) { - if (!dir.exists()) { - dir.mkdirs(); - } - if (!dir.canWrite()) { - throw new IllegalStateException(dir.getName() + " does not have write permissions."); - } - if (!dir.isDirectory()) { - throw new IllegalStateException(dir.getName() + " is not a directory."); - } - } } http://git-wip-us.apache.org/repos/asf/storm/blob/6811c9b8/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/JmxPreparableReporter.java ---------------------------------------------------------------------- diff --git a/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/JmxPreparableReporter.java b/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/JmxPreparableReporter.java index eff6e5a..cf4aa1c 100644 --- a/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/JmxPreparableReporter.java +++ b/storm-core/src/jvm/org/apache/storm/daemon/metrics/reporters/JmxPreparableReporter.java @@ -6,9 +6,9 @@ * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at - * + * <p/> * http://www.apache.org/licenses/LICENSE-2.0 - * + * <p/> * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -50,7 +50,7 @@ public class JmxPreparableReporter implements PreparableReporter<JmxReporter> { @Override public void start() { - if (reporter != null ) { + if (reporter != null) { LOG.debug("Starting..."); reporter.start(); } else { @@ -60,7 +60,7 @@ public class JmxPreparableReporter implements PreparableReporter<JmxReporter> { @Override public void stop() { - if (reporter !=null) { + if (reporter != null) { LOG.debug("Stopping..."); reporter.stop(); } else {
