Add metric to Agent Configuration service

genie-docs/src/docs/asciidoc/_metrics.adoc

@@ -288,6 +288,12 @@ It will have to be added if one is desired otherwise metrics are just published
 |ScriptManager
 |status, exceptionClass, scriptUri
 
+|genie.services.agentConfiguration.reloadProperties.timer
+|Time taken to refresh the set of properties forwarded to bootstrapping agents
+|nanoseconds
+|AgentConfigurationServiceImpl
+|status, exceptionClass, numProperties
+
 |genie.services.agentJob.handshake.counter
 |Counter for calls to the 'handshake' protocol of the Genie Agent Job Service
 |count

genie-web/src/main/java/com/netflix/genie/web/agent/services/impl/AgentConfigurationServiceImpl.java

@@ -23,6 +23,9 @@
 import com.google.common.collect.Sets;
 import com.netflix.genie.web.agent.services.AgentConfigurationService;
 import com.netflix.genie.web.properties.AgentConfigurationProperties;
+import com.netflix.genie.web.util.MetricsUtils;
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Tag;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.checkerframework.checker.nullness.qual.NonNull;
@@ -35,6 +38,7 @@
 
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 import java.util.regex.Pattern;
 
 /**
@@ -49,24 +53,30 @@
 public class AgentConfigurationServiceImpl implements AgentConfigurationService {
 
     private static final String AGENT_PROPERTIES_CACHE_KEY = "agent-properties";
+    private static final String RELOAD_PROPERTIES_TIMER = "genie.services.agentConfiguration.reloadProperties.timer";
+    private static final String PROPERTIES_COUNT_TAG = "numProperties";
 
     private final AgentConfigurationProperties agentConfigurationProperties;
     private final Environment environment;
+    private final MeterRegistry registry;
     private final Pattern agentPropertiesPattern;
     private final LoadingCache<String, Map<String, String>> cache;
 
     /**
      * Constructor.
      *
      * @param agentConfigurationProperties the properties
-     * @param environment the environment
+     * @param environment                  the environment
+     * @param registry                     the metrics registry
      */
     public AgentConfigurationServiceImpl(
         final AgentConfigurationProperties agentConfigurationProperties,
-        final Environment environment
+        final Environment environment,
+        final MeterRegistry registry
     ) {
         this.agentConfigurationProperties = agentConfigurationProperties;
         this.environment = environment;
+        this.registry = registry;
 
         this.agentPropertiesPattern = Pattern.compile(
             this.agentConfigurationProperties.getAgentPropertiesFilterPattern(),
@@ -92,10 +102,26 @@ public Map<String, String> getAgentProperties() {
 
 
     private Map<String, String> loadProperties(@NonNull final String propertiesKey) {
-        if (AGENT_PROPERTIES_CACHE_KEY.equals(propertiesKey)) {
-            return reloadAgentProperties();
+        if (!AGENT_PROPERTIES_CACHE_KEY.equals(propertiesKey)) {
+            throw new IllegalArgumentException("Unknown key to load: " + propertiesKey);
+        }
+
+        final Set<Tag> tags = Sets.newHashSet();
+        final long start = System.nanoTime();
+        Integer numProperties = null;
+        try {
+            final Map<String, String> properties = reloadAgentProperties();
+            MetricsUtils.addSuccessTags(tags);
+            numProperties = properties.size();
+            return properties;
+        } catch (Throwable t) {
+            MetricsUtils.addFailureTagsWithException(tags, t);
+            throw t;
+        } finally {
+            tags.add(Tag.of(PROPERTIES_COUNT_TAG, String.valueOf(numProperties)));
+            this.registry.timer(RELOAD_PROPERTIES_TIMER, tags)
+                .record(System.nanoTime() - start, TimeUnit.NANOSECONDS);
         }
-        throw new IllegalArgumentException("Unknown key to load: " + propertiesKey);
     }
 
     private Map<String, String> reloadAgentProperties() {

genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/agent/services/AgentServicesAutoConfiguration.java

@@ -188,18 +188,16 @@ public AgentFilterServiceImpl agentFilterService(
      *
      * @param agentConfigurationProperties the service properties
      * @param environment                  the environment
+     * @param registry                     the metrics registry
      * @return a {@link AgentConfigurationService} instance
      */
     @Bean
     @ConditionalOnMissingBean(AgentConfigurationService.class)
     public AgentConfigurationServiceImpl agentConfigurationService(
         final AgentConfigurationProperties agentConfigurationProperties,
-        final Environment environment
+        final Environment environment,
+        final MeterRegistry registry
     ) {
-        final AgentConfigurationServiceImpl agentConfigurationService =
-            new AgentConfigurationServiceImpl(agentConfigurationProperties, environment);
-        // Warm the cache
-        agentConfigurationService.getAgentProperties();
-        return agentConfigurationService;
+        return new AgentConfigurationServiceImpl(agentConfigurationProperties, environment, registry);
     }
 }

genie-web/src/test/groovy/com/netflix/genie/web/agent/services/impl/AgentConfigurationServiceImplSpec.groovy

@@ -19,6 +19,8 @@ package com.netflix.genie.web.agent.services.impl
 
 import com.netflix.genie.web.agent.services.AgentConfigurationService
 import com.netflix.genie.web.properties.AgentConfigurationProperties
+import io.micrometer.core.instrument.MeterRegistry
+import io.micrometer.core.instrument.Timer
 import org.springframework.core.env.ConfigurableEnvironment
 import org.springframework.core.env.Environment
 import org.springframework.core.env.MapPropertySource
@@ -29,15 +31,19 @@ class AgentConfigurationServiceImplSpec extends Specification {
     AgentConfigurationProperties props
     ConfigurableEnvironment env
     MutablePropertySources propertySources
+    MeterRegistry registry
+    Timer timer
 
     void setup() {
         this.props = new AgentConfigurationProperties()
         this.env = Mock(ConfigurableEnvironment)
+        this.registry = Mock(MeterRegistry)
+        this.timer = Mock(Timer)
         this.propertySources = new MutablePropertySources()
     }
 
     def "Result is cached"() {
-        AgentConfigurationService service = new AgentConfigurationServiceImpl(props, env)
+        AgentConfigurationService service = new AgentConfigurationServiceImpl(props, env, registry)
 
         then:
 
@@ -46,24 +52,29 @@ class AgentConfigurationServiceImplSpec extends Specification {
 
         then:
         1 * env.getPropertySources() >> propertySources
+        1 * registry.timer(AgentConfigurationServiceImpl.RELOAD_PROPERTIES_TIMER, _) >> timer
+        1 * timer.record(_, _)
         agentProperties.isEmpty()
 
         when:
         service.getAgentProperties()
 
         then:
         0 * env.getPropertySources() >> propertySources
+        0 * registry.timer(_, _)
     }
 
     def "Fallback to standard environment during load"() {
         Environment env = Mock(Environment)
 
         when:
-        AgentConfigurationService service = new AgentConfigurationServiceImpl(props, env)
+        AgentConfigurationService service = new AgentConfigurationServiceImpl(props, env, registry)
         Map<String, String> agentProperties = service.getAgentProperties()
 
         then:
         _ * env.getProperty(_ as String) >> UUID.randomUUID().toString()
+        1 * registry.timer(AgentConfigurationServiceImpl.RELOAD_PROPERTIES_TIMER, _) >> timer
+        1 * timer.record(_, _)
         agentProperties != null
     }
 
@@ -94,7 +105,7 @@ class AgentConfigurationServiceImplSpec extends Specification {
         propertySources.addFirst(new MapPropertySource("map2", map2))
 
         when:
-        AgentConfigurationService service = new AgentConfigurationServiceImpl(props, env)
+        AgentConfigurationService service = new AgentConfigurationServiceImpl(props, env, registry)
         Map<String, String> agentProperties = service.getAgentProperties()
 
         then:
@@ -103,6 +114,8 @@ class AgentConfigurationServiceImplSpec extends Specification {
         1 * env.getProperty("agent.bar") >> "Bar"
         1 * env.getProperty("agent.null") >> null
         1 * env.getProperty("agent.empty") >> " "
+        1 * registry.timer(AgentConfigurationServiceImpl.RELOAD_PROPERTIES_TIMER, _) >> timer
+        1 * timer.record(_, _)
         agentProperties == expectedProperties
     }
 }