Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverting commits which might cause startup issues on non docker instances #650

Merged
merged 2 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit use of Guice in Jenkins plugins is almost always a bad idea. Whatever the issue was here (reproducible?), this was probably not the right way to solve it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this pr is quite old, this class no longer exists in current versions

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,9 +1,6 @@
package org.jenkinsci.plugins.prometheus.service;

import hudson.Extension;
import hudson.ExtensionList;
import hudson.init.InitMilestone;
import hudson.init.Initializer;
import io.prometheus.client.Collector;
import io.prometheus.client.CollectorRegistry;
import io.prometheus.client.dropwizard.DropwizardExports;
Expand All @@ -13,49 +10,36 @@
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;
private final CollectorRegistry collectorRegistry;
private final AtomicReference<String> cachedMetrics;

public DefaultPrometheusMetrics() {
this.collectorRegistry = CollectorRegistry.defaultRegistry;
this.cachedMetrics = new AtomicReference<>("");
}
CollectorRegistry collectorRegistry = CollectorRegistry.defaultRegistry;
collectorRegistry.register(new JobCollector());
collectorRegistry.register(new JenkinsStatusCollector());
collectorRegistry.register(new DropwizardExports(Metrics.metricRegistry(), new JenkinsNodeBuildsSampleBuilder()));
collectorRegistry.register(new DiskUsageCollector());
collectorRegistry.register(new ExecutorCollector());
collectorRegistry.register(new CodeCoverageCollector());

@Initializer(after = InitMilestone.JOB_LOADED)
public void registerCollectors() {
if (!initialized && !initializing) {
initializing = true;
LOGGER.debug("Initializing...");
collectorRegistry.register(new JobCollector());
collectorRegistry.register(new JenkinsStatusCollector());
collectorRegistry.register(
new DropwizardExports(Metrics.metricRegistry(), new JenkinsNodeBuildsSampleBuilder()));
collectorRegistry.register(new DiskUsageCollector());
collectorRegistry.register(new ExecutorCollector());
collectorRegistry.register(new CodeCoverageCollector());
// other collectors from other plugins
ExtensionList.lookup(Collector.class).forEach(collectorRegistry::register);
DefaultExports.initialize();
initialized = true;
initializing = false;
}
// other collectors from other plugins
ExtensionList.lookup(Collector.class).forEach(collectorRegistry::register);

DefaultExports.initialize();

this.collectorRegistry = collectorRegistry;
this.cachedMetrics = new AtomicReference<>("");
}

@Override
Expand All @@ -65,15 +49,11 @@

@Override
public void collectMetrics() {
if(!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");

Check warning on line 56 in src/main/java/org/jenkinsci/plugins/prometheus/service/DefaultPrometheusMetrics.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 56 is not covered by tests
}
}
}
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
Loading