Log warning if ServoMonitorCache size exceeds threshold.
Fixes gh-947
This commit is contained in:
@@ -71,8 +71,8 @@ public class ServoMetricsAutoConfiguration {
|
||||
}
|
||||
|
||||
@Bean
|
||||
public ServoMonitorCache monitorCache(MonitorRegistry monitorRegistry) {
|
||||
return new ServoMonitorCache(monitorRegistry);
|
||||
public ServoMonitorCache monitorCache(MonitorRegistry monitorRegistry, ServoMetricsConfigBean servoMetricsConfig) {
|
||||
return new ServoMonitorCache(monitorRegistry, servoMetricsConfig);
|
||||
}
|
||||
|
||||
@Bean
|
||||
|
||||
@@ -34,19 +34,33 @@ public class ServoMetricsConfigBean {
|
||||
*/
|
||||
String registryClass = "com.netflix.servo.BasicMonitorRegistry";
|
||||
|
||||
public String getRegistryClass() {
|
||||
return this.registryClass;
|
||||
}
|
||||
/**
|
||||
* When the `ServoMonitorCache` reaches this size, a warning is logged.
|
||||
* This will be useful if you are using string concatenation in RestTemplate urls.
|
||||
*/
|
||||
int cacheWarningThreshold = 1000;
|
||||
|
||||
public boolean getEnabled() {
|
||||
return this.enabled;
|
||||
}
|
||||
|
||||
public void setEnabled(boolean enabled) {
|
||||
public void isEnabled(boolean enabled) {
|
||||
this.enabled = enabled;
|
||||
}
|
||||
|
||||
public String getRegistryClass() {
|
||||
return this.registryClass;
|
||||
}
|
||||
|
||||
public void setRegistryClass(String registryClass) {
|
||||
this.registryClass = registryClass;
|
||||
}
|
||||
|
||||
public int getCacheWarningThreshold() {
|
||||
return cacheWarningThreshold;
|
||||
}
|
||||
|
||||
public void setCacheWarningThreshold(int cacheWarningThreshold) {
|
||||
this.cacheWarningThreshold = cacheWarningThreshold;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,17 +20,22 @@ import com.netflix.servo.MonitorRegistry;
|
||||
import com.netflix.servo.monitor.BasicTimer;
|
||||
import com.netflix.servo.monitor.MonitorConfig;
|
||||
|
||||
import lombok.extern.apachecommons.CommonsLog;
|
||||
|
||||
/**
|
||||
* Servo does not provide a mechanism to retrieve an existing monitor by name + tags.
|
||||
*
|
||||
* @author Jon Schneider
|
||||
*/
|
||||
@CommonsLog
|
||||
public class ServoMonitorCache {
|
||||
private final Map<MonitorConfig, BasicTimer> timerCache = new HashMap<>();
|
||||
private final MonitorRegistry monitorRegistry;
|
||||
private final ServoMetricsConfigBean config;
|
||||
|
||||
public ServoMonitorCache(MonitorRegistry monitorRegistry) {
|
||||
public ServoMonitorCache(MonitorRegistry monitorRegistry, ServoMetricsConfigBean config) {
|
||||
this.monitorRegistry = monitorRegistry;
|
||||
this.config = config;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -39,13 +44,18 @@ public class ServoMonitorCache {
|
||||
* return it.
|
||||
*/
|
||||
public synchronized BasicTimer getTimer(MonitorConfig config) {
|
||||
BasicTimer t = timerCache.get(config);
|
||||
BasicTimer t = this.timerCache.get(config);
|
||||
if (t != null)
|
||||
return t;
|
||||
|
||||
t = new BasicTimer(config);
|
||||
timerCache.put(config, t);
|
||||
monitorRegistry.register(t);
|
||||
this.timerCache.put(config, t);
|
||||
|
||||
if (this.timerCache.size() > this.config.getCacheWarningThreshold()) {
|
||||
log.warn("timerCache is above the warning threshold of " + this.config.getCacheWarningThreshold() + " with size " + this.timerCache.size() + ".");
|
||||
}
|
||||
|
||||
this.monitorRegistry.register(t);
|
||||
return t;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,7 +21,7 @@ public class ServoMetricReaderTests {
|
||||
new DimensionalServoMetricNaming());
|
||||
|
||||
MonitorConfig.Builder builder = new MonitorConfig.Builder("metricName");
|
||||
ServoMonitorCache servoMonitorCache = new ServoMonitorCache(registry);
|
||||
ServoMonitorCache servoMonitorCache = new ServoMonitorCache(registry, new ServoMetricsConfigBean());
|
||||
servoMonitorCache.getTimer(builder.build());
|
||||
|
||||
List<Metric<?>> metrics = Lists.newArrayList(reader.findAll());
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
package org.springframework.cloud.netflix.metrics.servo;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.MockitoAnnotations;
|
||||
import org.springframework.boot.test.OutputCapture;
|
||||
|
||||
import com.netflix.servo.MonitorRegistry;
|
||||
import com.netflix.servo.monitor.MonitorConfig;
|
||||
|
||||
import static org.hamcrest.Matchers.*;
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
/**
|
||||
* @author Spencer Gibb
|
||||
*/
|
||||
public class ServoMonitorCacheTests {
|
||||
|
||||
@Rule
|
||||
public OutputCapture capture = new OutputCapture();
|
||||
|
||||
@Mock
|
||||
MonitorRegistry monitorRegistry;
|
||||
|
||||
@Before
|
||||
public void before() {
|
||||
MockitoAnnotations.initMocks(this);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCacheWarningLog() {
|
||||
ServoMetricsConfigBean config = new ServoMetricsConfigBean();
|
||||
config.setCacheWarningThreshold(1);
|
||||
ServoMonitorCache cache = new ServoMonitorCache(monitorRegistry, config);
|
||||
|
||||
cache.getTimer(MonitorConfig.builder("monitorA").build());
|
||||
|
||||
assertThat(this.capture.toString(), not(containsString("timerCache is above the warning threshold")));
|
||||
|
||||
cache.getTimer(MonitorConfig.builder("monitorB").build());
|
||||
|
||||
assertThat(this.capture.toString(), containsString("timerCache is above the warning threshold"));
|
||||
}
|
||||
}
|
||||
@@ -13,7 +13,6 @@
|
||||
|
||||
package org.springframework.cloud.netflix.metrics.spectator;
|
||||
|
||||
import org.springframework.beans.factory.annotation.Value;
|
||||
import org.springframework.boot.actuate.autoconfigure.MetricRepositoryAutoConfiguration;
|
||||
import org.springframework.boot.actuate.endpoint.MetricReaderPublicMetrics;
|
||||
import org.springframework.boot.actuate.metrics.CounterService;
|
||||
@@ -25,6 +24,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
|
||||
import org.springframework.cloud.netflix.metrics.DefaultMetricsTagProvider;
|
||||
import org.springframework.cloud.netflix.metrics.MetricsInterceptorConfiguration;
|
||||
import org.springframework.cloud.netflix.metrics.MetricsTagProvider;
|
||||
import org.springframework.cloud.netflix.metrics.servo.ServoMetricsConfigBean;
|
||||
import org.springframework.cloud.netflix.metrics.servo.ServoMonitorCache;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
@@ -50,14 +50,18 @@ import com.netflix.spectator.servo.ServoRegistry;
|
||||
@ConditionalOnClass({ Registry.class, MetricReader.class })
|
||||
@Import(MetricsInterceptorConfiguration.class)
|
||||
public class SpectatorMetricsAutoConfiguration {
|
||||
@Value("${netflix.metrics.servo.registryClass:com.netflix.servo.BasicMonitorRegistry}")
|
||||
String servoRegistryClass;
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
public MonitorRegistry monitorRegistry() {
|
||||
public ServoMetricsConfigBean servoMetricsConfig() {
|
||||
return new ServoMetricsConfigBean();
|
||||
}
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
public MonitorRegistry monitorRegistry(ServoMetricsConfigBean configBean) {
|
||||
System.setProperty(DefaultMonitorRegistry.class.getCanonicalName()
|
||||
+ ".registryClass", servoRegistryClass);
|
||||
+ ".registryClass", configBean.getRegistryClass());
|
||||
return DefaultMonitorRegistry.getInstance();
|
||||
}
|
||||
|
||||
@@ -68,8 +72,8 @@ public class SpectatorMetricsAutoConfiguration {
|
||||
}
|
||||
|
||||
@Bean
|
||||
public ServoMonitorCache monitorCache() {
|
||||
return new ServoMonitorCache(monitorRegistry());
|
||||
public ServoMonitorCache monitorCache(MonitorRegistry monitorRegistry, ServoMetricsConfigBean configBean) {
|
||||
return new ServoMonitorCache(monitorRegistry, configBean);
|
||||
}
|
||||
|
||||
@Bean
|
||||
|
||||
Reference in New Issue
Block a user