[GitHub] drill pull request #1020: DRILL-5921: Display counter metrics in table

[prasadns14]

Github user prasadns14 commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1020#discussion_r149881450
  
    --- Diff: exec/java-exec/src/main/resources/rest/metrics/metrics.ftl ---
    @@ -138,21 +154,14 @@
           });
         };
     
    -    function updateOthers(metrics) {
    -      $.each(["counters", "meters"], function(i, key) {
    -        if(! $.isEmptyObject(metrics[key])) {
    -          $("#" + key + "Val").html(JSON.stringify(metrics[key], null, 2));
    -        }
    -      });
    -    };
    -
         var update = function() {
           $.get("/status/metrics", function(metrics) {
             updateGauges(metrics.gauges);
             updateBars(metrics.gauges);
             if(! $.isEmptyObject(metrics.timers)) createTable(metrics.timers, 
"timers");
             if(! $.isEmptyObject(metrics.histograms)) 
createTable(metrics.histograms, "histograms");
    -        updateOthers(metrics);
    +        if(! $.isEmptyObject(metrics.counters)) 
createCountersTable(metrics.counters);
    +        if(! $.isEmptyObject(metrics.meters)) 
$("#metersVal").html(JSON.stringify(metrics.meters, null, 2));
    --- End diff --
    
    @arina-ielchiieva, please review
    
    1) Added three screenshots before (current behavior), snapshot1(reusing 
createTable function) and snapshot2 (new createCountersTable function)
    
    2) The function "createTable" lists the keys for each metric class, which 
looks good for "timers" and "histograms" as they have many keys. But the 
"counters" metric has only one key "count" (same as guages metric which has 
only key value). So, I have a added a new function to just list the class and 
count value.
    
    3) Currently we do not have any "meters" metric. So, I don't know how many 
keys "meters" metric will have. If we have multiple keys we can use createTable 
or else we may have to create a different function similar to 
createCountersTable. (depending on the key name)
    
    I personally think if we have a single key, we shouldn't use the 
createTable function. We should just display the class and the only key value.
    
    <img width="1068" alt="before" 
src="https://user-images.githubusercontent.com/4907022/32592926-b419532e-c4da-11e7-93f1-02514e51e28d.png";>
    <img width="961" alt="snapshot1" 
src="https://user-images.githubusercontent.com/4907022/32592927-b42e1b38-c4da-11e7-850e-32902b6eb6d0.png";>
    <img width="1078" alt="snapshot2" 
src="https://user-images.githubusercontent.com/4907022/32592928-b4438b30-c4da-11e7-97c8-edc0482b1924.png";>
    
    
    



---