Guard against InaccessibleObjectException in ExecutorServiceMetrics (#…

…2452)

This is an issue with our current implementation for getting at the wrapped ThreadPoolExecutor in some types returned by Executors, when --illegal-access=deny is set. This is the default from Java 16. Without these changes, an uncaught exception is thrown when trying to perform the reflective access on binding ExecutorServiceMetrics with one of the private type ExecutorService in Executors.

With these changes, the exception will be caught and logged. We end up catching all RuntimeExceptions since we cannot use the InaccessibleObjectException type introduced in Java 9, but this is probably fine in this implementation anyways.

Resolves gh-2447

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jvm/ExecutorServiceMetrics.java

@@ -301,8 +301,10 @@ private ThreadPoolExecutor unwrapThreadPoolExecutor(ExecutorService executor, Cl
             Field e = wrapper.getDeclaredField("e");
             e.setAccessible(true);
             return (ThreadPoolExecutor) e.get(executor);
-        } catch (NoSuchFieldException | IllegalAccessException e) {
+        } catch (NoSuchFieldException | IllegalAccessException | RuntimeException e) {
+            // Cannot use InaccessibleObjectException since it was introduced in Java 9, so catch all RuntimeExceptions instead
             // Do nothing. We simply can't get to the underlying ThreadPoolExecutor.
+            log.info("Cannot unwrap ThreadPoolExecutor for monitoring from {} due to {}: {}", wrapper.getName(), e.getClass().getName(), e.getMessage());
         }
         return null;
     }

micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jvm/ExecutorServiceMetricsTest.java

@@ -15,19 +15,23 @@
  */
 package io.micrometer.core.instrument.binder.jvm;
 
+import io.micrometer.core.Issue;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.MockClock;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
 import io.micrometer.core.instrument.simple.SimpleConfig;
 import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
 import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledForJreRange;
+import org.junit.jupiter.api.condition.JRE;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 
 import java.util.concurrent.*;
 
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.*;
 
 /**
  * Tests for {@link ExecutorServiceMetrics}.
@@ -106,6 +110,7 @@ void scheduledThreadPoolExecutorAsExecutorService(String metricPrefix, String ex
     }
 
     @DisplayName("ExecutorService can be monitored with a default set of metrics")
+    @DisabledForJreRange(min = JRE.JAVA_16, disabledReason = "See gh-2317 for why we can't run this full test on Java 16+")
     @ParameterizedTest
     @CsvSource({ "custom,custom.", "custom.,custom.", ",''", "' ',''" })
     void monitorExecutorService(String metricPrefix, String expectedMetricPrefix) throws InterruptedException {
@@ -137,6 +142,14 @@ void monitorExecutorService(String metricPrefix, String expectedMetricPrefix) th
         assertThat(registry.get(expectedMetricPrefix + "executor.queued").tags(userTags).gauge().value()).isEqualTo(0.0);
     }
 
+    @DisplayName("No exception thrown trying to monitor Executors private class")
+    @Test
+    @Issue("#2447") // Note: only reproduces on Java 16+ or with --illegal-access=deny
+    void monitorExecutorsExecutorServicePrivateClass() {
+        assertThatCode(() -> ExecutorServiceMetrics.monitor(registry, Executors.newSingleThreadExecutor(), ""))
+                .doesNotThrowAnyException();
+    }
+
     @DisplayName("ScheduledExecutorService can be monitored with a default set of metrics")
     @ParameterizedTest
     @CsvSource({ "custom,custom.", "custom.,custom.", ",''", "' ',''" })