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.

Sign up for GitHub

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

Jump to bottom

[JENKINS-57909] JMX: VM and System metrics shouldn't be exported #48

Merged
merged 4 commits into from
Jul 3, 2019
Merged

[JENKINS-57909] JMX: VM and System metrics shouldn't be exported #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94cf272
Reformat
Vlatombe Jun 13, 2019
ecf45b7
[JENKINS-57908] Use io.jenkins as jmx domain
Vlatombe Jun 13, 2019
1cd4376
[JENKINS-57909] Exclude some metrics that don't make sense for JMX ex…
Vlatombe Jun 14, 2019
e9106eb
Add a control case
Vlatombe Jun 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/main/java/jenkins/metrics/api/Metrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Pattern;

import jenkins.metrics.impl.MetricsFilter;
import jenkins.metrics.util.HealthChecksThreadPool;
import jenkins.model.Jenkins;
Expand Down Expand Up @@ -113,6 +115,17 @@ public class Metrics extends Plugin {
*/
public static final Permission HEALTH_CHECK = new Permission(PERMISSIONS,
"HealthCheck", Messages._Metrics_HealthCheckPermission_Description(), Jenkins.ADMINISTER, PermissionScope.JENKINS);

/**
* JMX domain
*/
public static final String JMX_DOMAIN = "io.jenkins";

/**
* Metrics excluded from JMX export
*/
private static final Pattern JMX_EXCLUSIONS = Pattern.compile("^(vm|system)\\..*|.*\\.(5m|15m|1h|history)$");

/**
* Our logger.
*/
Expand Down Expand Up @@ -265,7 +278,11 @@ public static void reindexAccessKeys() {
@Override
public void start() throws Exception {
PluginServletFilter.addFilter(filter);
jmxReporter = JmxReporter.forRegistry(metricRegistry).build();
jmxReporter = JmxReporter
.forRegistry(metricRegistry)
.inDomain(JMX_DOMAIN)
.filter((name, metric) -> !JMX_EXCLUSIONS.matcher(name).matches())
.build();
jmxReporter.start();
}

Expand Down
46 changes: 32 additions & 14 deletions src/test/java/jenkins/metrics/api/MetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,25 @@

import hudson.util.VersionNumber;
import jenkins.model.Jenkins;
import org.apache.commons.lang.ArrayUtils;
import org.junit.AfterClass;
import org.junit.Assume;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.RestartableJenkinsRule;

import javax.management.MBeanServer;
import javax.management.ObjectName;
import java.io.IOException;
import java.lang.management.ManagementFactory;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.core.IsNot.not;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

Expand All @@ -36,22 +43,33 @@ public static void cleanProperty() throws IOException {
@Test
public void logsDirectory() {

// Lambda-ify when Java 8+
story.then(new RestartableJenkinsRule.Step() {
@Override
public void run(JenkinsRule j) throws Throwable {
Assume.assumeTrue(Jenkins.getVersion().isNewerThan(new VersionNumber("2.113"))); // JENKINS-50291 introduced in 2.114
story.then(j -> {
Assume.assumeTrue(Jenkins.getVersion().isNewerThan(new VersionNumber("2.113"))); // JENKINS-50291 introduced in 2.114

assertTrue(Metrics.HealthChecker.getLogFile(j.jenkins).getAbsolutePath().startsWith(j.getInstance().getRootDir().getAbsolutePath()));
System.setProperty("hudson.triggers.SafeTimerTask.logsTargetDir", folder.newFolder().getAbsolutePath());
}
assertTrue(Metrics.HealthChecker.getLogFile(j.jenkins).getAbsolutePath().startsWith(j.getInstance().getRootDir().getAbsolutePath()));
System.setProperty("hudson.triggers.SafeTimerTask.logsTargetDir", folder.newFolder().getAbsolutePath());
});

story.then(new RestartableJenkinsRule.Step() {
@Override
public void run(JenkinsRule j) throws Throwable {
assertFalse(Metrics.HealthChecker.getLogFile(j.jenkins).getAbsolutePath().startsWith(j.getInstance().getRootDir().getAbsolutePath()));
}
story.then(j -> assertFalse(Metrics.HealthChecker.getLogFile(j.jenkins).getAbsolutePath().startsWith(j.getInstance().getRootDir().getAbsolutePath())));
}

@Test
public void jmxDomain() {
story.then(r -> {
assertTrue(ArrayUtils.contains(ManagementFactory.getPlatformMBeanServer().getDomains(), "io.jenkins"));
});
}

@Test
public void jmxMetricsExcluded() {
story.then(r -> {
MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer();
assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:name=gc.*"), null), is(empty()));
assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:name=vm.*"), null), is(empty()));
assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:name=*history"), null), is(empty()));
assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:name=*5m"), null), is(empty()));
assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:name=*1h"), null), is(empty()));

This comment was marked as resolved.

Sign in to view

assertThat(mBeanServer.queryNames(new ObjectName("io.jenkins:name=jenkins.*"), null), is(not(empty())));
});
}
}