Fixes #635: Changing creation of service classes away from explicit g…

…uice usage to avoid duplicate instantiation of DefaultPrometheusMetrics class.
jenkinsci · Mar 22, 2024 · 4615bf3 · 4615bf3
1 parent 1e8361e
commit 4615bf3
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 117 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/prometheus/context/Context.java b/src/main/java/org/jenkinsci/plugins/prometheus/context/Context.java
diff --git a/src/main/java/org/jenkinsci/plugins/prometheus/rest/PrometheusAction.java b/src/main/java/org/jenkinsci/plugins/prometheus/rest/PrometheusAction.java
@@ -1,7 +1,7 @@
 package org.jenkinsci.plugins.prometheus.rest;
 
-import com.google.inject.Inject;
 import hudson.Extension;
+import hudson.ExtensionList;
 import hudson.model.UnprotectedRootAction;
 import hudson.util.HttpResponses;
 import io.prometheus.client.exporter.common.TextFormat;
@@ -15,14 +15,7 @@
 
 @Extension
 public class PrometheusAction implements UnprotectedRootAction {
-
-    private PrometheusMetrics prometheusMetrics;
-
-    @Inject
-    public void setPrometheusMetrics(PrometheusMetrics prometheusMetrics) {
-        this.prometheusMetrics = prometheusMetrics;
-    }
-
+
     @Override
     public String getIconFileName() {
         return null;
@@ -56,6 +49,7 @@ private boolean hasAccess() {
     }
 
     private HttpResponse prometheusResponse() {
+        PrometheusMetrics prometheusMetrics = ExtensionList.lookupSingleton(PrometheusMetrics.class);
         return (request, response, node) -> {
             response.setStatus(StaplerResponse.SC_OK);
             response.setContentType(TextFormat.CONTENT_TYPE_004);

diff --git a/src/main/java/org/jenkinsci/plugins/prometheus/service/DefaultPrometheusMetrics.java b/src/main/java/org/jenkinsci/plugins/prometheus/service/DefaultPrometheusMetrics.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.prometheus.service;
 
+import hudson.Extension;
 import hudson.ExtensionList;
 import hudson.init.InitMilestone;
 import hudson.init.Initializer;
@@ -8,26 +9,24 @@
 import io.prometheus.client.dropwizard.DropwizardExports;
 import io.prometheus.client.exporter.common.TextFormat;
 import io.prometheus.client.hotspot.DefaultExports;
-import java.io.IOException;
-import java.io.StringWriter;
-import java.util.concurrent.atomic.AtomicReference;
 import jenkins.metrics.api.Metrics;
-import org.jenkinsci.plugins.prometheus.CodeCoverageCollector;
-import org.jenkinsci.plugins.prometheus.DiskUsageCollector;
-import org.jenkinsci.plugins.prometheus.ExecutorCollector;
-import org.jenkinsci.plugins.prometheus.JenkinsStatusCollector;
-import org.jenkinsci.plugins.prometheus.JobCollector;
+import org.jenkinsci.plugins.prometheus.*;
 import org.jenkinsci.plugins.prometheus.config.disabledmetrics.FilteredMetricEnumeration;
 import org.jenkinsci.plugins.prometheus.util.JenkinsNodeBuildsSampleBuilder;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
+import java.io.StringWriter;
+import java.util.concurrent.atomic.AtomicReference;
+
 @Restricted(NoExternalUse.class)
+@Extension
 public class DefaultPrometheusMetrics implements PrometheusMetrics {
 
-    private static final Logger logger = LoggerFactory.getLogger(DefaultPrometheusMetrics.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(DefaultPrometheusMetrics.class);
 
     private volatile boolean initialized = false;
     private volatile boolean initializing = false;
@@ -40,10 +39,10 @@ public DefaultPrometheusMetrics() {
     }
 
     @Initializer(after = InitMilestone.JOB_LOADED)
-    public void init() {
+    public void registerCollectors() {
         if (!initialized && !initializing) {
             initializing = true;
-            logger.debug("Initializing...");
+            LOGGER.debug("Initializing...");
             collectorRegistry.register(new JobCollector());
             collectorRegistry.register(new JenkinsStatusCollector());
             collectorRegistry.register(
@@ -67,14 +66,14 @@ public String getMetrics() {
     @Override
     public void collectMetrics() {
         if(!initialized) {
-            logger.debug("Not initialized");
+            LOGGER.debug("Not initialized");
             return;
         }
         try (StringWriter buffer = new StringWriter()) {
             TextFormat.write004(buffer, new FilteredMetricEnumeration(collectorRegistry.metricFamilySamples().asIterator()));
             cachedMetrics.set(buffer.toString());
         } catch (IOException e) {
-            logger.debug("Unable to collect metrics");
+            LOGGER.debug("Unable to collect metrics");
         }
     }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/prometheus/service/PrometheusAsyncWorker.java b/src/main/java/org/jenkinsci/plugins/prometheus/service/PrometheusAsyncWorker.java
@@ -1,7 +1,7 @@
 package org.jenkinsci.plugins.prometheus.service;
 
-import com.google.inject.Inject;
 import hudson.Extension;
+import hudson.ExtensionList;
 import hudson.model.AsyncPeriodicWork;
 import hudson.model.TaskListener;
 import org.jenkinsci.plugins.prometheus.config.PrometheusConfiguration;
@@ -16,16 +16,11 @@ public class PrometheusAsyncWorker extends AsyncPeriodicWork {
 
     private static final Logger logger = LoggerFactory.getLogger(PrometheusAsyncWorker.class);
 
-    private PrometheusMetrics prometheusMetrics;
 
     public PrometheusAsyncWorker() {
         super("prometheus_async_worker");
     }
 
-    @Inject
-    public void setPrometheusMetrics(PrometheusMetrics prometheusMetrics) {
-        this.prometheusMetrics = prometheusMetrics;
-    }
 
     @Override
     public long getRecurrencePeriod() {
@@ -38,6 +33,7 @@ public long getRecurrencePeriod() {
     @Override
     public void execute(TaskListener taskListener) {
         logger.debug("Collecting prometheus metrics");
+        PrometheusMetrics prometheusMetrics = ExtensionList.lookupSingleton(PrometheusMetrics.class);
         prometheusMetrics.collectMetrics();
         logger.debug("Prometheus metrics collected successfully");
     }

diff --git a/src/main/java/org/jenkinsci/plugins/prometheus/service/PrometheusMetrics.java b/src/main/java/org/jenkinsci/plugins/prometheus/service/PrometheusMetrics.java
@@ -1,6 +1,8 @@
 package org.jenkinsci.plugins.prometheus.service;
 
-public interface PrometheusMetrics {
+import hudson.ExtensionPoint;
+
+public interface PrometheusMetrics extends ExtensionPoint {
 
     String getMetrics();
 

diff --git a/src/test/java/org/jenkinsci/plugins/prometheus/context/ContextTest.java b/src/test/java/org/jenkinsci/plugins/prometheus/context/ContextTest.java
diff --git a/src/test/java/org/jenkinsci/plugins/prometheus/rest/PrometheusActionTest.java b/src/test/java/org/jenkinsci/plugins/prometheus/rest/PrometheusActionTest.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.prometheus.rest;
 
+import hudson.ExtensionList;
 import io.prometheus.client.exporter.common.TextFormat;
 import jenkins.metrics.api.Metrics;
 import jenkins.model.Jenkins;
@@ -91,21 +92,24 @@ public void shouldReturnMetrics() throws IOException, ServletException {
         PrometheusMetrics prometheusMetrics = mock(PrometheusMetrics.class);
         String responseBody = "testMetric";
         when(prometheusMetrics.getMetrics()).thenReturn(responseBody);
-        action.setPrometheusMetrics(prometheusMetrics);
-        StaplerRequest request = mock(StaplerRequest.class);
-        String url = "prometheus";
-        when(request.getRestOfPath()).thenReturn(url);
-
-        // when
-        HttpResponse actual = action.doDynamic(request);
+        try (MockedStatic<ExtensionList> extensionListMockedStatic = mockStatic(ExtensionList.class)) {
+            extensionListMockedStatic.when(() -> ExtensionList.lookupSingleton(PrometheusMetrics.class)).thenReturn(prometheusMetrics);
+            StaplerRequest request = mock(StaplerRequest.class);
+            String url = "prometheus";
+            when(request.getRestOfPath()).thenReturn(url);
+
+            // when
+            HttpResponse actual = action.doDynamic(request);
+
+            // then
+            AssertStaplerResponse.from(actual)
+                    .call()
+                    .assertHttpStatus(HTTP_OK)
+                    .assertContentType(TextFormat.CONTENT_TYPE_004)
+                    .assertHttpHeader("Cache-Control", "must-revalidate,no-cache,no-store")
+                    .assertBody(responseBody);
+        }
 
-        // then
-        AssertStaplerResponse.from(actual)
-            .call()
-            .assertHttpStatus(HTTP_OK)
-            .assertContentType(TextFormat.CONTENT_TYPE_004)
-            .assertHttpHeader("Cache-Control", "must-revalidate,no-cache,no-store")
-            .assertBody(responseBody);
     }
 
     private static class AssertStaplerResponse {

diff --git a/src/test/java/org/jenkinsci/plugins/prometheus/service/PrometheusAsyncWorkerTest.java b/src/test/java/org/jenkinsci/plugins/prometheus/service/PrometheusAsyncWorkerTest.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.prometheus.service;
 
+import hudson.ExtensionList;
 import org.jenkinsci.plugins.prometheus.config.PrometheusConfiguration;
 import org.junit.jupiter.api.Test;
 import org.jvnet.hudson.test.Issue;
@@ -20,15 +21,15 @@ public void shouldCollectMetrics() {
         // given
         PrometheusAsyncWorker asyncWorker = new PrometheusAsyncWorker();
         PrometheusMetrics metrics = new TestPrometheusMetrics();
-        asyncWorker.setPrometheusMetrics(metrics);
-
-        // when
-        asyncWorker.execute(null);
-
-        // then
-        String actual = metrics.getMetrics();
-        assertEquals("1", actual);
+        try (MockedStatic<ExtensionList> extensionListMockedStatic = mockStatic(ExtensionList.class)) {
+            extensionListMockedStatic.when(() -> ExtensionList.lookupSingleton(PrometheusMetrics.class)).thenReturn(metrics);
+            // when
+            asyncWorker.execute(null);
 
+            // then
+            String actual = metrics.getMetrics();
+            assertEquals("1", actual);
+        }
     }
     @Test
     public void testConvertSecondsToMillis() {