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

Fix not working garbage collection #811

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
@@ -1,5 +1,10 @@
package io.prometheus.client.hotspot;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import io.prometheus.client.Collector;
import io.prometheus.client.CollectorRegistry;

/**
Expand All @@ -18,6 +23,18 @@
public class DefaultExports {

private static boolean initialized = false;

private final static List<Collector> defaultCollectors = Collections.unmodifiableList(Arrays.asList(
new BufferPoolsExports(),
new ClassLoadingExports(),
new CompilationExports(),
new GarbageCollectorExports(),
new MemoryAllocationExports(),
new MemoryPoolsExports(),
new StandardExports(),
new ThreadExports(),
new VersionInfoExports()
));

/**
* Register the default Hotspot collectors with the default
Expand All @@ -30,19 +47,48 @@ public static synchronized void initialize() {
initialized = true;
}
}

/**
* Release the resources used by the default Hotspot
* collectors. It is safe to call this method multiple times,
* as this will unregister all collectors once.
*/
public static synchronized void terminate() {
if (initialized) {
unregister(CollectorRegistry.defaultRegistry);
initialized = false;
}
}

/**
* Register the default Hotspot collectors with the given registry.
*
* @param registry to which the collector is added. Null values ​​are
* not allowed.
*/
public static void register(CollectorRegistry registry) {
new BufferPoolsExports().register(registry);
new ClassLoadingExports().register(registry);
new CompilationExports().register(registry);
new GarbageCollectorExports().register(registry);
new MemoryAllocationExports().register(registry);
new MemoryPoolsExports().register(registry);
new StandardExports().register(registry);
new ThreadExports().register(registry);
new VersionInfoExports().register(registry);
for (Collector collector : defaultCollectors) {
collector.register(registry);

if (collector instanceof MemoryAllocationExports) {
((MemoryAllocationExports)collector).addGarbageCollectorListener();
}
}
}

/**
* Unregister the default Hotspot collectors from the given registry.
*
* @param registry from that the default collectors are removed. Null
* values ​​are not allowed.
*/
public static void unregister(CollectorRegistry registry) {
for (Collector collector : defaultCollectors) {
registry.unregister(collector);

if (collector instanceof MemoryAllocationExports) {
((MemoryAllocationExports)collector).removeGarbageCollectorListener();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,19 @@
import io.prometheus.client.Collector;
import io.prometheus.client.Counter;

import javax.management.ListenerNotFoundException;
import javax.management.Notification;
import javax.management.NotificationEmitter;
import javax.management.NotificationListener;
import javax.management.openmbean.CompositeData;
import java.lang.management.GarbageCollectorMXBean;
import java.lang.management.ManagementFactory;
import java.lang.management.MemoryUsage;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.logging.Logger;

public class MemoryAllocationExports extends Collector {
private final Counter allocatedCounter = Counter.build()
Expand All @@ -23,18 +26,45 @@ public class MemoryAllocationExports extends Collector {
.labelNames("pool")
.create();

private static final Logger LOGGER = Logger.getLogger(MemoryAllocationExports.class.getName());

private NotificationListener listener = null;

public MemoryAllocationExports() {
AllocationCountingNotificationListener listener = new AllocationCountingNotificationListener(allocatedCounter);
for (GarbageCollectorMXBean garbageCollectorMXBean : getGarbageCollectorMXBeans()) {
if (garbageCollectorMXBean instanceof NotificationEmitter) {
((NotificationEmitter) garbageCollectorMXBean).addNotificationListener(listener, null, null);
}
}
addGarbageCollectorListener();
}

@Override
public List<MetricFamilySamples> collect() {
return allocatedCounter.collect();
return null == listener ? new ArrayList<Collector.MetricFamilySamples>() : allocatedCounter.collect();
}

void removeGarbageCollectorListener() {
if (null != listener ) {
for (GarbageCollectorMXBean garbageCollectorMXBean : getGarbageCollectorMXBeans()) {
if (garbageCollectorMXBean instanceof NotificationEmitter) {
try {
((NotificationEmitter) garbageCollectorMXBean).removeNotificationListener(listener);
} catch (ListenerNotFoundException e) {
LOGGER.warning("The default notification listener could not be removed from the garbage collector.");
}
}
}

listener = null;
}
}

void addGarbageCollectorListener() {
if (null == listener) {
listener = new AllocationCountingNotificationListener(allocatedCounter);

for (GarbageCollectorMXBean garbageCollectorMXBean : getGarbageCollectorMXBeans()) {
if (garbageCollectorMXBean instanceof NotificationEmitter) {
((NotificationEmitter) garbageCollectorMXBean).addNotificationListener(listener, null, null);
}
}
}
}

static class AllocationCountingNotificationListener implements NotificationListener {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.prometheus.client.hotspot;

import io.prometheus.client.Collector.MetricFamilySamples;
import io.prometheus.client.Counter;
import org.junit.Test;
import org.mockito.Mockito;
Expand All @@ -11,6 +12,8 @@
import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.times;

public class MemoryAllocationExportsTest {

Expand Down Expand Up @@ -62,7 +65,26 @@ protected List<GarbageCollectorMXBean> getGarbageCollectorMXBeans() {
}
};

Mockito.verify((NotificationEmitter) notificationEmitterMXBean).addNotificationListener(Mockito.any(MemoryAllocationExports.AllocationCountingNotificationListener.class), Mockito.isNull(), Mockito.isNull());
Mockito.verify((NotificationEmitter) notificationEmitterMXBean).addNotificationListener(Mockito.any(MemoryAllocationExports.AllocationCountingNotificationListener.class), Mockito.nullable(NotificationFilter.class), Mockito.nullable(Object.class));
Mockito.verifyNoInteractions(notNotificationEmitterMXBean);
}

@Test
public void testEmptyMetricsWithRemovedNotificationEmitter() {
MemoryAllocationExports allocationExporter = new MemoryAllocationExports();
allocationExporter.removeGarbageCollectorListener();

List<MetricFamilySamples> metrics = allocationExporter.collect();

assertTrue("Metrics should be empty, if no notification emitter is registered", metrics.isEmpty());
}

@Test
public void testAddNotificationEmitterNotCalledMultipleTimesDuringInstantiation() {
MemoryAllocationExports allocationExporter = Mockito.mock(MemoryAllocationExports.class);

allocationExporter.addGarbageCollectorListener();

Mockito.verify(allocationExporter, times(1)).addGarbageCollectorListener();
}
}