[WIP] Improve metrics UI with charts (#5467)

* Add multi-line chart for total execution time by job

* Fiddling with the UX

* Refactor metrics `top_jobs` query

* debugging

* revert debugging

* revert debugging

* Add failed and avg time, just one data table

* Add color swatch in data table

* Measure in seconds

* Fix duplicate color

* standard style

* Rename for clarity

* Bring back empty metrics test

* Execution time is not consistent, assert processed counts instead

* Only include top 5 in chart, change swatch element to checkbox

* Wire up the checkboxes to show/hide each job class on the chart

* The checkboxes should not appear disabled

* Ensure seconds for y-axis to match table and UX improvements

- All data shown on metrics page is now in seconds
- Tooltip now includes "UTC" with the time
- Tooltip rounds the number of seconds and includes "seconds"

* Show deploy marks in metrics chart

* Fix annotation position when updating datasets

* Remove deploy labels on chart

We shouldn't assume the first word of the label is the git SHA, and the label annotations were hacky anyway.

* tweaks

Co-authored-by: Mike Perham <mperham@gmail.com>

lib/sidekiq/metrics/query.rb

@@ -35,48 +35,41 @@ module Sidekiq
         @klass = nil
       end
 
-      # Get metric data from the last hour and roll it up
+      # Get metric data for all jobs from the last hour
-      # into top processed count and execution time based on class.
+      def top_jobs(minutes: 60)
-      def top_jobs
+        result = Result.new
-        resultset = {}
-        resultset[:date] = @time.to_date
-        resultset[:period] = :hour
-        resultset[:ends_at] = @time
-        time = @time
 
-        results = @pool.with do |conn|
+        time = @time
+        redis_results = @pool.with do |conn|
           conn.pipelined do |pipe|
-            resultset[:size] = 60
+            minutes.times do |idx|
-            60.times do |idx|
               key = "j|#{time.strftime("%Y%m%d")}|#{time.hour}:#{time.min}"
               pipe.hgetall key
+              result.prepend_bucket time
               time -= 60
             end
-            resultset[:starts_at] = time
           end
         end
 
-        t = Hash.new(0)
+        time = @time
-        klsset = Set.new
+        redis_results.each do |hash|
-        # merge the per-minute data into a totals hash for the hour
+          hash.each do |k, v|
-        results.each do |hash|
+            kls, metric = k.split("|")
-          hash.each { |k, v| t[k] = t[k] + v.to_i }
+            result.job_results[kls].add_metric metric, time, v.to_i
-          klsset.merge(hash.keys.map { |k| k.split("|")[0] })
+          end
-        end
+          time -= 60
-        resultset[:job_classes] = klsset.delete_if { |item| item.size < 3 }
-        resultset[:totals] = t
-        top = t.each_with_object({}) do |(k, v), memo|
-          (kls, metric) = k.split("|")
-          memo[metric] ||= Hash.new(0)
-          memo[metric][kls] = v
         end
 
-        sorted = {}
+        marks = @pool.with { |c| c.hgetall("#{@time.strftime("%Y%m%d")}-marks") }
-        top.each_pair do |metric, hash|
+        result_range = result.starts_at..result.ends_at
-          sorted[metric] = hash.sort_by { |k, v| v }.reverse.to_h
+        marks.each do |timestamp, label|
+          time = Time.parse(timestamp)
+          if result_range.cover? time
+            result.marks << MarkResult.new(time, label)
+          end
         end
-        resultset[:top_classes] = sorted
+
-        resultset
+        result
       end
 
       def for_job(klass)
@@ -119,6 +112,43 @@ module Sidekiq
         resultset[:data] = results
         resultset
       end
+
+      class Result < Struct.new(:starts_at, :ends_at, :size, :buckets, :job_results, :marks)
+        def initialize
+          super
+          self.buckets = []
+          self.marks = []
+          self.job_results = Hash.new { |h, k| h[k] = JobResult.new }
+        end
+
+        def prepend_bucket(time)
+          buckets.unshift time.strftime("%H:%M")
+          self.ends_at ||= time
+          self.starts_at = time
+        end
+      end
+
+      class JobResult < Struct.new(:series, :totals)
+        def initialize
+          super
+          self.series = Hash.new { |h, k| h[k] = {} }
+          self.totals = Hash.new(0)
+        end
+
+        def add_metric(metric, time, value)
+          totals[metric] += value
+          series[metric][time.strftime("%H:%M")] = value
+
+          # Include timing measurements in seconds for convenience
+          add_metric("s", time, value / 1000.0) if metric == "ms"
+        end
+      end
+
+      class MarkResult < Struct.new(:time, :label)
+        def bucket
+          time.strftime("%H:%M")
+        end
+      end
     end
   end
 end

lib/sidekiq/web/application.rb

@@ -62,7 +62,7 @@ module Sidekiq
 
     get "/metrics" do
       q = Sidekiq::Metrics::Query.new
-      @resultset = q.top_jobs
+      @query_result = q.top_jobs
       erb(:metrics)
     end
 

test/test_metrics.rb

@@ -91,9 +91,9 @@ describe Sidekiq::Metrics do
   describe "querying" do
     it "handles empty metrics" do
       q = Sidekiq::Metrics::Query.new(now: fixed_time)
-      rs = q.top_jobs
+      result = q.top_jobs
-      refute_nil rs
+      assert_equal 60, result.buckets.size
-      assert_equal 8, rs.size
+      assert_equal({}, result.job_results)
 
       q = Sidekiq::Metrics::Query.new(now: fixed_time)
       rs = q.for_job("DoesntExist")
@@ -103,17 +103,28 @@ describe Sidekiq::Metrics do
 
     it "fetches top job data" do
       create_known_metrics
+      d = Sidekiq::Metrics::Deploy.new
+      d.mark(at: fixed_time - 300, label: "cafed00d - some git summary line")
+
       q = Sidekiq::Metrics::Query.new(now: fixed_time)
-      rs = q.top_jobs
+      result = q.top_jobs
-      assert_equal Date.new(2022, 7, 22), rs[:date]
+      assert_equal fixed_time - 59 * 60, result.starts_at
-      assert_equal 2, rs[:job_classes].size
+      assert_equal fixed_time, result.ends_at
-      assert_equal "App::SomeJob", rs[:job_classes].first
+      assert_equal 1, result.marks.size
-      bucket = rs[:totals]
+      assert_equal "cafed00d - some git summary line", result.marks[0].label
-      refute_nil bucket
+      assert_equal "21:58", result.marks[0].bucket
-      assert_equal bucket.keys.sort, ["App::FooJob|ms", "App::FooJob|p", "App::SomeJob|f", "App::SomeJob|ms", "App::SomeJob|p"]
+
-      assert_equal 3, bucket["App::SomeJob|p"]
+      assert_equal 60, result.buckets.size
-      assert_equal 4, bucket["App::FooJob|p"]
+      assert_equal "21:04", result.buckets.first
-      assert_equal 1, bucket["App::SomeJob|f"]
+      assert_equal "22:03", result.buckets.last
+
+      assert_equal %w[App::SomeJob App::FooJob].sort, result.job_results.keys.sort
+      some_job_result = result.job_results["App::SomeJob"]
+      refute_nil some_job_result
+      assert_equal %w[p f ms s].sort, some_job_result.series.keys.sort
+      assert_equal %w[p f ms s].sort, some_job_result.totals.keys.sort
+      assert_equal 2, some_job_result.series.dig("p", "22:03")
+      assert_equal 3, some_job_result.totals["p"]
     end
 
     it "fetches job-specific data" do

web/assets/javascripts/metrics.js

@@ -0,0 +1,123 @@
+class MetricsChart {
+  constructor(id, options) {
+    this.ctx = document.getElementById(id);
+    this.series = options.series;
+    this.marks = options.marks;
+    this.labels = options.labels;
+    this.swatches = [];
+    this.fallbackColor = "#999";
+    this.colors = [
+      // Colors taken from https://www.chartjs.org/docs/latest/samples/utils.html
+      "#4dc9f6",
+      "#f67019",
+      "#f53794",
+      "#537bc4",
+      "#acc236",
+      "#166a8f",
+      "#00a950",
+      "#58595b",
+      "#8549ba",
+      "#991b1b",
+    ];
+
+    const datasets = Object.entries(this.series)
+      .filter(([kls, _]) => options.visible.includes(kls))
+      .map(([kls, _]) => this.dataset(kls));
+
+    this.chart = new Chart(this.ctx, {
+      type: "line",
+      data: { labels: this.labels, datasets: datasets },
+      options: this.chartOptions,
+    });
+
+    this.addMarksToChart();
+    this.chart.update();
+  }
+
+  registerSwatch(id) {
+    const el = document.getElementById(id);
+    el.onchange = () => this.toggle(el.value, el.checked);
+    this.swatches[el.value] = el;
+    this.updateSwatch(el.value);
+  }
+
+  updateSwatch(kls) {
+    const el = this.swatches[kls];
+    const ds = this.chart.data.datasets.find((ds) => ds.label == kls);
+    el.checked = !!ds;
+    el.style.color = ds ? ds.borderColor : null;
+  }
+
+  toggle(kls, visible) {
+    if (visible) {
+      this.chart.data.datasets.push(this.dataset(kls));
+    } else {
+      const i = this.chart.data.datasets.findIndex((ds) => ds.label == kls);
+      this.colors.unshift(this.chart.data.datasets[i].borderColor);
+      this.chart.data.datasets.splice(i, 1);
+    }
+
+    this.updateSwatch(kls);
+    this.chart.update();
+  }
+
+  dataset(kls) {
+    const color = this.colors.shift() || this.fallbackColor;
+
+    return {
+      label: kls,
+      data: this.series[kls],
+      borderColor: color,
+      backgroundColor: color,
+      borderWidth: 2,
+      pointRadius: 2,
+    };
+  }
+
+  addMarksToChart() {
+    this.marks.forEach(([bucket, label], i) => {
+      this.chart.options.plugins.annotation.annotations[`deploy-${i}`] = {
+        type: "line",
+        xMin: bucket,
+        xMax: bucket,
+        borderColor: "rgba(220, 38, 38, 0.4)",
+        borderWidth: 2,
+      };
+    });
+  }
+
+  get chartOptions() {
+    return {
+      aspectRatio: 4,
+      scales: {
+        y: {
+          beginAtZero: true,
+          title: {
+            text: "Total execution time (sec)",
+            display: true,
+          },
+        },
+      },
+      interaction: {
+        mode: "x",
+      },
+      plugins: {
+        legend: {
+          display: false,
+        },
+        tooltip: {
+          callbacks: {
+            title: (items) => `${items[0].label} UTC`,
+            label: (item) =>
+              `${item.dataset.label}: ${item.parsed.y.toFixed(1)} seconds`,
+            footer: (items) => {
+              const bucket = items[0].label;
+              const marks = this.marks.filter(([b, _]) => b == bucket);
+              return marks.map(([b, msg]) => `Deploy: ${msg}`);
+            },
+          },
+        },
+      },
+    };
+  }
+}

web/assets/stylesheets/application.css

@@ -954,3 +954,37 @@ div.interval-slider input {
   padding: 3px 7px;
   margin-left: 5px;
 }
+
+.metrics-swatch-wrapper {
+  display: flex;
+  align-items: center;
+  gap: 6px;
+}
+
+.metrics-swatch[type=checkbox] {
+  display: inline-block;
+  width: 16px;
+  height: 16px;
+  margin: 0;
+  border-radius: 2px;
+  appearance: none;
+  -webkit-appearance: none;
+  -moz-appearance: none;
+  border: 1px solid #bbb;
+  color: white;
+  background-color: currentColor;
+}
+
+/* We need to add the checkmark since we've taken over the appearance */
+.metrics-swatch[type=checkbox]:checked {
+  border-color: currentColor;
+  background-image: url("data:image/svg+xml,%3csvg viewBox='0 0 16 16' fill='white' xmlns='http://www.w3.org/2000/svg'%3e%3cpath d='M12.207 4.793a1 1 0 010 1.414l-5 5a1 1 0 01-1.414 0l-2-2a1 1 0 011.414-1.414L6.5 9.086l4.293-4.293a1 1 0 011.414 0z'/%3e%3c/svg%3e");
+  background-size: 100% 100%;
+  background-position: center;
+  background-repeat: no-repeat;
+}
+
+.metrics-swatch[type=checkbox]:focus {
+  outline: 1px solid #888;
+  outline-offset: 2px;
+}

web/locales/en.yml

@@ -86,5 +86,6 @@ en: # <---- change this to your locale code
   Unpause: Unpause
   Metrics: Metrics
   NoDataFound: No data found
-  ExecutionTime: Execution Time
+  ExecutionTime: Total Execution Time
+  AvgExecutionTime: Average Execution Time
   Context: Context

web/views/metrics.erb

@@ -1,59 +1,69 @@
+<script type="text/javascript" src="<%= root_path %>javascripts/chart.min.js"></script>
+<script type="text/javascript" src="<%= root_path %>javascripts/chartjs-plugin-annotation.min.js"></script>
+<script type="text/javascript" src="<%= root_path %>javascripts/metrics.js"></script>
 
-<h1><%= t('Metrics') %></h1>
+<h3><%= t('Metrics') %></h3>
 
-<h3>Top Jobs by Processed Count</h3>
+<%
-<% top = @resultset[:top_classes] %>
+  table_limit = 20
+  chart_limit = 5
+  job_results = @query_result.job_results.sort_by { |(kls, jr)| jr.totals["s"] }.reverse.first(table_limit)
+  visible_kls = job_results.first(chart_limit).map(&:first)
+%>
+
+<canvas id="metrics-chart"></canvas>
+
+<script>
+  window.metricsChart = new MetricsChart(
+    "metrics-chart",
+    <%= Sidekiq.dump_json({
+      series: job_results.map { |(kls, jr)| [kls, jr.dig("series", "s")] }.to_h,
+      marks: @query_result.marks.map { |m| [m.bucket, m.label] },
+      visible: visible_kls,
+      labels: @query_result.buckets,
+    }) %>
+  )
+</script>
+
+<h3>Most Time-Consuming Jobs</h3>
 
-<% topp = top["p"]&.first(10) %>
 <div class="table_container">
   <table class="table table-bordered table-striped table-hover">
     <tbody>
       <tr>
         <th><%= t('Name') %></th>
         <th><%= t('Processed') %></th>
+        <th><%= t('Failed') %></th>
         <th><%= t('ExecutionTime') %></th>
+        <th><%= t('AvgExecutionTime') %></th>
       </tr>
-      <% if topp %>
+      <% if job_results.any? %>
-        <% topp.each do |kls, val| %>
+        <% job_results.each_with_index do |(kls, jr), i| %>
           <tr>
-            <td><code><a href="<%= root_path %>metrics/<%= kls %>"><%= kls %></a></code></td>
+            <td>
-            <td><%= val %></td>
+              <div class="metrics-swatch-wrapper">
-            <td><%= top.dig("ms", kls) %></td>
+                <% id = "metrics-swatch-#{kls}" %>
+                <input
+                  type="checkbox"
+                  id="<%= id %>"
+                  class="metrics-swatch"
+                  value="<%= kls %>"
+                />
+                <code><a href="<%= root_path %>metrics/<%= kls %>"><%= kls %></a></code>
+              </div>
+              <script>metricsChart.registerSwatch("<%= id %>")</script>
+            </td>
+            <td><%= jr.dig("totals", "p") %></td>
+            <td><%= jr.dig("totals", "f") %></td>
+            <td><%= jr.dig("totals", "s").round(0) %> seconds</td>
+            <td><%= (jr.dig("totals", "s") / jr.dig("totals", "p")).round(2) %> seconds</td>
           </tr>
         <% end %>
       <% else %>
-          <tr><td colspan=3><%= t("NoDataFound") %></td></tr>
+          <tr><td colspan=5><%= t("NoDataFound") %></td></tr>
       <% end %>
     </tbody>
   </table>
 </div>
 
-<h3>Top Jobs by Execution Time</h3>
+<p><small>Data from <%= @query_result.starts_at %> to <%= @query_result.ends_at %></small></p>
-
-<% topms = top["ms"]&.first(10) %>
-<div class="table_container">
-  <table class="table table-bordered table-striped table-hover">
-    <tbody>
-      <tr>
-        <th><%= t('Name') %></th>
-        <th><%= t('Processed') %></th>
-        <th><%= t('ExecutionTime') %></th>
-      </tr>
-      <% if topms %>
-        <% topms.each do |kls, val| %>
-          <tr>
-            <td><code><a href="<%= root_path %>metrics/<%= kls %>"><%= kls %></a></code></td>
-            <td><%= top.dig("p", kls) %></td>
-            <td><%= val %></td>
-          </tr>
-        <% end %>
-      <% else %>
-          <tr><td colspan=3><%= t("NoDataFound") %></td></tr>
-      <% end %>
-    </tbody>
-  </table>
-</div>
-
-<p>
-Data from <%= @resultset[:starts_at] %> to <%= @resultset[:ends_at] %>
-</p>