glassfish
  1. glassfish
  2. GLASSFISH-17777

localization issue with get-health command for REST client

    Details

      Description

      Current get-health Properties field in the action report doesn't allow for localization. I just realize that when trying to add this info to the admin console.
      Current action report says:

      {"message":"ABC-1 stopped since Sun Nov 20 12:02:12 PST 2011\nABC-3 started since Sun Nov 20 12:00:20 PST 2011\nABC-4 failed since Sun Nov 20 12:03:07 PST 2011\nABC-5 not started","command":"get-health AdminCommand","exit_code":"SUCCESS",
      "properties":

      { "ABC-5":"not started", "ABC-4":"failed since Sun Nov 20 12:03:07 PST 201", "ABC-3":"started since Sun Nov 20 12:00:20 PST 201", "ABC-1":"stopped since Sun Nov 20 12:02:12 PST 201"}

      ,"
      extraProperties":{"methods":[

      {"name":"GET"}

      ,{}]}}

      This probably returns based on the locale that the server is running, but the console maybe running on a different browser locale.
      This make it hard to localize it. Also, it maybe better to separate the Status (failed, started, not started) etc. from the actual data (time).
      Maybe properties can be a List of Map.
      The Map has the following key:

      Name: instance name
      Status: Started, Stopped, Not Started, Failed etc. (an enum will be good)
      timestamp: the time expressed as a Long, so that it can be converted back to Date.

      This is similar to the list-instances command. You can try :4848/management/domain/list-instances.json

      Sorry i didn't realize there maybe an issue when suggesting the fix before.

      As of now, the get-health info is displayed in the Instance General Info page, which may have i18n issue. When this bug is fixed, console code will change to extract the info differently.

        Activity

        Hide
        Bobby Bissett added a comment -

        Hi Anissa,

        For the state, we have an Enum of states already in HealthHistory.STATE in the gms-bootstrap module. Can you use those? I'll have to express the timestamp as String.valueOf(long) so that it can be stored, but that's simple enough for you to change back to long.

        I'm not clear at all about how to stuff all this into properties in the report.getTopMessagePart() object. I know how to express the List of Maps of List of Maps as json or xml, but not how to get it into the report. Example of what I mean in json:

        "properties":{ "ABC-1": { "Status" : "foo", "timestamp": "123" }}

        Instead, could I store the props with a separator between the state and timestamp? I could do something like this in the get health command code:

        top.addProperty(instanceName, health.state.name() + ":" + String.valueOf(health.time))

        ...and the result should be something like this:

        "properties":

        { "ABC-1": "RUNNING:12345678", "ABC-2": "FAILURE:12345678", "ABC-3": "NOT_RUNNING:"}

        Note that there's no time associated with the NOT_RUNNING state, so I can either leave it empty after the colon or use a flag like -1. Probably empty is better so you can use String.isEmpty() to check it. Let me know what you'd prefer.

        Thanks,
        Bobby

        Show
        Bobby Bissett added a comment - Hi Anissa, For the state, we have an Enum of states already in HealthHistory.STATE in the gms-bootstrap module. Can you use those? I'll have to express the timestamp as String.valueOf(long) so that it can be stored, but that's simple enough for you to change back to long. I'm not clear at all about how to stuff all this into properties in the report.getTopMessagePart() object. I know how to express the List of Maps of List of Maps as json or xml, but not how to get it into the report. Example of what I mean in json: "properties":{ "ABC-1": { "Status" : "foo", "timestamp": "123" }} Instead, could I store the props with a separator between the state and timestamp? I could do something like this in the get health command code: top.addProperty(instanceName, health.state.name() + ":" + String.valueOf(health.time)) ...and the result should be something like this: "properties": { "ABC-1": "RUNNING:12345678", "ABC-2": "FAILURE:12345678", "ABC-3": "NOT_RUNNING:"} Note that there's no time associated with the NOT_RUNNING state, so I can either leave it empty after the colon or use a flag like -1. Probably empty is better so you can use String.isEmpty() to check it. Let me know what you'd prefer. Thanks, Bobby
        Hide
        Anissa Lam added a comment - - edited

        Having:

        "properties": [
                    {  "name" : "ABC-1",
                       "status" : "RUNNING",
                       "time": "12345678"
                    },
        
                    {  "name" : "ABC-2",
                       "status" : "FAILURE",
                       "time": "12345678"
                    },
        
        	   {  "name" : "ABC-3",
                       "status" : "NOT-RUNNING",
                       "time": ""
                    },
        }
        can be parsed much easier than
        
        "properties":{ "ABC-1": "RUNNING:12345678", "ABC-2": "FAILURE:12345678", "ABC-3": "NOT_RUNNING:"}
        

        It can be in the extraProperties like the list-instances command instead of using properties.
        Can you take a look at how list-instances is done and do the same ?
        thanks

        Show
        Anissa Lam added a comment - - edited Having: "properties" : [ { "name" : "ABC-1" , "status" : "RUNNING" , "time" : "12345678" }, { "name" : "ABC-2" , "status" : "FAILURE" , "time" : "12345678" }, { "name" : "ABC-3" , "status" : "NOT-RUNNING" , "time" : "" }, } can be parsed much easier than "properties" :{ "ABC-1" : "RUNNING:12345678" , "ABC-2" : "FAILURE:12345678" , "ABC-3" : "NOT_RUNNING:" } It can be in the extraProperties like the list-instances command instead of using properties. Can you take a look at how list-instances is done and do the same ? thanks
        Hide
        Bobby Bissett added a comment -

        Ok, I think I have it. This work for you?

        {"message":"inst1 started since Mon Nov 21 13:42:39 EST 2011\ninst2 stopped since Mon Nov 21 15:46:09 EST 2011\ninst3 not started","command":"get-health AdminCommand","exit_code":"SUCCESS","extraProperties":{"methods":[

        {"name":"GET"}

        ,{}],"instances":[

        {"status":"RUNNING","name":"inst1","time":"1321900959398"}

        ,

        {"status":"SHUTDOWN","name":"inst2","time":"1321908369298"}

        ,

        {"status":"NOT_RUNNING","name":"inst3","time":""}

        ]}}

        So in extraProperties, the key "instances" is mapped to a list of maps containing the name, status, and time for each instance. The status matches the enum mentioned above.

        Show
        Bobby Bissett added a comment - Ok, I think I have it. This work for you? {"message":"inst1 started since Mon Nov 21 13:42:39 EST 2011\ninst2 stopped since Mon Nov 21 15:46:09 EST 2011\ninst3 not started","command":"get-health AdminCommand","exit_code":"SUCCESS","extraProperties":{"methods":[ {"name":"GET"} ,{}],"instances":[ {"status":"RUNNING","name":"inst1","time":"1321900959398"} , {"status":"SHUTDOWN","name":"inst2","time":"1321908369298"} , {"status":"NOT_RUNNING","name":"inst3","time":""} ]}} So in extraProperties, the key "instances" is mapped to a list of maps containing the name, status, and time for each instance. The status matches the enum mentioned above.
        Hide
        Anissa Lam added a comment -

        yes, that will work. thanks.

        Show
        Anissa Lam added a comment - yes, that will work. thanks.
        Hide
        Bobby Bissett added a comment -

        Just commited fix in 3.1.2 branch. Will fix in trunk as well and mark fixed here.

        Show
        Bobby Bissett added a comment - Just commited fix in 3.1.2 branch. Will fix in trunk as well and mark fixed here.
        Hide
        Bobby Bissett added a comment -

        Changes checked into revisions 51038 (3.1.2 branch) and 51054 (trunk).

        Show
        Bobby Bissett added a comment - Changes checked into revisions 51038 (3.1.2 branch) and 51054 (trunk).

          People

          • Assignee:
            Bobby Bissett
            Reporter:
            Anissa Lam
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: