From 3fb0ae3e3bb0bb17ec8bd36b7efc0823f3a0e78f Mon Sep 17 00:00:00 2001 From: Jay Anderson Date: Thu, 17 Sep 2015 13:06:05 -0700 Subject: [PATCH 1/2] Register dropwizard gauges once and then update them The previous implementation would remove and add a new Gauge each time a metric was written. After this change the Gauge is registered once and the value is updated on subsequent calls. --- .../dropwizard/DropwizardMetricServices.java | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java index a3d9856ff7..4f24bfa038 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java @@ -51,7 +51,7 @@ public class DropwizardMetricServices implements CounterService, GaugeService { private final MetricRegistry registry; - private final ConcurrentMap gaugeLocks = new ConcurrentHashMap(); + private final ConcurrentMap gauges = new ConcurrentHashMap(); private final ConcurrentHashMap names = new ConcurrentHashMap(); @@ -99,15 +99,22 @@ public class DropwizardMetricServices implements CounterService, GaugeService { } else { name = wrapGaugeName(name); - final double gauge = value; - // Ensure we synchronize to avoid another thread pre-empting this thread after - // remove causing an error in Dropwizard metrics - // NOTE: Dropwizard provides no way to do this atomically - synchronized (getGaugeLock(name)) { - this.registry.remove(name); - this.registry.register(name, new SimpleGauge(gauge)); + setGaugeValue(name, value); + } + } + + private void setGaugeValue(final String name, final double value) { + // NOTE: Dropwizard provides no way to do this atomically + SimpleGauge gauge = this.gauges.get(name); + if (gauge == null) { + final SimpleGauge newGauge = new SimpleGauge(value); + gauge = this.gauges.putIfAbsent(name, newGauge); + if (gauge == null) { + gauge = newGauge; + this.registry.register(name, gauge); } } + gauge.setValue(value); } private String wrapGaugeName(String metricName) { @@ -130,16 +137,6 @@ public class DropwizardMetricServices implements CounterService, GaugeService { return name; } - private Object getGaugeLock(String name) { - Object lock = this.gaugeLocks.get(name); - if (lock == null) { - Object newLock = new Object(); - lock = this.gaugeLocks.putIfAbsent(name, newLock); - lock = (lock == null ? newLock : lock); - } - return lock; - } - @Override public void reset(String name) { if (!name.startsWith("meter")) { @@ -153,7 +150,7 @@ public class DropwizardMetricServices implements CounterService, GaugeService { */ private final static class SimpleGauge implements Gauge { - private final double value; + private volatile double value; private SimpleGauge(double value) { this.value = value; @@ -163,6 +160,10 @@ public class DropwizardMetricServices implements CounterService, GaugeService { public Double getValue() { return this.value; } + + public void setValue(final double value) { + this.value = value; + } } } From 64bcba47a96ef869908fb12f320a2c94da6905ee Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 30 Sep 2015 10:37:59 +0100 Subject: [PATCH 2/2] Polish contribution - Add @author tag - Remove unnecessary final modifiers - Avoid writing to volatile field when new gauge is used Closes gh-3977 --- .../metrics/dropwizard/DropwizardMetricServices.java | 12 +++++++----- .../dropwizard/DropwizardMetricServicesTests.java | 3 ++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java index 4f24bfa038..45b3beb063 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java @@ -46,6 +46,8 @@ import com.codahale.metrics.Timer; * * * @author Dave Syer + * @author Jay Anderson + * @author Andy Wilkinson */ public class DropwizardMetricServices implements CounterService, GaugeService { @@ -103,15 +105,15 @@ public class DropwizardMetricServices implements CounterService, GaugeService { } } - private void setGaugeValue(final String name, final double value) { + private void setGaugeValue(String name, double value) { // NOTE: Dropwizard provides no way to do this atomically SimpleGauge gauge = this.gauges.get(name); if (gauge == null) { - final SimpleGauge newGauge = new SimpleGauge(value); + SimpleGauge newGauge = new SimpleGauge(value); gauge = this.gauges.putIfAbsent(name, newGauge); if (gauge == null) { - gauge = newGauge; - this.registry.register(name, gauge); + this.registry.register(name, newGauge); + return; } } gauge.setValue(value); @@ -161,7 +163,7 @@ public class DropwizardMetricServices implements CounterService, GaugeService { return this.value; } - public void setValue(final double value) { + public void setValue(double value) { this.value = value; } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java index 0666a59642..4e7aadd091 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java @@ -65,9 +65,10 @@ public class DropwizardMetricServicesTests { @Test public void setGauge() { this.writer.submit("foo", 2.1); - this.writer.submit("foo", 2.3); @SuppressWarnings("unchecked") Gauge gauge = (Gauge) this.registry.getMetrics().get("gauge.foo"); + assertEquals(new Double(2.1), gauge.getValue()); + this.writer.submit("foo", 2.3); assertEquals(new Double(2.3), gauge.getValue()); }