From 825d01ea7d49be5f2e428e7a94401e8a62b79085 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 15 Apr 2015 11:49:28 +0100 Subject: [PATCH] When cache names clash include clash manager name in all prefixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when multiple cache managers had a cache with the same name, the prefix for the first cache that was processed would not include its cache manager’s name, but all subsequent prefixes would include the cache manager’s name. This was inconsistent, and due to the cache managers being unordered, the prefixing was not deterministic. This commit updates the prefixing logic so that when there is a clash, all of the affected prefixes will include the name of the cache manager. For example, with cache managers named first and second and each with a cache named users, the prefixes will be cache.first_users and cache.second_users rather than cache.users and cache.{first|second}_users. Closes gh-2824 --- .../actuate/endpoint/CachePublicMetrics.java | 40 ++++++++++++++----- .../PublicMetricsAutoConfigurationTests.java | 2 +- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/CachePublicMetrics.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/CachePublicMetrics.java index 89eee1b3a1f..70ded1aed89 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/CachePublicMetrics.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/CachePublicMetrics.java @@ -16,10 +16,12 @@ package org.springframework.boot.actuate.endpoint; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; -import java.util.Set; import javax.annotation.PostConstruct; @@ -54,8 +56,8 @@ public class CachePublicMetrics implements PublicMetrics { @Override public Collection> metrics() { - Set targetNames = new HashSet(); Collection> metrics = new HashSet>(); + Map> cacheManagerNamesByCacheName = getCacheManagerNamesByCacheName(); for (Map.Entry entry : this.cacheManagers.entrySet()) { String cacheManagerName = entry.getKey(); CacheManager cacheManager = entry.getValue(); @@ -64,8 +66,8 @@ public class CachePublicMetrics implements PublicMetrics { CacheStatistics cacheStatistics = this.cacheStatisticsProvider .getCacheStatistics(cache, cacheManager); if (cacheStatistics != null) { - String prefix = cleanPrefix(createPrefix(targetNames, cacheName, - cacheManagerName)); + String prefix = cleanPrefix(createPrefix( + cacheManagerNamesByCacheName, cacheName, cacheManagerName)); metrics.addAll(cacheStatistics.toMetrics(prefix)); } } @@ -73,22 +75,38 @@ public class CachePublicMetrics implements PublicMetrics { return metrics; } + private Map> getCacheManagerNamesByCacheName() { + Map> cacheManagerNamesByCacheName = new HashMap>(); + for (Map.Entry entry : this.cacheManagers.entrySet()) { + for (String cacheName : entry.getValue().getCacheNames()) { + List cacheManagerNames = cacheManagerNamesByCacheName + .get(cacheName); + if (cacheManagerNames == null) { + cacheManagerNames = new ArrayList(); + cacheManagerNamesByCacheName.put(cacheName, cacheManagerNames); + } + cacheManagerNames.add(entry.getKey()); + } + } + return cacheManagerNamesByCacheName; + } + /** - * Create the prefix to use for the specified cache. The specified {@code targetNames} - * set contains the names that have been acquired so far. - * @param targetNames the target names that have been used for other caches + * Create the prefix to use for the specified cache. The generated prefix should be + * unique among those that will be generated for the given map of cache names + * @param cacheManagerNamesByCacheName a mapping of cache names to the names of the + * cache managers that have a cache with that name * @param cacheName the name of the cache * @param cacheManagerName the name of its cache manager * @return a prefix to use for the specified cache */ - protected String createPrefix(Set targetNames, String cacheName, - String cacheManagerName) { - if (targetNames.contains(cacheName)) { + protected String createPrefix(Map> cacheManagerNamesByCacheName, + String cacheName, String cacheManagerName) { + if (cacheManagerNamesByCacheName.get(cacheName).size() > 1) { String target = cacheManagerName + "_" + cacheName; return createPrefixFor(target); } else { - targetNames.add(cacheName); return createPrefixFor(cacheName); } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/PublicMetricsAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/PublicMetricsAutoConfigurationTests.java index fcd17aed2e0..285d73c4789 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/PublicMetricsAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/PublicMetricsAutoConfigurationTests.java @@ -218,7 +218,7 @@ public class PublicMetricsAutoConfigurationTests { CachePublicMetrics bean = this.context.getBean(CachePublicMetrics.class); Collection> metrics = bean.metrics(); assertMetrics(metrics, "cache.books.size", "cache.second_speakers.size", - "cache.speakers.size", "cache.users.size"); + "cache.first_speakers.size", "cache.users.size"); } private void assertHasMetric(Collection> metrics, Metric metric) {