From d59cbc830a979781feaaaea5590192351ed8b593 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 22 May 2014 09:33:55 +0100 Subject: [PATCH] Group resolution for RedisMultiMetricRepository with prefix The prefix needs to be added before looking for keys. In addition I rationalized the constructor and final fields (it didn't make any sense for the prefix to be mutable). Fixes gh-927 --- .../redis/RedisMetricRepository.java | 2 +- .../redis/RedisMultiMetricRepository.java | 24 ++++++++-------- .../RedisMultiMetricRepositoryTests.java | 28 ++++++++++++++++--- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMetricRepository.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMetricRepository.java index 201d04dd2ed..fb5ca2c0c3f 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMetricRepository.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMetricRepository.java @@ -75,7 +75,7 @@ public class RedisMetricRepository implements MetricRepository { /** * The redis key to use to store the index of other keys. The redis store will hold a - * zset under this key. Defaults to "keys.spring.metrics". REad operations, especially + * zset under this key. Defaults to "keys.spring.metrics". Read operations, especially * {@link #findAll()} and {@link #count()}, will be much more efficient if the key is * unique to the {@link #setPrefix(String) prefix} of this repository. * @param key the key to set diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepository.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepository.java index 5404c37983d..4611f989011 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepository.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepository.java @@ -34,7 +34,7 @@ import org.springframework.util.Assert; * {@link MultiMetricRepository} implementation backed by a redis store. Metric values are * stored as regular values against a key composed of the group name prefixed with a * constant prefix (default "spring.groups."). The group names are stored as a zset under - * [prefix] + "keys". + * "keys." + [prefix]. * * @author Dave Syer */ @@ -42,30 +42,28 @@ public class RedisMultiMetricRepository implements MultiMetricRepository { private static final String DEFAULT_METRICS_PREFIX = "spring.groups."; - private String prefix = DEFAULT_METRICS_PREFIX; + private final String prefix; - private String keys = "keys." + this.prefix; + private final String keys; private final BoundZSetOperations zSetOperations; private final RedisOperations redisOperations; public RedisMultiMetricRepository(RedisConnectionFactory redisConnectionFactory) { - Assert.notNull(redisConnectionFactory, "RedisConnectionFactory must not be null"); - this.redisOperations = RedisUtils.stringTemplate(redisConnectionFactory); - this.zSetOperations = this.redisOperations.boundZSetOps(this.keys); + this(redisConnectionFactory, DEFAULT_METRICS_PREFIX); } - /** - * The prefix for all metrics keys. - * @param prefix the prefix to set for all metrics keys - */ - public void setPrefix(String prefix) { + public RedisMultiMetricRepository(RedisConnectionFactory redisConnectionFactory, + String prefix) { + Assert.notNull(redisConnectionFactory, "RedisConnectionFactory must not be null"); + this.redisOperations = RedisUtils.stringTemplate(redisConnectionFactory); if (!prefix.endsWith(".")) { prefix = prefix + "."; } this.prefix = prefix; - this.keys = "keys." + this.prefix; + this.keys = "keys." + this.prefix.substring(0, prefix.length() - 1); + this.zSetOperations = this.redisOperations.boundZSetOps(this.keys); } @Override @@ -105,7 +103,7 @@ public class RedisMultiMetricRepository implements MultiMetricRepository { Set range = this.zSetOperations.range(0, -1); Collection result = new ArrayList(); for (String key : range) { - result.add(nameFor(key)); + result.add(nameFor(this.prefix + key)); } return range; } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepositoryTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepositoryTests.java index e2309def8a9..d8366ddc0e5 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepositoryTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepositoryTests.java @@ -17,11 +17,16 @@ package org.springframework.boot.actuate.metrics.repository.redis; import java.util.Arrays; +import java.util.List; import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; import org.springframework.boot.actuate.metrics.Iterables; import org.springframework.boot.actuate.metrics.Metric; import org.springframework.data.redis.core.StringRedisTemplate; @@ -33,27 +38,42 @@ import static org.junit.Assert.assertTrue; /** * @author Dave Syer */ +@RunWith(Parameterized.class) public class RedisMultiMetricRepositoryTests { @Rule public RedisServer redis = RedisServer.running(); private RedisMultiMetricRepository repository; + @Parameter(0) + public String prefix; + + @Parameters + public static List parameters() { + return Arrays. asList(new Object[] { null }, new Object[] { "test" }); + } @Before public void init() { - this.repository = new RedisMultiMetricRepository(this.redis.getResource()); + if (this.prefix == null) { + this.prefix = "spring.groups"; + this.repository = new RedisMultiMetricRepository(this.redis.getResource()); + } + else { + this.repository = new RedisMultiMetricRepository(this.redis.getResource(), + this.prefix); + } } @After public void clear() { assertTrue(new StringRedisTemplate(this.redis.getResource()).opsForZSet().size( - "spring.groups.foo") > 0); + "keys." + this.prefix) > 0); this.repository.reset("foo"); this.repository.reset("bar"); assertNull(new StringRedisTemplate(this.redis.getResource()).opsForValue().get( - "spring.groups.foo")); + this.prefix + ".foo")); assertNull(new StringRedisTemplate(this.redis.getResource()).opsForValue().get( - "spring.groups.bar")); + this.prefix + ".bar")); } @Test