prometheus / client_java

Prometheus instrumentation library for JVM applications
http://prometheus.github.io/client_java/
Apache License 2.0
2.17k stars 792 forks source link

Package com.sun.management used directly #478

Open MrEasy opened 5 years ago

MrEasy commented 5 years ago

Hi,

This issue is similar to https://github.com/prometheus/client_java/pull/281:

Class io.prometheus.client.hotspot.MemoryAllocationExports introduced with #434 imports classes from com.sun.management, which are not available with other vendors' VMs and Oracle's module system.

Instead, the same approach as with io.prometheus.client.hotspot.StandardExports should be used.

MrEasy commented 5 years ago

Following is a proposal - passes the unit test, but not tested furthermore.

From 6baad02f2c672cab4cea24db5b3de3a9eae2dc01 Thu, 25 Apr 2019 14:22:27 +0200
From: Rico Neubauer <R.Neubauer@seeburger.de>
Date: Thu, 25 Apr 2019 14:22:19 +0200
Subject: [PATCH] #478 Avoid direct usage of com.sun classes

diff --git a/simpleclient_hotspot/src/main/java/io/prometheus/client/hotspot/MemoryAllocationExports.java b/simpleclient_hotspot/src/main/java/io/prometheus/client/hotspot/MemoryAllocationExports.java
index afad259..2dbb410 100644
--- a/simpleclient_hotspot/src/main/java/io/prometheus/client/hotspot/MemoryAllocationExports.java
+++ b/simpleclient_hotspot/src/main/java/io/prometheus/client/hotspot/MemoryAllocationExports.java
@@ -1,7 +1,5 @@
 package io.prometheus.client.hotspot;

-import com.sun.management.GarbageCollectionNotificationInfo;
-import com.sun.management.GcInfo;
 import io.prometheus.client.Collector;
 import io.prometheus.client.Counter;

@@ -9,6 +7,7 @@
 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;
@@ -45,10 +44,12 @@

     @Override
     public synchronized void handleNotification(Notification notification, Object handback) {
-      GarbageCollectionNotificationInfo info = GarbageCollectionNotificationInfo.from((CompositeData) notification.getUserData());
-      GcInfo gcInfo = info.getGcInfo();
-      Map<String, MemoryUsage> memoryUsageBeforeGc = gcInfo.getMemoryUsageBeforeGc();
-      Map<String, MemoryUsage> memoryUsageAfterGc = gcInfo.getMemoryUsageAfterGc();
+      CompositeData userData = (CompositeData) notification.getUserData();
+      CompositeData gcInfo = (CompositeData) userData.get("gcInfo"); // equal to GarbageCollectionNotificationInfo info = GarbageCollectionNotificationInfo.from(userData).getGcInfo();
+
+      Map<String, MemoryUsage> memoryUsageBeforeGc = (Map<String, MemoryUsage>) gcInfo.get("memoryUsageBeforeGc"); // gcInfo.getMemoryUsageBeforeGc();
+      Map<String, MemoryUsage> memoryUsageAfterGc = (Map<String, MemoryUsage>) gcInfo.get("memoryUsageAfterGc");   // gcInfo.getMemoryUsageAfterGc();
+
       for (Map.Entry<String, MemoryUsage> entry : memoryUsageBeforeGc.entrySet()) {
         String memoryPool = entry.getKey();
         long before = entry.getValue().getUsed();
brian-brazil commented 5 years ago

We'd need to test it on all the relevant VMs, and ensure that it works if this information is missing.

MrEasy commented 5 years ago

Tested patch - not yet working, needs further adaptions.

brian-brazil commented 5 years ago

Did you get any further with this?

MrEasy commented 5 years ago

@brian-brazil No not really. Would need more time and love to make it fully work.