Commit 23c1cfcc authored by Sean McGivern's avatar Sean McGivern 🎄

Return warnings for performance bar from backend

For each DetailedView subclass, we add a `warnings` array to:

1. The top-level response.
2. Each individual call under the `details` key.

We use the `.thresholds` hash on the DetailedView to determine what's a
warning. If that hash is empty (the default), then the warnings array
will always be empty.
parent 7671c592
......@@ -3,6 +3,24 @@
module Peek
module Views
class ActiveRecord < DetailedView
DEFAULT_THRESHOLDS = {
calls: 100,
duration: 3,
individual_call: 1
}.freeze
THRESHOLDS = {
production: {
calls: 100,
duration: 15,
individual_call: 5
}
}.freeze
def self.thresholds
@thresholds ||= THRESHOLDS.fetch(Rails.env.to_sym, DEFAULT_THRESHOLDS)
end
private
def setup_subscribers
......
......@@ -3,11 +3,16 @@
module Peek
module Views
class DetailedView < View
def self.thresholds
{}
end
def results
{
duration: formatted_duration,
duration: format_duration(duration),
calls: calls,
details: details
details: details,
warnings: warnings
}
end
......@@ -18,30 +23,48 @@ module Peek
private
def duration
detail_store.map { |entry| entry[:duration] }.sum # rubocop:disable CodeReuse/ActiveRecord
detail_store.map { |entry| entry[:duration] }.sum * 1000 # rubocop:disable CodeReuse/ActiveRecord
end
def calls
detail_store.count
end
def details
call_details
.sort { |a, b| b[:duration] <=> a[:duration] }
.map(&method(:format_call_details))
end
def warnings
[
warning_for(calls, self.class.thresholds[:calls], label: "#{key} calls"),
warning_for(duration, self.class.thresholds[:duration], label: "#{key} duration")
].flatten.compact
end
def call_details
detail_store
end
def format_call_details(call)
call.merge(duration: (call[:duration] * 1000).round(3))
end
duration = (call[:duration] * 1000).round(3)
def details
call_details
.sort { |a, b| b[:duration] <=> a[:duration] }
.map(&method(:format_call_details))
call.merge(duration: duration,
warnings: warning_for(duration, self.class.thresholds[:individual_call]))
end
def formatted_duration
ms = duration * 1000
def warning_for(actual, threshold, label: nil)
if threshold && actual > threshold
prefix = "#{label}: " if label
["#{prefix}#{actual} over #{threshold}"]
else
[]
end
end
def format_duration(ms)
if ms >= 1000
"%.2fms" % ms
else
......
......@@ -3,6 +3,24 @@
module Peek
module Views
class Gitaly < DetailedView
DEFAULT_THRESHOLDS = {
calls: 30,
duration: 1,
individual_call: 0.5
}.freeze
THRESHOLDS = {
production: {
calls: 30,
duration: 1,
individual_call: 0.5
}
}.freeze
def self.thresholds
@thresholds ||= THRESHOLDS.fetch(Rails.env.to_sym, DEFAULT_THRESHOLDS)
end
private
def duration
......
......@@ -12,7 +12,7 @@ module Peek
private
def duration
::Gitlab::RuggedInstrumentation.query_time
::Gitlab::RuggedInstrumentation.query_time_ms
end
def calls
......
# frozen_string_literal: true
require 'fast_spec_helper'
describe Peek::Views::DetailedView, :request_store do
context 'when a class defines thresholds' do
let(:threshold_view) do
Class.new(described_class) do
def self.thresholds
{
calls: 1,
duration: 10,
individual_call: 5
}
end
def key
'threshold-view'
end
end.new
end
context 'when the results exceed the calls threshold' do
before do
allow(threshold_view)
.to receive(:detail_store).and_return([{ duration: 0.001 }, { duration: 0.001 }])
end
it 'adds a warning to the results key' do
expect(threshold_view.results).to include(warnings: [a_string_matching('threshold-view calls')])
end
end
context 'when the results exceed the duration threshold' do
before do
allow(threshold_view)
.to receive(:detail_store).and_return([{ duration: 0.011 }])
end
it 'adds a warning to the results key' do
expect(threshold_view.results).to include(warnings: [a_string_matching('threshold-view duration')])
end
end
context 'when a single call exceeds the duration threshold' do
before do
allow(threshold_view)
.to receive(:detail_store).and_return([{ duration: 0.001 }, { duration: 0.006 }])
end
it 'adds a warning to that call detail entry' do
expect(threshold_view.results)
.to include(details: a_collection_containing_exactly(
{ duration: 1.0, warnings: [] },
{ duration: 6.0, warnings: ['6.0 over 5'] }
))
end
end
end
context 'when a view does not define thresholds' do
let(:no_threshold_view) { Class.new(described_class).new }
before do
allow(no_threshold_view)
.to receive(:detail_store).and_return([{ duration: 100 }, { duration: 100 }])
end
it 'does not add warnings to the top level' do
expect(no_threshold_view.results).to include(warnings: [])
end
it 'does not add warnings to call details entries' do
expect(no_threshold_view.results)
.to include(details: a_collection_containing_exactly(
{ duration: 100000, warnings: [] },
{ duration: 100000, warnings: [] }
))
end
end
end
......@@ -21,10 +21,10 @@ describe Peek::Views::RedisDetailed, :request_store do
expect(subject.results[:details].count).to eq(1)
expect(subject.results[:details].first)
.to eq({
cmd: expected,
duration: 1000
})
.to include({
cmd: expected,
duration: 1000
})
end
end
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment