glassfish
  1. glassfish
  2. GLASSFISH-20696

The --target option related to the enable-monitoring and disable-monitoring can't work well

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 4.0_dev
    • Fix Version/s: 4.1
    • Component/s: monitoring
    • Labels:
      None
    • Environment:

      Win 7
      JDK1.7.0_21

      Description

      Here's my reproduced stpes:
      1). asadmin start-domain
      2). asadmin create-local-instance ins1
      3). asadmin start-instance ins1
      4). Open the Firefox and login the adminstrator console and you will found boh the ins1 and DAS are not monitored(This step is used to confirm both the monitor related to the ins1 and DAS are disabled)
      5). asadmin enable-monitoring --target ins1
      6). After step 5, both the the monitor related to the ins1 and DAS are enabled(I think it is illegal to enable the monitor related to the DAS here because we specify the value of --target to ins1).

      On the other hand, there's a similar bug when it comes to the command of disable-monitoring:
      7). After step 6, we should confirm both the the monitor related to the ins1 and DAS are enabled.
      8). asadmin disable-monitoring --target ins1
      9). After step 5, both the the monitor related to the ins1 and DAS are disabled(I think it is illegal to disable the monitor related to the DAS here because we specify the value of --target to ins1)

        Activity

        Hide
        Terry2013 added a comment -

        Hi Byron:
        About this bug, I create a patch.Could you confirm it?

        MonitoringBootstrap.java
        Index: MonitoringBootstrap.java
        ===================================================================
        --- MonitoringBootstrap.java	(revision 62298)
        +++ MonitoringBootstrap.java	(working copy)
        @@ -64,6 +64,7 @@
         import org.jvnet.hk2.annotations.Optional;
         
         import org.jvnet.hk2.annotations.Service;
        +import org.jvnet.hk2.config.ConfigBeanProxy;
         import org.jvnet.hk2.config.ConfigListener;
         import org.jvnet.hk2.config.UnprocessedChangeEvents;
         
        @@ -508,6 +509,10 @@
                     if(event == null)
                         continue;
         
        +            if (!isCurrentInstanceMatchingTarget(event)) {
        +                continue;
        +            }
        +
                     String propName = event.getPropertyName();
                     Object oldVal = event.getOldValue();
                     Object newVal = event.getNewValue();
        @@ -559,6 +564,32 @@
                return null;
             }
         
        +    private boolean isCurrentInstanceMatchingTarget(PropertyChangeEvent event) {
        +        // DAS receive all the events, so we need to figure out 
        +        // whether we should take action on DAS depending on the event.
        +
        +        if(serverEnv.isInstance()) {
        +            return true;
        +        } 
        +        
        +        ConfigBeanProxy proxy = (ConfigBeanProxy)(event.getSource());
        +        Config config;
        +        if(proxy instanceof MonitoringService) {
        +            config = (Config)proxy.getParent();
        +        } else {
        +            config = (Config)proxy.getParent().getParent();
        +        }       
        +        
        +        String name = config.getName();
        +
        +        // We think that DAS's config is always server-config.
        +        if(name.equalsIgnoreCase("server-config")) {
        +            return true;
        +        }
        +        
        +        return false;
        +    }
        +
        
        Show
        Terry2013 added a comment - Hi Byron: About this bug, I create a patch.Could you confirm it? MonitoringBootstrap.java Index: MonitoringBootstrap.java =================================================================== --- MonitoringBootstrap.java (revision 62298) +++ MonitoringBootstrap.java (working copy) @@ -64,6 +64,7 @@ import org.jvnet.hk2.annotations.Optional; import org.jvnet.hk2.annotations.Service; + import org.jvnet.hk2.config.ConfigBeanProxy; import org.jvnet.hk2.config.ConfigListener; import org.jvnet.hk2.config.UnprocessedChangeEvents; @@ -508,6 +509,10 @@ if (event == null ) continue ; + if (!isCurrentInstanceMatchingTarget(event)) { + continue ; + } + String propName = event.getPropertyName(); Object oldVal = event.getOldValue(); Object newVal = event.getNewValue(); @@ -559,6 +564,32 @@ return null ; } + private boolean isCurrentInstanceMatchingTarget(PropertyChangeEvent event) { + // DAS receive all the events, so we need to figure out + // whether we should take action on DAS depending on the event. + + if (serverEnv.isInstance()) { + return true ; + } + + ConfigBeanProxy proxy = (ConfigBeanProxy)(event.getSource()); + Config config; + if (proxy instanceof MonitoringService) { + config = (Config)proxy.getParent(); + } else { + config = (Config)proxy.getParent().getParent(); + } + + String name = config.getName(); + + // We think that DAS's config is always server-config. + if (name.equalsIgnoreCase( "server-config" )) { + return true ; + } + + return false ; + } +
        Hide
        Jeremy_Lv added a comment -

        Hi, Byron:
        I have tested the above modification and it seems fine, pls review whether the changes is fine?

        Jeremy

        Show
        Jeremy_Lv added a comment - Hi, Byron: I have tested the above modification and it seems fine, pls review whether the changes is fine? Jeremy
        Hide
        Jeremy_Lv added a comment -

        Checked in the changes as revision 62763.

        Show
        Jeremy_Lv added a comment - Checked in the changes as revision 62763.

          People

          • Assignee:
            Byron Nevins
            Reporter:
            Jeremy_Lv
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: