drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From arina-ielchiieva <...@git.apache.org>
Subject [GitHub] drill pull request #1020: DRILL-5921: Display counter metrics in table
Date Thu, 09 Nov 2017 08:27:26 GMT
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1020#discussion_r149891566
  
    --- 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 --
    
    @prasadns14
    1. Thanks for adding the screenshots.
    2. Most of the code in `createTable` and `createCountersTable` coincide.  I suggested
you make one function. For example, with three parameters, `createTable(metric, name, addReportingClass)`.
When you don't need to add reporting class you'll call this method with false. Our goal here
is generify existing methods rather then adding new specific with almost the same content.
    3. If we don't have any meters, let's remove them.



---

Mime
View raw message