Mailing-List: contact dev-help@drill.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@drill.apache.org
From: arina-ielchiieva <git@git.apache.org>
To: dev@drill.apache.org
Reply-To: dev@drill.apache.org
References: <git-pr-1020-drill@git.apache.org>
In-Reply-To: <git-pr-1020-drill@git.apache.org>
Subject: [GitHub] drill pull request #1020: DRILL-5921: Display counter metrics in table
Content-Type: text/plain
Message-Id: <20171109082726.AE6B4DFCF9@git1-us-west.apache.org>
Date: Thu,  9 Nov 2017 08:27:26 +0000 (UTC)
archived-at: Thu, 09 Nov 2017 08:27:28 -0000

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.


---