Commit d3802036 authored by David KIm's avatar David KIm Committed by Nick Thomas

Resolve "Add monitoring for diff limits"

parent 73e95ecf
...@@ -161,6 +161,18 @@ module DiffHelper ...@@ -161,6 +161,18 @@ module DiffHelper
end end
end end
def render_overflow_warning?(diffs_collection)
diff_files = diffs_collection.diff_files
if diff_files.any?(&:too_large?)
Gitlab::Metrics.add_event(:diffs_overflow_single_file_limits)
end
diff_files.overflow?.tap do |overflown|
Gitlab::Metrics.add_event(:diffs_overflow_collection_limits) if overflown
end
end
private private
def diff_btn(title, name, selected) def diff_btn(title, name, selected)
...@@ -203,12 +215,6 @@ module DiffHelper ...@@ -203,12 +215,6 @@ module DiffHelper
link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class] link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class]
end end
def render_overflow_warning?(diffs_collection)
diffs = @merge_request_diff.presence || diffs_collection.diff_files
diffs.overflow?
end
def diff_file_path_text(diff_file, max: 60) def diff_file_path_text(diff_file, max: 60)
path = diff_file.new_path path = diff_file.new_path
......
...@@ -4,12 +4,16 @@ module Gitlab ...@@ -4,12 +4,16 @@ module Gitlab
module Diff module Diff
module FileCollection module FileCollection
class MergeRequestDiff < MergeRequestDiffBase class MergeRequestDiff < MergeRequestDiffBase
include Gitlab::Utils::StrongMemoize
def diff_files def diff_files
diff_files = super strong_memoize(:diff_files) do
diff_files = super
diff_files.each { |diff_file| cache.decorate(diff_file) } diff_files.each { |diff_file| cache.decorate(diff_file) }
diff_files diff_files
end
end end
override :write_cache override :write_cache
......
...@@ -258,6 +258,51 @@ describe DiffHelper do ...@@ -258,6 +258,51 @@ describe DiffHelper do
end end
end end
context '#render_overflow_warning?' do
let(:diffs_collection) { instance_double(Gitlab::Diff::FileCollection::MergeRequestDiff, diff_files: diff_files) }
let(:diff_files) { Gitlab::Git::DiffCollection.new(files) }
let(:safe_file) { { too_large: false, diff: '' } }
let(:large_file) { { too_large: true, diff: '' } }
let(:files) { [safe_file, safe_file] }
before do
allow(diff_files).to receive(:overflow?).and_return(false)
end
context 'when neither collection nor individual file hit the limit' do
it 'returns false and does not log any overflow events' do
expect(Gitlab::Metrics).not_to receive(:add_event).with(:diffs_overflow_collection_limits)
expect(Gitlab::Metrics).not_to receive(:add_event).with(:diffs_overflow_single_file_limits)
expect(render_overflow_warning?(diffs_collection)).to be false
end
end
context 'when the file collection has an overflow' do
before do
allow(diff_files).to receive(:overflow?).and_return(true)
end
it 'returns false and only logs collection overflow event' do
expect(Gitlab::Metrics).to receive(:add_event).with(:diffs_overflow_collection_limits).exactly(:once)
expect(Gitlab::Metrics).not_to receive(:add_event).with(:diffs_overflow_single_file_limits)
expect(render_overflow_warning?(diffs_collection)).to be true
end
end
context 'when two individual files are too big' do
let(:files) { [safe_file, large_file, large_file] }
it 'returns false and only logs single file overflow events' do
expect(Gitlab::Metrics).to receive(:add_event).with(:diffs_overflow_single_file_limits).exactly(:once)
expect(Gitlab::Metrics).not_to receive(:add_event).with(:diffs_overflow_collection_limits)
expect(render_overflow_warning?(diffs_collection)).to be false
end
end
end
context '#diff_file_path_text' do context '#diff_file_path_text' do
it 'returns full path by default' do it 'returns full path by default' do
expect(diff_file_path_text(diff_file)).to eq(diff_file.new_path) expect(diff_file_path_text(diff_file)).to eq(diff_file.new_path)
......
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