Merge branch '31286-add-monitoring-for-diff-limits' into 'master'

Resolve "Add monitoring for diff limits"

Closes #31286

See merge request gitlab-org/gitlab!20022

app/helpers/diff_helper.rb

@@ -161,6 +161,18 @@ module DiffHelper
     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
 
   def diff_btn(title, name, selected)
@@ -203,12 +215,6 @@ module DiffHelper
     link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class]
   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)
     path = diff_file.new_path
 

lib/gitlab/diff/file_collection/merge_request_diff.rb

@@ -4,12 +4,16 @@ module Gitlab
   module Diff
     module FileCollection
       class MergeRequestDiff < MergeRequestDiffBase
+        include Gitlab::Utils::StrongMemoize
+
         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
 
         override :write_cache

spec/helpers/diff_helper_spec.rb

@@ -258,6 +258,51 @@ describe DiffHelper do
     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
     it 'returns full path by default' do
       expect(diff_file_path_text(diff_file)).to eq(diff_file.new_path)