Skip to content

Commit

Permalink
Revert "Fixes #635: Changing creation of service classes away from ex…
Browse files Browse the repository at this point in the history
…plicit guice usage to avoid duplicate instantiation of DefaultPrometheusMetrics class. (#644)"

This reverts commit 9401c6e.
  • Loading branch information
waschndolos committed Mar 28, 2024
1 parent 0c3c655 commit 0943cc1
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 47 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.jenkinsci.plugins.prometheus.context;

import com.google.inject.AbstractModule;
import hudson.Extension;
import org.jenkinsci.plugins.prometheus.service.DefaultPrometheusMetrics;
import org.jenkinsci.plugins.prometheus.service.PrometheusMetrics;

@Extension
public class Context extends AbstractModule {

@Override
public void configure() {
bind(PrometheusMetrics.class).to(DefaultPrometheusMetrics.class).in(com.google.inject.Singleton.class);
}

}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -15,7 +15,14 @@

@Extension
public class PrometheusAction implements UnprotectedRootAction {


private PrometheusMetrics prometheusMetrics;

@Inject
public void setPrometheusMetrics(PrometheusMetrics prometheusMetrics) {
this.prometheusMetrics = prometheusMetrics;
}

@Override
public String getIconFileName() {
return null;
Expand Down Expand Up @@ -49,7 +56,6 @@ 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.jenkinsci.plugins.prometheus.service;

import hudson.Extension;
import hudson.ExtensionList;
import hudson.init.InitMilestone;
import hudson.init.Initializer;
Expand All @@ -9,24 +8,26 @@
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.*;
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.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;
Expand All @@ -39,10 +40,10 @@ public DefaultPrometheusMetrics() {
}

@Initializer(after = InitMilestone.JOB_LOADED)
public void registerCollectors() {
public void init() {
if (!initialized && !initializing) {
initializing = true;
LOGGER.debug("Initializing...");
logger.debug("Initializing...");
collectorRegistry.register(new JobCollector());
collectorRegistry.register(new JenkinsStatusCollector());
collectorRegistry.register(
Expand All @@ -66,14 +67,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");
}
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -16,11 +16,16 @@ 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() {
Expand All @@ -33,7 +38,6 @@ 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");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package org.jenkinsci.plugins.prometheus.service;

import hudson.ExtensionPoint;

public interface PrometheusMetrics extends ExtensionPoint {
public interface PrometheusMetrics {

String getMetrics();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package org.jenkinsci.plugins.prometheus.context;

import com.codahale.metrics.MetricRegistry;
import com.google.inject.Guice;
import com.google.inject.Injector;
import jenkins.metrics.api.Metrics;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.prometheus.config.PrometheusConfiguration;
import org.jenkinsci.plugins.prometheus.service.DefaultPrometheusMetrics;
import org.jenkinsci.plugins.prometheus.service.PrometheusMetrics;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.MockedStatic;

import static org.mockito.Mockito.*;

public class ContextTest {

@Test
public void testCanInjectContext() {
try (MockedStatic<Jenkins> jenkinsStatic = mockStatic(Jenkins.class);
MockedStatic<PrometheusConfiguration> prometheusConfigurationStatic = mockStatic(PrometheusConfiguration.class);
MockedStatic<Metrics> metricsStatic = mockStatic(Metrics.class)) {
Jenkins mockedJenkins = mock(Jenkins.class);
jenkinsStatic.when(Jenkins::get).thenReturn(mockedJenkins);

MetricRegistry mockedMetrics = mock(MetricRegistry.class);
metricsStatic.when(Metrics::metricRegistry).thenReturn(mockedMetrics);

PrometheusConfiguration mockedPrometheusConfiguration = mock(PrometheusConfiguration.class);
when(mockedPrometheusConfiguration.getLabeledBuildParameterNamesAsArray()).thenReturn(new String[]{});
when(mockedPrometheusConfiguration.getDefaultNamespace()).thenReturn("default");
prometheusConfigurationStatic.when(PrometheusConfiguration::get).thenReturn(mockedPrometheusConfiguration);

Injector injector = Guice.createInjector(new Context());
PrometheusMetrics prometheusMetrics = injector.getInstance(PrometheusMetrics.class);

Assertions.assertNotNull(prometheusMetrics);
Assertions.assertEquals(DefaultPrometheusMetrics.class, prometheusMetrics.getClass());

PrometheusMetrics prometheusMetrics2 = injector.getInstance(PrometheusMetrics.class);

Assertions.assertEquals(prometheusMetrics, prometheusMetrics2, "Should be the same as it's a singleton!");


}

}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
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;
Expand Down Expand Up @@ -92,24 +91,21 @@ public void shouldReturnMetrics() throws IOException, ServletException {
PrometheusMetrics prometheusMetrics = mock(PrometheusMetrics.class);
String responseBody = "testMetric";
when(prometheusMetrics.getMetrics()).thenReturn(responseBody);
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);
}
action.setPrometheusMetrics(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);
}

private static class AssertStaplerResponse {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
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;
Expand All @@ -21,15 +20,15 @@ public void shouldCollectMetrics() {
// given
PrometheusAsyncWorker asyncWorker = new PrometheusAsyncWorker();
PrometheusMetrics metrics = new TestPrometheusMetrics();
try (MockedStatic<ExtensionList> extensionListMockedStatic = mockStatic(ExtensionList.class)) {
extensionListMockedStatic.when(() -> ExtensionList.lookupSingleton(PrometheusMetrics.class)).thenReturn(metrics);
// when
asyncWorker.execute(null);
asyncWorker.setPrometheusMetrics(metrics);

// when
asyncWorker.execute(null);

// then
String actual = metrics.getMetrics();
assertEquals("1", actual);

// then
String actual = metrics.getMetrics();
assertEquals("1", actual);
}
}
@Test
public void testConvertSecondsToMillis() {
Expand Down

0 comments on commit 0943cc1

Please sign in to comment.