Code Maintenance - remove warnings (#631)

* Removing duplicate code

* Fix some minor codestyle issues

* Fixing duplicated code

* Fixing warnings

* fixing minor warnings

src/main/java/org/jenkinsci/plugins/prometheus/JobCollector.java

@@ -8,9 +8,9 @@
 import org.jenkinsci.plugins.prometheus.collectors.CollectorType;
 import org.jenkinsci.plugins.prometheus.collectors.MetricCollector;
 import org.jenkinsci.plugins.prometheus.collectors.builds.BuildCompletionListener;
+import org.jenkinsci.plugins.prometheus.collectors.builds.BuildCompletionListener.CloseableIterator;
 import org.jenkinsci.plugins.prometheus.collectors.builds.CounterManager;
 import org.jenkinsci.plugins.prometheus.collectors.builds.JobLabel;
-import org.jenkinsci.plugins.prometheus.collectors.builds.BuildCompletionListener.CloseableIterator;
 import org.jenkinsci.plugins.prometheus.config.PrometheusConfiguration;
 import org.jenkinsci.plugins.prometheus.util.Jobs;
 import org.jenkinsci.plugins.prometheus.util.Runs;
@@ -23,16 +23,9 @@
 
 public class JobCollector extends Collector {
 
-    private static final Logger logger = LoggerFactory.getLogger(JobCollector.class);
-    private static final String NOT_AVAILABLE = "NA";
-    private static final String UNDEFINED = "UNDEFINED";
+    private static final Logger LOGGER = LoggerFactory.getLogger(JobCollector.class);
 
     private MetricCollector<Run<?, ?>, ? extends Collector> summary;
-    private MetricCollector<Run<?, ?>, ? extends Collector> jobSuccessCount;
-    private MetricCollector<Run<?, ?>, ? extends Collector> jobFailedCount;
-    private MetricCollector<Run<?, ?>, ? extends Collector> jobAbortedCount;
-    private MetricCollector<Run<?, ?>, ? extends Collector> jobUnstableCount;
-    private MetricCollector<Run<?, ?>, ? extends Collector> jobTotalCount;
     private MetricCollector<Job<?, ?>, ? extends Collector> jobHealthScoreGauge;
     private MetricCollector<Job<?, ?>, ? extends Collector> nbBuildsGauge;
     private MetricCollector<Job<?, ?>, ? extends Collector> buildDiscardGauge;
@@ -79,30 +72,13 @@ public JobCollector() {}
 
     @Override
     public List<MetricFamilySamples> collect() {
-        logger.debug("Collecting metrics for prometheus");
+        LOGGER.debug("Collecting metrics for prometheus");
 
         CollectorFactory factory = new CollectorFactory();
         List<MetricFamilySamples> samples = new ArrayList<>();
-        String jobAttribute = PrometheusConfiguration.get().getJobAttributeName();
-
-        String[] labelBaseNameArray = {jobAttribute, "repo", "buildable"};
-
-        String[] labelNameArray = labelBaseNameArray;
-        if (PrometheusConfiguration.get().isAppendParamLabel()) {
-            labelNameArray = Arrays.copyOf(labelNameArray, labelNameArray.length + 1);
-            labelNameArray[labelNameArray.length - 1] = "parameters";
-        }
-        if (PrometheusConfiguration.get().isAppendStatusLabel()) {
-            labelNameArray = Arrays.copyOf(labelNameArray, labelNameArray.length + 1);
-            labelNameArray[labelNameArray.length - 1] = "status";
-        }
-
-        String[] buildParameterNamesAsArray = PrometheusConfiguration.get().getLabeledBuildParameterNamesAsArray();
-        for (String buildParam : buildParameterNamesAsArray) {
-            labelNameArray = Arrays.copyOf(labelNameArray, labelNameArray.length + 1);
-            labelNameArray[labelNameArray.length - 1] = buildParam.trim();
-        }
 
+        String[] labelBaseNameArray = JobLabel.getBaseLabelNames();
+        String[] labelNameArray = JobLabel.getJobLabelNames();
 
         boolean processDisabledJobs = PrometheusConfiguration.get().isProcessingDisabledBuilds();
         boolean ignoreBuildMetrics =
@@ -123,17 +99,17 @@ public List<MetricFamilySamples> collect() {
 
         // Counter manager acts as a DB to retrieve any counters that are already in memory instead of reinitializing
         // them with each iteration of collect.
-        var manager = CounterManager.getManager();       
-        jobSuccessCount = manager.getCounter(CollectorType.BUILD_SUCCESSFUL_COUNTER, labelBaseNameArray, null);
-        jobFailedCount = manager.getCounter(CollectorType.BUILD_FAILED_COUNTER, labelBaseNameArray, null);
-        jobTotalCount = manager.getCounter(CollectorType.BUILD_TOTAL_COUNTER, labelBaseNameArray, null);
-        jobAbortedCount = manager.getCounter(CollectorType.BUILD_ABORTED_COUNTER, labelBaseNameArray, null);
-        jobUnstableCount = manager.getCounter(CollectorType.BUILD_UNSTABLE_COUNTER, labelBaseNameArray, null);
+        var manager = CounterManager.getManager();
+        MetricCollector<Run<?, ?>, ? extends Collector> jobSuccessCount = manager.getCounter(CollectorType.BUILD_SUCCESSFUL_COUNTER, labelBaseNameArray, null);
+        MetricCollector<Run<?, ?>, ? extends Collector> jobFailedCount = manager.getCounter(CollectorType.BUILD_FAILED_COUNTER, labelBaseNameArray, null);
+        MetricCollector<Run<?, ?>, ? extends Collector> jobTotalCount = manager.getCounter(CollectorType.BUILD_TOTAL_COUNTER, labelBaseNameArray, null);
+        MetricCollector<Run<?, ?>, ? extends Collector> jobAbortedCount = manager.getCounter(CollectorType.BUILD_ABORTED_COUNTER, labelBaseNameArray, null);
+        MetricCollector<Run<?, ?>, ? extends Collector> jobUnstableCount = manager.getCounter(CollectorType.BUILD_UNSTABLE_COUNTER, labelBaseNameArray, null);
 
         // This is a try with resources block it ensures close is called
         // so if an exception occurs we don't reach deadlock. This is analogous to a using
         // block where dispose is called after we leave the block.
-        // The closeable iterator synchronizes receiving jobs and reading the iterator
+        // The closeable iterator synchronizes receiving jobs and reading the iterator,
         // so we don't modify the collection while iterating.
         try (CloseableIterator<Run<?,?>> iterator = listener.iterator()) {
             // Go through each run received since the last scrape.
@@ -177,17 +153,17 @@ public List<MetricFamilySamples> collect() {
         Jobs.forEachJob(job -> {
             try {
                 if (!job.isBuildable() && processDisabledJobs) {
-                    logger.debug("job [{}] is disabled", job.getFullName());
+                    LOGGER.debug("job [{}] is disabled", job.getFullName());
                     return;
                 }
-                logger.debug("Collecting metrics for job [{}]", job.getFullName());
+                LOGGER.debug("Collecting metrics for job [{}]", job.getFullName());
                 appendJobMetrics(job);
             } catch (IllegalArgumentException e) {
                 if (!e.getMessage().contains("Incorrect number of labels")) {
-                    logger.warn("Caught error when processing job [{}] error: ", job.getFullName(), e);
+                    LOGGER.warn("Caught error when processing job [{}] error: ", job.getFullName(), e);
                 } // else - ignore exception
             } catch (Exception e) {
-                logger.warn("Caught error when processing job [{}] error: ", job.getFullName(), e);
+                LOGGER.warn("Caught error when processing job [{}] error: ", job.getFullName(), e);
             }
 
         });
@@ -215,7 +191,7 @@ private void addSamples(List<MetricFamilySamples> allSamples, List<MetricFamilyS
         for (MetricFamilySamples metricFamilySample : newSamples) {
             int sampleCount = metricFamilySample.samples.size();
             if (sampleCount > 0) {
-                logger.debug(logMessage, sampleCount, metricFamilySample.name);
+                LOGGER.debug(logMessage, sampleCount, metricFamilySample.name);
                 allSamples.addAll(newSamples);
             }
         }
@@ -240,7 +216,7 @@ protected void appendJobMetrics(Job<?, ?> job) {
         Run<?, ?> lastBuild = job.getLastBuild();
         // Never built
         if (null == lastBuild) {
-            logger.debug("job [{}] never built", job.getFullName());
+            LOGGER.debug("job [{}] never built", job.getFullName());
             return;
         }
 
@@ -254,10 +230,10 @@ protected void appendJobMetrics(Job<?, ?> job) {
 
         Run<?, ?> run = lastBuild;
         while (run != null) {
-            logger.debug("getting metrics for run [{}] from job [{}], include per run metrics [{}]", run.getNumber(), job.getName(), isPerBuildMetrics);
+            LOGGER.debug("getting metrics for run [{}] from job [{}], include per run metrics [{}]", run.getNumber(), job.getName(), isPerBuildMetrics);
             if (Runs.includeBuildInMetrics(run)) {
-                logger.debug("getting build info for run [{}] from job [{}]", run.getNumber(), job.getName());
-                String[] labelValueArray = JobLabel.getJobLabelVaues(job, run);
+                LOGGER.debug("getting build info for run [{}] from job [{}]", run.getNumber(), job.getName());
+                String[] labelValueArray = JobLabel.getJobLabelValues(job, run);
 
                 summary.calculateMetric(run, labelValueArray);
                 if (isPerBuildMetrics) {
@@ -272,7 +248,7 @@ protected void appendJobMetrics(Job<?, ?> job) {
     }
 
     private void processRun(Job<?, ?> job, Run<?, ?> run, String[] buildLabelValueArray, BuildMetrics buildMetrics) {
-        logger.debug("Processing run [{}] from job [{}]", run.getNumber(), job.getName());
+        LOGGER.debug("Processing run [{}] from job [{}]", run.getNumber(), job.getName());
         buildMetrics.jobBuildResultOrdinal.calculateMetric(run, buildLabelValueArray);
         buildMetrics.jobBuildResult.calculateMetric(run, buildLabelValueArray);
         buildMetrics.jobBuildStartMillis.calculateMetric(run, buildLabelValueArray);

src/main/java/org/jenkinsci/plugins/prometheus/collectors/BaseMetricCollector.java

@@ -13,7 +13,7 @@ public abstract class BaseMetricCollector<T, I extends SimpleCollector<?>> imple
     protected final String namespace;
     protected final String subsystem;
     protected final String namePrefix;
-    protected I collector;
+    protected final I collector;
 
     protected BaseMetricCollector(String[] labelNames, String namespace, String subsystem, String namePrefix) {
         this.labelNames = labelNames;
@@ -79,7 +79,7 @@ private boolean isBaseNameSet() {
     }
 
     private boolean isNamePrefixSet() {
-        return namePrefix != null && !"".equals(namePrefix);
+        return namePrefix != null && !namePrefix.isEmpty();
     }
 
     protected String getBaseName() {

src/main/java/org/jenkinsci/plugins/prometheus/collectors/builds/BuildAbortedCounter.java

@@ -4,7 +4,6 @@
 import hudson.model.Run;
 import io.prometheus.client.Counter;
 import io.prometheus.client.SimpleCollector;
-
 import org.jenkinsci.plugins.prometheus.collectors.CollectorType;
 
 public class BuildAbortedCounter extends BuildsMetricCollector<Run<?, ?>, Counter>  {

src/main/java/org/jenkinsci/plugins/prometheus/collectors/builds/BuildCompletionListener.java

@@ -1,17 +1,18 @@
 package org.jenkinsci.plugins.prometheus.collectors.builds;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.Extension;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.model.listeners.RunListener;
+
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
-import hudson.Extension;
-import hudson.model.Run;
-import hudson.model.TaskListener;
-import hudson.model.listeners.RunListener;
-
 /*
  * Listens to builds that have been completed and stores them in a list.
  * The JobCollector reads items in the list when it performs a scrape and 
@@ -23,10 +24,10 @@ public class BuildCompletionListener extends RunListener<Run<?,?>> {
     private static BuildCompletionListener _Listener;
 
     // Lock to synchronize iteration and adding to the collection
-    private Lock lock;
+    private final Lock lock;
 
     // Holds the list o runs in queue.
-    private List<Run<?,?>> runStack;
+    private final List<Run<?,?>> runStack;
 
     // Iterable that defines a close method (allows us to use try resource) block
     // in JobCollector.java
@@ -42,7 +43,7 @@ protected BuildCompletionListener(){
 
     /*
      * Extension tells Jenkins to register this class as a RunListener and to use
-     * this method in order to retrieve an instance of the class. It is a singleton
+     * this method in order to retrieve an instance of the class. It is a singleton,
      * so we can get the same reference registered in Jenkins in another class.
      */
     @Extension
@@ -56,7 +57,7 @@ public synchronized static BuildCompletionListener getInstance(){
     /*
      * Fires on completion of a job.
      */
-    public void onCompleted(Run<?,?> run, TaskListener listener){
+    public void onCompleted(Run<?,?> run, @NonNull TaskListener listener){
         push(run);
     }
 
@@ -83,17 +84,17 @@ private synchronized void push(Run<?,?> run){
     public synchronized CloseableIterator<Run<?,?>> iterator(){
         // acquire lock before iterating
         lock.lock();
-        return new CloseableIterator<Run<?,?>>() {
+        return new CloseableIterator<>() {
             // Get iterator from the list
-            private Iterator<Run<?,?>> iterator = runStack.iterator();
+            private final Iterator<Run<?, ?>> iterator = runStack.iterator();
 
             @Override
             public boolean hasNext() {
                 return iterator.hasNext();
             }
 
             @Override
-            public Run<?,?> next() {
+            public Run<?, ?> next() {
                 return iterator.next();
             }
 

src/main/java/org/jenkinsci/plugins/prometheus/collectors/builds/BuildFailedCounter.java

@@ -1,11 +1,9 @@
 package org.jenkinsci.plugins.prometheus.collectors.builds;
 
-import hudson.model.Job;
 import hudson.model.Result;
 import hudson.model.Run;
 import io.prometheus.client.Counter;
 import io.prometheus.client.SimpleCollector;
-
 import org.jenkinsci.plugins.prometheus.collectors.CollectorType;
 
 public class BuildFailedCounter extends BuildsMetricCollector<Run<?, ?>, Counter>  {

src/main/java/org/jenkinsci/plugins/prometheus/collectors/builds/BuildResultGauge.java

@@ -31,7 +31,7 @@ public void calculateMetric(Run<?, ?> jenkinsObject, String[] labelValues) {
          * _last_build_result _last_build_result_ordinal
          *
          * SUCCESS   0 true  - The build had no errors.
-         * UNSTABLE  1 true  - The build had some errors but they were not fatal. For example, some tests failed.
+         * UNSTABLE  1 true  - The build had some errors, but they were not fatal. For example, some tests failed.
          * FAILURE   2 false - The build had a fatal error.
          * NOT_BUILT 3 false - The module was not built.
          * ABORTED   4 false - The build was manually aborted.

src/main/java/org/jenkinsci/plugins/prometheus/collectors/builds/BuildSuccessfulCounter.java

@@ -4,7 +4,6 @@
 import hudson.model.Run;
 import io.prometheus.client.Counter;
 import io.prometheus.client.SimpleCollector;
-
 import org.jenkinsci.plugins.prometheus.collectors.CollectorType;
 
 public class BuildSuccessfulCounter extends BuildsMetricCollector<Run<?, ?>, Counter> {

src/main/java/org/jenkinsci/plugins/prometheus/collectors/builds/BuildTotalCounter.java

@@ -4,7 +4,6 @@
 import hudson.model.Run;
 import io.prometheus.client.Counter;
 import io.prometheus.client.SimpleCollector;
-
 import org.jenkinsci.plugins.prometheus.collectors.CollectorType;
 
 public class BuildTotalCounter extends BuildsMetricCollector<Run<?, ?>, Counter> {

src/main/java/org/jenkinsci/plugins/prometheus/collectors/builds/BuildUnstableCounter.java

@@ -4,7 +4,6 @@
 import hudson.model.Run;
 import io.prometheus.client.Counter;
 import io.prometheus.client.SimpleCollector;
-
 import org.jenkinsci.plugins.prometheus.collectors.CollectorType;
 
 public class BuildUnstableCounter extends BuildsMetricCollector<Run<?, ?>, Counter>  {

src/main/java/org/jenkinsci/plugins/prometheus/collectors/builds/CounterManager.java

@@ -1,33 +1,31 @@
 package org.jenkinsci.plugins.prometheus.collectors.builds;
 
-import java.util.Arrays;
-import java.util.HashMap;
-
+import hudson.model.Run;
+import io.prometheus.client.Collector;
 import org.jenkinsci.plugins.prometheus.collectors.CollectorType;
 import org.jenkinsci.plugins.prometheus.collectors.MetricCollector;
-import org.jenkinsci.plugins.prometheus.config.PrometheusConfiguration;
 import org.jenkinsci.plugins.prometheus.util.ConfigurationUtils;
 
-import hudson.model.Run;
-import io.prometheus.client.Collector;
+import java.util.Arrays;
+import java.util.HashMap;
 
 /*
  * This class acts as a database to keep track of counters and return an existing counter
  * if it has already been initialized. This class is necessary due to the way the plugin handles
  * configuration changes. Changing the plugins configuration can cause the labels of a metric
  * to change. This manager compares whether it has seen a counter with a specific label before
- * and returns an existing counter if it exists. Otherwise it will return a new counter initialized at zero.
+ * and returns an existing counter if it exists. Otherwise, it will return a new counter initialized at zero.
  */
 public class CounterManager {
     // Keeps track of Counters we have seen.
-    private HashMap<CounterEntry, MetricCollector<Run<?, ?>, ? extends Collector>> registeredCounters;
+    private final HashMap<CounterEntry, MetricCollector<Run<?, ?>, ? extends Collector>> registeredCounters;
 
     // Static singleton instance.
     private static CounterManager manager;
 
     // Initialize the map
     private CounterManager() {
-        registeredCounters = new HashMap<CounterEntry, MetricCollector<Run<?, ?>, ? extends Collector>>();
+        registeredCounters = new HashMap<>();
     }
 
     /*
@@ -49,7 +47,7 @@ private Boolean hasCounter(CounterEntry entry) {
     }
 
     /*
-     * Retrives a counter or initializes a new one if it doesn't exist
+     * Retrieves a counter or initializes a new one if it doesn't exist
      * @return Metric collector counter.
      */
     public MetricCollector<Run<?, ?>, ? extends Collector> getCounter(CollectorType type, String[]labels, String prefix){
@@ -74,16 +72,16 @@ private Boolean hasCounter(CounterEntry entry) {
      */
     private static class CounterEntry {
         // Labels that the counter was initialized with
-        private String[] labels;
+        private final String[] labels;
 
         // What collector type the counter is.
-        private CollectorType type;
+        private final CollectorType type;
 
         // Prefix of the counter.
-        private String prefix;
+        private final String prefix;
 
         // namespace of the counter
-        private String namespace;
+        private final String namespace;
 
         /*
          * Creates new counter entry
@@ -128,8 +126,7 @@ public int hashCode() {
             int typeHash = type != null ? type.hashCode() : 0;
             int prefixHash = prefix != null ? prefix.hashCode() : 0;
             int namespaceHash = namespace != null ? namespace.hashCode() : 0;
-            int result = 31 * (typeHash + Arrays.hashCode(labels) + prefixHash + namespaceHash);
-            return result;
+            return 31 * (typeHash + Arrays.hashCode(labels) + prefixHash + namespaceHash);
         }
     }
 }