Merge pull request #4198 from AlanRoth/PAYARA-3501

PAYARA-3501 Configuration changes in MP health and metrics in Admin console don't warn about restart needed
payara · Sep 9, 2019 · 31700f4 · 31700f4
2 parents 0c40816 + d12aaa3
commit 31700f4
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 24 deletions.
diff --git a/...s/microprofile/metrics/src/main/java/fish/payara/microprofile/metrics/MetricsService.java b/...s/microprofile/metrics/src/main/java/fish/payara/microprofile/metrics/MetricsService.java
@@ -50,6 +50,7 @@
 import fish.payara.monitoring.collect.MonitoringDataCollector;
 import fish.payara.monitoring.collect.MonitoringDataSource;
 import fish.payara.nucleus.executorservice.PayaraExecutorService;
+import java.beans.PropertyChangeEvent;
 
 import org.eclipse.microprofile.metrics.Counting;
 import org.eclipse.microprofile.metrics.Metadata;
@@ -86,19 +87,29 @@
 
 import static org.eclipse.microprofile.metrics.MetricRegistry.Type.BASE;
 import static org.eclipse.microprofile.metrics.MetricRegistry.Type.VENDOR;
+import org.glassfish.internal.data.ApplicationRegistry;
+import org.jvnet.hk2.config.ConfigListener;
+import org.jvnet.hk2.config.UnprocessedChangeEvent;
+import org.jvnet.hk2.config.UnprocessedChangeEvents;
 
 @Service(name = "microprofile-metrics-service")
 @RunLevel(StartupRunLevel.VAL)
-public class MetricsService implements EventListener, MonitoringDataSource {
+public class MetricsService implements EventListener, ConfigListener, MonitoringDataSource {
 
     private static final Logger LOGGER = Logger.getLogger(MetricsService.class.getName());
 
     @Inject
     Events events;
 
     @Inject
-    private ServerEnvironment serverEnv;
+    ApplicationRegistry applicationRegistry;
+
+    @Inject
+    MetricsServiceConfiguration configuration;
 
+    @Inject
+    private ServerEnvironment serverEnv;
+
     @Inject
     ServiceLocator serviceLocator;
 
@@ -123,6 +134,9 @@ public MetricsService() {
 
     @PostConstruct
     public void init() {
+        if(events == null){
+            events = Globals.getDefaultBaseServiceLocator().getService(Events.class);
+        }
         events.register(this);
         metricsServiceConfiguration = serviceLocator.getService(MetricsServiceConfiguration.class);
         // Only start if metrics are enabled
@@ -455,4 +469,14 @@ private void bootstrap() {
         checkSystemCpuLoadIssue(metadataConfig); // PAYARA 2938
         initMetadataConfig(metadataConfig.getBaseMetadata(), metadataConfig.getVendorMetadata(), false);
     }
+
+    @Override
+    public UnprocessedChangeEvents changed(PropertyChangeEvent[] events) {
+        List<UnprocessedChangeEvent> unchangedList = new ArrayList<>();
+        for(PropertyChangeEvent event : events) {
+                unchangedList.add(new UnprocessedChangeEvent(event, "Microprofile Metrics configuration changed:" + event.getPropertyName()
+                        + " was changed from " + event.getOldValue().toString() + " to " + event.getNewValue().toString()));
+            }
+        return new UnprocessedChangeEvents(unchangedList);
+    }
 }
diff --git a/...ics/src/main/java/fish/payara/microprofile/metrics/admin/MetricsServiceConfiguration.java b/...ics/src/main/java/fish/payara/microprofile/metrics/admin/MetricsServiceConfiguration.java
@@ -89,7 +89,7 @@ public interface MetricsServiceConfiguration extends ConfigBeanProxy, ConfigExte
     /**
      * @return a String value defines the attached virtual servers.
      */
-    @Attribute(dataType = String.class)
+    @Attribute(defaultValue = "", dataType = String.class)
     String getVirtualServers();
     void setVirtualServers(String value) throws PropertyVetoException;
 

diff --git a/.../src/main/java/fish/payara/microprofile/metrics/admin/SetMetricsConfigurationCommand.java b/.../src/main/java/fish/payara/microprofile/metrics/admin/SetMetricsConfigurationCommand.java
@@ -62,8 +62,10 @@
 import static org.glassfish.config.support.CommandTarget.STANDALONE_INSTANCE;
 import org.glassfish.config.support.TargetType;
 import org.glassfish.hk2.api.PerLookup;
+import org.glassfish.hk2.api.ServiceLocator;
 import org.glassfish.internal.api.Globals;
 import org.glassfish.internal.api.Target;
+import org.glassfish.internal.config.UnprocessedConfigListener;
 import org.jvnet.hk2.annotations.Service;
 import org.jvnet.hk2.config.ConfigSupport;
 import org.jvnet.hk2.config.TransactionFailure;
@@ -109,6 +111,12 @@ public class SetMetricsConfigurationCommand extends SetSecureMicroprofileConfigu
     @Inject
     private Domain domain;
 
+    @Inject
+    UnprocessedConfigListener unprocessedListener;
+
+    @Inject
+    ServiceLocator habitat;
+
     @Override
     public void execute(AdminCommandContext context) {
         ActionReport actionReport = context.getActionReport();
@@ -133,51 +141,36 @@ public void execute(AdminCommandContext context) {
 
         try {
             ConfigSupport.apply(configProxy -> {
-                boolean restart = false;
                 if (dynamic != null) {
                     configProxy.setDynamic(dynamic.toString());
                 }
                 if (enabled != null) {
                     configProxy.setEnabled(enabled.toString());
-                    if ((dynamic != null && dynamic)
-                            || Boolean.valueOf(metricsConfiguration.getDynamic())) {
-                        metricsService.resetMetricsEnabledProperty();
-                    } else {
-                        restart = true;
-                    }
                 }
                 if (secure != null) {
                     actionReport.setMessage("--secureMetrics option is deprecated, replaced by --securityEnabled option.");
                     configProxy.setSecureMetrics(secure.toString());
-                    if ((dynamic != null && dynamic)
-                            || Boolean.valueOf(metricsConfiguration.getDynamic())) {
-                        metricsService.resetMetricsSecureProperty();
-                    } else {
-                        restart = true;
-                    }
                 }
                 if (endpoint != null) {
                     configProxy.setEndpoint(endpoint);
-                    restart = true;
                 }
                 if (virtualServers != null) {
                     configProxy.setVirtualServers(virtualServers);
-                    restart = true;
                 }
                 if (securityEnabled != null) {
                     configProxy.setSecurityEnabled(securityEnabled.toString());
-                    restart = true;
                 }
                 if (roles != null) {
                     configProxy.setRoles(roles);
-                    restart = true;
                 }
 
-                if (restart) {
-                    actionReport.setMessage("Restart server for change to take effect");
-                }
+                actionReport.setActionExitCode(ActionReport.ExitCode.SUCCESS);
                 return configProxy;
             }, metricsConfiguration);
+
+            if(metricsConfiguration != null && !Boolean.valueOf(metricsConfiguration.getDynamic())) {
+                actionReport.setMessage("Restart server for change to take effect");
+            }
         } catch (TransactionFailure ex) {
             actionReport.failure(LOGGER, "Failed to update Metrics configuration", ex);
         }
@@ -188,5 +181,4 @@ public void execute(AdminCommandContext context) {
         }
     }
 
-
 }