Do not create Collectors for each Job. (#692)

jenkinsci · Aug 23, 2024 · eaecaf6 · eaecaf6
1 parent 8dc56e9
commit eaecaf6
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 65 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/prometheus/CodeCoverageCollector.java b/src/main/java/org/jenkinsci/plugins/prometheus/CodeCoverageCollector.java
@@ -1,11 +1,9 @@
 package org.jenkinsci.plugins.prometheus;
 
-import hudson.model.Job;
 import hudson.model.Run;
 import io.jenkins.plugins.coverage.metrics.steps.CoverageBuildAction;
 import io.prometheus.client.Collector;
 import jenkins.model.Jenkins;
-import org.apache.commons.collections.CollectionUtils;
 import org.jenkinsci.plugins.prometheus.collectors.CollectorFactory;
 import org.jenkinsci.plugins.prometheus.collectors.CollectorType;
 import org.jenkinsci.plugins.prometheus.collectors.MetricCollector;
@@ -36,25 +34,12 @@ public List<MetricFamilySamples> collect() {
             return Collections.emptyList();
         }
 
-        List<List<MetricFamilySamples>> samples = new ArrayList<>();
-        Jobs.forEachJob(job -> CollectionUtils.addIgnoreNull(samples, collectCoverageMetricForJob(job)));
+        List<MetricCollector<Run<?,?>, ? extends Collector>> collectors = createCollectors();
 
-
-        return samples.stream().flatMap(Collection::stream).collect(Collectors.toList());
+        return collectCoverageMetricForJob(collectors);
     }
 
-    private List<MetricFamilySamples> collectCoverageMetricForJob(Job<?,?> job) {
-
-        Run<?,?> lastBuild = job.getLastBuild();
-        if (lastBuild == null || lastBuild.isBuilding()) {
-            return Collections.emptyList();
-        }
-
-        CoverageBuildAction coverageBuildAction = lastBuild.getAction(CoverageBuildAction.class);
-        if (coverageBuildAction == null) {
-            return Collections.emptyList();
-        }
-
+    private List<MetricCollector<Run<?, ?>, ? extends Collector>> createCollectors() {
         CollectorFactory factory = new CollectorFactory();
         List<MetricCollector<Run<?,?>, ? extends Collector>> collectors = new ArrayList<>();
 
@@ -85,13 +70,31 @@ private List<MetricFamilySamples> collectCoverageMetricForJob(Job<?,?> job) {
         collectors.add(factory.createCoverageRunCollector(CollectorType.COVERAGE_LINE_TOTAL, new String[]{jobAttributeName}));
         collectors.add(factory.createCoverageRunCollector(CollectorType.COVERAGE_LINE_PERCENT, new String[]{jobAttributeName}));
 
-        collectors.forEach(c -> c.calculateMetric(lastBuild, new String[]{job.getFullName()}));
+        return collectors;
+    }
+
+    private List<MetricFamilySamples> collectCoverageMetricForJob(List<MetricCollector<Run<?, ?>, ? extends Collector>> collectors) {
+
+        Jobs.forEachJob(job -> {
+            Run<?,?> lastBuild = job.getLastBuild();
+            if (lastBuild == null || lastBuild.isBuilding()) {
+                return;
+            }
+
+            CoverageBuildAction coverageBuildAction = lastBuild.getAction(CoverageBuildAction.class);
+            if (coverageBuildAction == null) {
+                return;
+            }
+
+            collectors.forEach(c -> c.calculateMetric(lastBuild, new String[]{job.getFullName()}));
+        });
 
         return collectors.stream()
                 .map(MetricCollector::collect)
                 .flatMap(Collection::stream)
                 .collect(Collectors.toList());
     }
+
     private boolean isCoveragePluginLoaded() {
         return Jenkins.get().getPlugin("coverage") != null;
     }

diff --git a/src/test/java/org/jenkinsci/plugins/prometheus/CodeCoverageCollectorTest.java b/src/test/java/org/jenkinsci/plugins/prometheus/CodeCoverageCollectorTest.java
@@ -59,52 +59,6 @@ void shouldNotProduceMetricsWhenItIsNotConfigured() {
         }
     }
 
-    @Test
-    void shouldNotProduceMetricsWhenJobIsBuilding() {
-        try (
-                MockedStatic<Jenkins> jenkinsStatic = mockStatic(Jenkins.class);
-                MockedStatic<PrometheusConfiguration> configurationStatic = mockStatic(PrometheusConfiguration.class);
-        ) {
-            jenkinsStatic.when(Jenkins::get).thenReturn(jenkins);
-            configurationStatic.when(PrometheusConfiguration::get).thenReturn(config);
-            Job jobUnderTest = mock(Job.class);
-            Run lastBuild = mock(Run.class);
-            when(lastBuild.isBuilding()).thenReturn(true);
-            when(jobUnderTest.getLastBuild()).thenReturn(lastBuild);
-            when(jenkins.getAllItems(Job.class)).thenReturn(List.of(jobUnderTest));
-            when(jenkins.getPlugin("coverage")).thenReturn(new Plugin.DummyImpl());
-            when(config.isCollectCodeCoverage()).thenReturn(true);
-
-            CodeCoverageCollector sut = new CodeCoverageCollector();
-
-            List<Collector.MetricFamilySamples> collect = sut.collect();
-            assertEquals(0, collect.size());
-        }
-    }
-
-    @Test
-    void shouldNotProduceMetricsWhenJobHasNoCoverageBuildAction() {
-        try (
-                MockedStatic<Jenkins> jenkinsStatic = mockStatic(Jenkins.class);
-                MockedStatic<PrometheusConfiguration> configurationStatic = mockStatic(PrometheusConfiguration.class);
-        ) {
-            jenkinsStatic.when(Jenkins::get).thenReturn(jenkins);
-            configurationStatic.when(PrometheusConfiguration::get).thenReturn(config);
-            Job jobUnderTest = mock(Job.class);
-            Run lastBuild = mock(Run.class);
-            when(lastBuild.isBuilding()).thenReturn(false);
-            when(jobUnderTest.getLastBuild()).thenReturn(lastBuild);
-            when(jenkins.getAllItems(Job.class)).thenReturn(List.of(jobUnderTest));
-            when(jenkins.getPlugin("coverage")).thenReturn(new Plugin.DummyImpl());
-            when(config.isCollectCodeCoverage()).thenReturn(true);
-
-            CodeCoverageCollector sut = new CodeCoverageCollector();
-
-            List<Collector.MetricFamilySamples> collect = sut.collect();
-            assertEquals(0, collect.size());
-        }
-    }
-
     @Test
     void shouldProduceMetricsWhenJobHasCoverageBuildAction() {
         try (