diff --git a/simpleclient_spring_web/src/main/java/io/prometheus/client/spring/web/MethodTimer.java b/simpleclient_spring_web/src/main/java/io/prometheus/client/spring/web/MethodTimer.java index 301b54188..fc02ac923 100644 --- a/simpleclient_spring_web/src/main/java/io/prometheus/client/spring/web/MethodTimer.java +++ b/simpleclient_spring_web/src/main/java/io/prometheus/client/spring/web/MethodTimer.java @@ -1,5 +1,7 @@ package io.prometheus.client.spring.web; +import io.prometheus.client.Histogram; +import io.prometheus.client.SimpleCollector; import io.prometheus.client.Summary; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; @@ -10,6 +12,7 @@ import org.springframework.core.annotation.AnnotationUtils; import org.springframework.web.bind.annotation.ControllerAdvice; +import java.lang.reflect.InvocationTargetException; import java.util.HashMap; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; @@ -25,8 +28,8 @@ @Scope("prototype") @ControllerAdvice public class MethodTimer { - private final ReadWriteLock summaryLock = new ReentrantReadWriteLock(); - private final HashMap summaries = new HashMap(); + private final ReadWriteLock collectorLock = new ReentrantReadWriteLock(); + private final HashMap collectors = new HashMap(); @Pointcut("@annotation(io.prometheus.client.spring.web.PrometheusTimeMethod)") public void annotatedMethod() {} @@ -50,7 +53,7 @@ private PrometheusTimeMethod getAnnotation(ProceedingJoinPoint pjp) throws NoSuc return AnnotationUtils.findAnnotation(pjp.getTarget().getClass().getDeclaredMethod(name, parameterTypes), PrometheusTimeMethod.class); } - private Summary ensureSummary(ProceedingJoinPoint pjp, String key) throws IllegalStateException { + private SimpleCollector ensureCollector(ProceedingJoinPoint pjp, String key) throws IllegalStateException { PrometheusTimeMethod annot; try { annot = getAnnotation(pjp); @@ -62,29 +65,43 @@ private Summary ensureSummary(ProceedingJoinPoint pjp, String key) throws Illega assert(annot != null); - Summary summary; + SimpleCollector simpleCollector; // We use a writeLock here to guarantee no concurrent reads. - final Lock writeLock = summaryLock.writeLock(); + final Lock writeLock = collectorLock.writeLock(); writeLock.lock(); try { // Check one last time with full mutual exclusion in case multiple readers got null before creation. - summary = summaries.get(key); - if (summary != null) { - return summary; + simpleCollector = collectors.get(key); + if (simpleCollector != null) { + return simpleCollector; } - // Now we know for sure that we have never before registered. - summary = Summary.build() - .name(annot.name()) - .help(annot.help()) - .register(); - - // Even a rehash of the underlying table will not cause issues as we mutually exclude readers while we - // perform our updates. - summaries.put(key, summary); - - return summary; + try { + SimpleCollector.Builder builder = (SimpleCollector.Builder) annot.collectorClass() + .getMethod("build").invoke(null); + + // Now we know for sure that we have never before registered. + simpleCollector = builder + .name(annot.name()) + .help(annot.help()) + .register(); + + // Even a rehash of the underlying table will not cause issues as we mutually exclude readers while we + // perform our updates. + collectors.put(key, simpleCollector); + + return simpleCollector; + } catch (NoSuchMethodException noSuchMethodException) { + throw new IllegalArgumentException("Invalid collectorClass specified. Only Summary and " + + "Histogram collectors are supported for PrometheusTimedMethod collection.", noSuchMethodException); + } catch (IllegalAccessException illegalAccessException) { + throw new IllegalArgumentException("Invalid collectorClass specified. Only Summary and " + + "Histogram collectors are supported for PrometheusTimedMethod collection.", illegalAccessException); + } catch (InvocationTargetException invocationTargetException) { + throw new IllegalArgumentException("Invalid collectorClass specified. Only Summary and " + + "Histogram collectors are supported for PrometheusTimedMethod collection.", invocationTargetException); + } } finally { writeLock.unlock(); } @@ -94,25 +111,37 @@ private Summary ensureSummary(ProceedingJoinPoint pjp, String key) throws Illega public Object timeMethod(ProceedingJoinPoint pjp) throws Throwable { String key = pjp.getSignature().toLongString(); - Summary summary; - final Lock r = summaryLock.readLock(); + SimpleCollector collector; + final Lock r = collectorLock.readLock(); r.lock(); try { - summary = summaries.get(key); + collector = collectors.get(key); } finally { r.unlock(); } - if (summary == null) { - summary = ensureSummary(pjp, key); + if (collector == null) { + collector = ensureCollector(pjp, key); } - final Summary.Timer t = summary.startTimer(); + if (collector.getClass().equals(Summary.class)) { + final Summary.Timer t = ((Summary) collector).startTimer(); - try { - return pjp.proceed(); - } finally { - t.observeDuration(); + try { + return pjp.proceed(); + } finally { + t.observeDuration(); + } + } else if (collector.getClass().equals(Histogram.class)) { + final Histogram.Timer t = ((Histogram) collector).startTimer(); + + try { + return pjp.proceed(); + } finally { + t.observeDuration(); + } + } else { + throw new IllegalStateException("Unsupported Collector class: " + collector.getClass().getCanonicalName()); } } } diff --git a/simpleclient_spring_web/src/main/java/io/prometheus/client/spring/web/PrometheusTimeMethod.java b/simpleclient_spring_web/src/main/java/io/prometheus/client/spring/web/PrometheusTimeMethod.java index 990f69ed5..f14e52de5 100644 --- a/simpleclient_spring_web/src/main/java/io/prometheus/client/spring/web/PrometheusTimeMethod.java +++ b/simpleclient_spring_web/src/main/java/io/prometheus/client/spring/web/PrometheusTimeMethod.java @@ -1,5 +1,8 @@ package io.prometheus.client.spring.web; +import io.prometheus.client.SimpleCollector; +import io.prometheus.client.Summary; + import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -7,8 +10,9 @@ /** * Enable Spring-AOP-based automated method timing for the annotated method. The timings will be recorded in a - * {@link io.prometheus.client.Summary} with a name specified by the required {@code name} parameter, and help - * specified by the {@code help} parameter. + * {@link io.prometheus.client.Summary} or a {@link io.prometheus.client.Histogram } with a name specified by the + * required {@code name} parameter, and help specified by the {@code help} parameter. The default collector is a + * Summary. Histograms use default bucketing. * * To properly work, {@link EnablePrometheusTiming} must be specified somewhere in your application configuration. * @@ -17,7 +21,7 @@ * public class MyController { * {@literal @}RequestMapping("/") * {@literal @}ResponseBody - * {@literal @}PrometheusTimeMethod(name = "my_method_seconds", help = "The number of seconds taken by the main handler") + * {@literal @}PrometheusTimeMethod(name = "my_method_seconds", help = "The number of seconds taken by the main handler", collectorClass = Summary.class) * public Object handleRequest() { * // Each invocation will be timed and recorded. * return database.withCache().get("some_data"); @@ -42,4 +46,10 @@ * @return A help string */ String help(); + + /** + * Defines the collector class to be used to capture metrics. Supports Summary (default) and Histogram. + * @return Collector class + */ + Class collectorClass() default Summary.class; } diff --git a/simpleclient_spring_web/src/test/java/io/prometheus/client/spring/web/MethodTimerTest.java b/simpleclient_spring_web/src/test/java/io/prometheus/client/spring/web/MethodTimerTest.java index a42f27f68..46b2abf1e 100644 --- a/simpleclient_spring_web/src/test/java/io/prometheus/client/spring/web/MethodTimerTest.java +++ b/simpleclient_spring_web/src/test/java/io/prometheus/client/spring/web/MethodTimerTest.java @@ -2,6 +2,7 @@ import io.prometheus.client.Collector; import io.prometheus.client.CollectorRegistry; +import io.prometheus.client.Histogram; import org.junit.Assert; import org.junit.Test; import org.springframework.aop.aspectj.annotation.AspectJProxyFactory; @@ -14,6 +15,7 @@ public class MethodTimerTest { private interface Timeable { void timeMe() throws Exception; + void timeMeWithHistogram() throws Exception; } private final class TestClass implements Timeable { @@ -22,10 +24,16 @@ public void timeMe() throws Exception { Thread.sleep(20); } + @PrometheusTimeMethod(name = "test_class_histogram", help = "help two", collectorClass = Histogram.class) + public void timeMeWithHistogram() throws Exception { + Thread.sleep(100); + } + } private interface Time2 { void timeMe() throws Exception; + void timeMeWithHistogram() throws Exception; void aSecondMethod() throws Exception; } @@ -41,6 +49,17 @@ public void timeMethod() throws Exception { final Double tot = CollectorRegistry.defaultRegistry.getSampleValue("test_class_sum"); Assert.assertNotNull(tot); assertEquals(0.02, tot, 0.01); + + proxy.timeMeWithHistogram(); + + final Double histogramTotalFiveMillisBucket = CollectorRegistry.defaultRegistry.getSampleValue("test_class_histogram_bucket", + new String[] { "le" }, new String[] { "0.005" }); + assertEquals(histogramTotalFiveMillisBucket, Double.valueOf(0.0)); + + final Double histogramTotal500msBucket = CollectorRegistry.defaultRegistry.getSampleValue("test_class_histogram_bucket", + new String[] { "le" }, new String[] { "0.5" }); + assertEquals(histogramTotal500msBucket, Double.valueOf(1.0)); + } T getProxy(T source){ @@ -52,6 +71,7 @@ T getProxy(T source){ @Test public void testValueParam() throws Exception { final String name = "foobar"; + final String nameHistogram = "foobarHistogram"; Time2 a = getProxy(new Time2() { @PrometheusTimeMethod(name = name, help="help") @Override @@ -59,6 +79,12 @@ public void timeMe() throws Exception { Thread.sleep(35); } + @Override + @PrometheusTimeMethod(name = nameHistogram, help="help", collectorClass = Histogram.class) + public void timeMeWithHistogram() throws Exception { + Thread.sleep(40); + } + @Override public void aSecondMethod() throws Exception { @@ -75,12 +101,22 @@ public void aSecondMethod() throws Exception { a.timeMe(); final Double tot2 = CollectorRegistry.defaultRegistry.getSampleValue(name + "_sum"); assertEquals(0.035*4, tot2, 0.1); + + a.timeMeWithHistogram(); + + final Double histogramTotalOneTenthBucket = CollectorRegistry.defaultRegistry.getSampleValue(nameHistogram + "_bucket", new String[] { "le" }, new String[] { "0.1" }); + assertEquals(histogramTotalOneTenthBucket, Double.valueOf(1.0)); + + final Double histogramTotOneHundredthSecond = CollectorRegistry.defaultRegistry.getSampleValue(nameHistogram + "_bucket", new String[] { "le" }, new String[] { "0.01" }); + assertEquals(histogramTotOneHundredthSecond, Double.valueOf(0.0)); } @Test public void testHelpParam() throws Exception { final String name = "foo"; final String help = "help"; + final String nameHistogram = "fooHistogram"; + final String helpHistogram = "help histogram"; Time2 a = getProxy(new Time2() { @Override @@ -89,6 +125,12 @@ public void timeMe() throws Exception { Thread.sleep(100); } + @Override + @PrometheusTimeMethod(name = nameHistogram, help= helpHistogram, collectorClass = Histogram.class) + public void timeMeWithHistogram() throws Exception { + Thread.sleep(25); + } + @Override public void aSecondMethod() throws Exception { @@ -96,18 +138,25 @@ public void aSecondMethod() throws Exception { }); a.timeMe(); + a.timeMeWithHistogram(); final Enumeration samples = CollectorRegistry.defaultRegistry.metricFamilySamples(); - Collector.MetricFamilySamples sample = null; + Collector.MetricFamilySamples sampleSummary = null; + Collector.MetricFamilySamples sampleHistogram = null; while (samples.hasMoreElements()) { - sample = samples.nextElement(); + Collector.MetricFamilySamples sample = samples.nextElement(); if (name.equals(sample.name)) { - break; + sampleSummary = sample; + } else if (nameHistogram.equals(sample.name)) { + sampleHistogram = sample; } } - Assert.assertNotNull(sample); - assertEquals(help, sample.help); + Assert.assertNotNull(sampleSummary); + assertEquals(help, sampleSummary.help); + + Assert.assertNotNull(sampleHistogram); + assertEquals(helpHistogram, sampleHistogram.help); } private class MyException extends Exception { @@ -126,6 +175,12 @@ public void timeMe() throws Exception { throw new MyException("Yo this is an exception"); } + @Override + @PrometheusTimeMethod(name = "fooasdf2", help="help", collectorClass = Histogram.class) + public void timeMeWithHistogram() throws Exception { + Thread.sleep(25); + } + @Override public void aSecondMethod() throws Exception { } @@ -155,6 +210,12 @@ public void timeMe() throws Exception { Thread.sleep(misnamedSleepTime); } + @Override + @PrometheusTimeMethod(name = "fooasdf3", help="help", collectorClass = Histogram.class) + public void timeMeWithHistogram() throws Exception { + Thread.sleep(25); + } + @Override @PrometheusTimeMethod(name = "second_method_name_seconds", help = "help two") public void aSecondMethod() throws Exception {