Tolerate Gauges with non-Number values

Spring Boot's metrics infrastructure requires a Metric to have a
Number value. Coda Hale's ThreadStatesGaugeSet includes a Gauge
named deadlocks with a Set<String> value (each entry in the set is a
description, including stacktrace, of a deadlocked thread). There's
no obvious way to coerce this to a Number, and there's already a
deadlocks.count metric in the set.

This commit updates MetricRegistryMetricReader to ignore the addition
of any Gauge with a non-Number value.

Fixes gh-2593

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/reader/MetricRegistryMetricReader.java

@@ -25,6 +25,8 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.BeanUtils;
 import org.springframework.beans.BeanWrapperImpl;
 import org.springframework.boot.actuate.metrics.Metric;
@@ -53,6 +55,8 @@ import com.codahale.metrics.Timer;
  */
 public class MetricRegistryMetricReader implements MetricReader, MetricRegistryListener {
 
+	private static Log logger = LogFactory.getLog(MetricRegistryMetricReader.class);
+
 	private static final Map<Class<?>, Set<String>> NUMBER_KEYS = new ConcurrentHashMap<Class<?>, Set<String>>();
 
 	private final Object monitor = new Object();
@@ -125,9 +129,17 @@ public class MetricRegistryMetricReader implements MetricReader, MetricRegistryL
 
 	@Override
 	public void onGaugeAdded(String name, Gauge<?> gauge) {
-		this.names.put(name, name);
-		synchronized (this.monitor) {
-			this.reverse.add(name, name);
+		if (gauge.getValue() instanceof Number) {
+			this.names.put(name, name);
+			synchronized (this.monitor) {
+				this.reverse.add(name, name);
+			}
+			return;
+		}
+
+		if (logger.isDebugEnabled()) {
+			logger.debug("Ignoring gauge '" + name + "' (" + gauge
+					+ ") as its value is not a Number");
 		}
 	}
 
@@ -218,8 +230,10 @@ public class MetricRegistryMetricReader implements MetricReader, MetricRegistryL
 			keys = this.reverse.remove(name);
 		}
 
-		for (String key : keys) {
-			this.names.remove(name + "." + key);
+		if (keys != null) {
+			for (String key : keys) {
+				this.names.remove(name + "." + key);
+			}
 		}
 	}
 

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/reader/MetricRegistryMetricReaderTests.java

+/*
+ * Copyright 2012-2015 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.actuate.metrics.reader;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.junit.Test;
+import org.springframework.boot.actuate.metrics.Metric;
+
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.MetricRegistry;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThat;
+
+/**
+ * Tests for {@link MetricRegistryMetricReader}
+ *
+ * @author Andy Wilkinson
+ */
+public class MetricRegistryMetricReaderTests {
+
+	private final MetricRegistry metricRegistry = new MetricRegistry();
+
+	private final MetricRegistryMetricReader metricReader = new MetricRegistryMetricReader(
+			this.metricRegistry);
+
+	@Test
+	public void nonNumberGaugesAreTolerated() {
+		this.metricRegistry.register("test", new Gauge<Set<String>>() {
+
+			@Override
+			public Set<String> getValue() {
+				return new HashSet<String>();
+			}
+
+		});
+		assertThat(this.metricReader.findOne("test"), is(nullValue()));
+		this.metricRegistry.remove("test");
+		assertThat(this.metricReader.findOne("test"), is(nullValue()));
+	}
+
+	@Test
+	@SuppressWarnings("unchecked")
+	public void numberGauge() {
+		this.metricRegistry.register("test", new Gauge<Number>() {
+
+			@Override
+			public Number getValue() {
+				return new Integer(5);
+			}
+
+		});
+		Metric<Integer> metric = (Metric<Integer>) this.metricReader.findOne("test");
+		assertThat(metric.getValue(), equalTo(new Integer(5)));
+		this.metricRegistry.remove("test");
+		assertThat(this.metricReader.findOne("test"), is(nullValue()));
+	}
+}