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

Package com.sun.management used directly #478

Open
MrEasy opened this issue Apr 25, 2019 · 5 comments
Open

Package com.sun.management used directly #478

MrEasy opened this issue Apr 25, 2019 · 5 comments

Comments

@MrEasy
Copy link

MrEasy commented Apr 25, 2019

Hi,

This issue is similar to #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
Copy link
Author

MrEasy commented Apr 25, 2019

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();

MrEasy added a commit to seeburger-ag/client_java that referenced this issue Apr 25, 2019
@brian-brazil
Copy link
Contributor

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

@MrEasy
Copy link
Author

MrEasy commented Apr 25, 2019

Tested patch - not yet working, needs further adaptions.

@brian-brazil
Copy link
Contributor

Did you get any further with this?

@MrEasy
Copy link
Author

MrEasy commented Aug 8, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants