Merge pull request #120 from xenit-eu/master

Release 0.7.1

Author: kerkhofsd

CHANGELOG.md

@@ -21,6 +21,12 @@ Version template:
  -->
 
 # Alfred Telemetry Changelog
+
+## [0.7.1] - 2021-07-12
+
+### Fixed
+* Hazelcast cache metrics broken causing, among other things, a broken Prometheus scrape endpoint [#116]
+
 ## [0.7.0] - 2021-07-09
 
 ### Added

alfred-telemetry-platform/build.gradle

@@ -32,7 +32,10 @@ dependencies {
 
     // 'alfresco-enterprise-repository' not transitive because it will try to download a bunch of unreachable artifacts
     alfrescoProvided('org.alfresco:alfresco-enterprise-repository') { transitive = false }
-    alfrescoProvided('com.hazelcast:hazelcast') { transitive = false }
+    alfrescoProvided('com.hazelcast:hazelcast:2.4') {
+        force = true
+        transitive = false
+    }
 
     implementation("io.micrometer:micrometer-core:${micrometerVersion}") {
         exclude group: "org.slf4j", module: "*"

alfred-telemetry-platform/src/main/java/eu/xenit/alfred/telemetry/binder/cache/Hazelcast2CacheMetrics.java

@@ -10,21 +10,19 @@
 import io.micrometer.core.instrument.binder.cache.CacheMeterBinder;
 import io.micrometer.core.instrument.binder.cache.HazelcastCacheMetrics;
 import io.micrometer.core.lang.Nullable;
-import java.lang.reflect.Method;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nonnull;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * {@link HazelcastCacheMetrics} equivalent which is compatible with Hazelcast 2 (Alfresco 5.x)
  */
 public class Hazelcast2CacheMetrics extends CacheMeterBinder {
 
-    private static final Logger logger = LoggerFactory.getLogger(Hazelcast2CacheMetrics.class);
+    static final String METER_CACHE_GETS_LATENCY = "cache.gets.latency";
+    static final String METER_CACHE_PUTS_LATENCY = "cache.puts.latency";
+    static final String METER_CACHE_REMOVALS_LATENCY = "cache.removals.latency";
 
     private static final String TAG_OWNERSHIP = "ownership";
-    private static final String METHOD_GET_OPERATION_STATS = "getOperationStats";
 
     private final IMap<?, ?> cache;
 
@@ -97,7 +95,7 @@ protected Long evictionCount() {
 
     @Override
     protected long putCount() {
-        return cache.getLocalMapStats().getPutOperationCount();
+        return cache.getLocalMapStats().getOperationStats().getNumberOfPuts();
     }
 
     @Override
@@ -125,8 +123,7 @@ protected void bindImplementationSpecificMetrics(@Nonnull MeterRegistry registry
                 .register(registry);
 
         FunctionCounter.builder("cache.partition.gets", cache,
-                cache -> extractMetricWithReflection(cache.getLocalMapStats(), METHOD_GET_OPERATION_STATS,
-                        "getNumberOfGets"))
+                c -> c.getLocalMapStats().getOperationStats().getNumberOfGets())
                 .tags(getTagsWithCacheName())
                 .description("The total number of get operations executed against this partition")
                 .register(registry);
@@ -136,50 +133,29 @@ protected void bindImplementationSpecificMetrics(@Nonnull MeterRegistry registry
     }
 
     private void timings(MeterRegistry registry) {
-        FunctionTimer.builder("cache.gets.latency", cache,
-                cache -> extractMetricWithReflection(cache.getLocalMapStats(), METHOD_GET_OPERATION_STATS,
-                        "getNumberOfGets"),
-                cache -> extractMetricWithReflection(cache.getLocalMapStats(), METHOD_GET_OPERATION_STATS,
-                        "getTotalGetLatency"),
+        FunctionTimer.builder(METER_CACHE_GETS_LATENCY, cache,
+                c -> c.getLocalMapStats().getOperationStats().getNumberOfGets(),
+                c -> c.getLocalMapStats().getOperationStats().getTotalGetLatency(),
                 TimeUnit.NANOSECONDS)
                 .tags(getTagsWithCacheName())
                 .description("Cache gets")
                 .register(registry);
 
-        FunctionTimer.builder("cache.puts.latency", cache,
-                cache -> extractMetricWithReflection(cache.getLocalMapStats(), METHOD_GET_OPERATION_STATS,
-                        "getNumberOfPuts"),
-                cache -> extractMetricWithReflection(cache.getLocalMapStats(), METHOD_GET_OPERATION_STATS,
-                        "getTotalPutLatency"),
+        FunctionTimer.builder(METER_CACHE_PUTS_LATENCY, cache,
+                c -> c.getLocalMapStats().getOperationStats().getNumberOfPuts(),
+                c -> c.getLocalMapStats().getOperationStats().getTotalPutLatency(),
                 TimeUnit.NANOSECONDS)
                 .tags(getTagsWithCacheName())
                 .description("Cache puts")
                 .register(registry);
 
-        FunctionTimer.builder("cache.removals.latency", cache,
-                cache -> extractMetricWithReflection(cache.getLocalMapStats(), METHOD_GET_OPERATION_STATS,
-                        "getNumberOfRemoves"),
-                cache -> extractMetricWithReflection(cache.getLocalMapStats(), METHOD_GET_OPERATION_STATS,
-                        "getTotalRemoveLatency"),
+        FunctionTimer.builder(METER_CACHE_REMOVALS_LATENCY, cache,
+                c -> c.getLocalMapStats().getOperationStats().getNumberOfRemoves(),
+                c -> c.getLocalMapStats().getOperationStats().getTotalRemoveLatency(),
                 TimeUnit.NANOSECONDS)
                 .tags(getTagsWithCacheName())
                 .description("Cache removals")
                 .register(registry);
     }
 
-    public static long extractMetricWithReflection(final Object object, final String... methods) {
-        try {
-            Object currentObject = object;
-            for (String methodToExecute : methods) {
-                final Method method = currentObject.getClass().getMethod(methodToExecute);
-                method.setAccessible(true);
-                currentObject = method.invoke(currentObject);
-            }
-            return (long) currentObject;
-        } catch (Throwable e) {
-            logger.warn("Unable to extract metric using reflection", e);
-            return -1;
-        }
-    }
-
 }

alfred-telemetry-platform/src/test/java/eu/xenit/alfred/telemetry/binder/cache/Hazelcast2CacheMetricsTest.java

@@ -0,0 +1,64 @@
+package eu.xenit.alfred.telemetry.binder.cache;
+
+import static eu.xenit.alfred.telemetry.binder.cache.Hazelcast2CacheMetrics.METER_CACHE_GETS_LATENCY;
+import static eu.xenit.alfred.telemetry.binder.cache.Hazelcast2CacheMetrics.METER_CACHE_PUTS_LATENCY;
+import static eu.xenit.alfred.telemetry.binder.cache.Hazelcast2CacheMetrics.METER_CACHE_REMOVALS_LATENCY;
+import static org.awaitility.Awaitility.await;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+
+import com.hazelcast.config.Config;
+import com.hazelcast.core.Hazelcast;
+import com.hazelcast.core.HazelcastInstance;
+import com.hazelcast.core.IMap;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+import java.time.Duration;
+import java.util.concurrent.TimeUnit;
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class Hazelcast2CacheMetricsTest {
+
+    private SimpleMeterRegistry meterRegistry;
+
+    @BeforeEach
+    void setup() {
+        meterRegistry = new SimpleMeterRegistry();
+    }
+
+    @Test
+    void testCacheMetrics() {
+        HazelcastInstance hazelcastInstance = Hazelcast.newHazelcastInstance(new Config());
+        IMap<String, String> cache = hazelcastInstance.getMap(this.getClass().getSimpleName());
+
+        Hazelcast2CacheMetrics.monitor(meterRegistry, cache);
+        cache.put("foo", "bar");
+
+        assertThat(cache.get("foo"), is("bar"));
+        assertThat(cache.get("baz"), is(nullValue()));
+
+        assertThat(meterRegistry.get("cache.gets").tag("result", "hit").functionCounter().count(), is(1.0));
+        await().atMost(Duration.ofSeconds(5))
+                .until(() -> meterRegistry.get("cache.puts").functionCounter().count(), is(1.0));
+
+        validateLatencyMetrics(METER_CACHE_GETS_LATENCY, is(2.0), is(greaterThanOrEqualTo(0.0)));
+        validateLatencyMetrics(METER_CACHE_PUTS_LATENCY, is(1.0), is(greaterThanOrEqualTo(0.0)));
+        validateLatencyMetrics(METER_CACHE_REMOVALS_LATENCY, is(0.0), is(0.0));
+
+        cache.remove("foo");
+        validateLatencyMetrics(METER_CACHE_REMOVALS_LATENCY, is(1.0), is(greaterThanOrEqualTo(0.0)));
+    }
+
+    private void validateLatencyMetrics(final String latencyMeterName, Matcher<Double> counterMatcher,
+            Matcher<Double> totalTimeNanosMatcher) {
+        await().atMost(Duration.ofSeconds(5))
+                .until(() -> meterRegistry.get(latencyMeterName).functionTimer().count(), counterMatcher);
+        await().atMost(Duration.ofSeconds(5))
+                .until(() -> meterRegistry.get(latencyMeterName).functionTimer().totalTime(TimeUnit.NANOSECONDS),
+                        totalTimeNanosMatcher);
+    }
+
+}

build.gradle

@@ -21,7 +21,7 @@ def copyPropertyValueIfExists(sourcePropertyName, targetPropertyName) {
 
 allprojects {
     group = 'eu.xenit.alfred.telemetry'
-    version = '0.7.0'
+    version = '0.7.1'
 
     boolean isRelease = ci.branch?.startsWith("release")
     if (!isRelease) {