Merge branch '329909-remove-track_highlight_timeouts-ff' into 'master'

Remove no-longer-required metrics in Gitlab::Highlight

See merge request gitlab-org/gitlab!82705

config/feature_flags/development/track_file_size_over_highlight_limit.yml

----
-name: track_file_size_over_highlight_limit
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61273
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/330374
-milestone: '13.12'
-type: development
-group: group::code review
-default_enabled: false

config/feature_flags/development/track_highlight_timeouts.yml

----
-name: track_highlight_timeouts
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60956
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329909
-milestone: '13.12'
-type: development
-group: group::code review
-default_enabled: false

lib/gitlab/highlight.rb

@@ -11,11 +11,7 @@ module Gitlab
     end
 
     def self.too_large?(size)
-      return false unless size.to_i > self.file_size_limit
-
-      over_highlight_size_limit.increment(source: "file size: #{self.file_size_limit}") if Feature.enabled?(:track_file_size_over_highlight_limit)
-
-      true
+      size.to_i > self.file_size_limit
     end
 
     attr_reader :blob_name
@@ -74,14 +70,10 @@ module Gitlab
     end
 
     def highlight_rich(text, continue: true)
-      add_highlight_attempt_metric
-
       tag = lexer.tag
       tokens = lexer.lex(text, continue: continue)
       Timeout.timeout(timeout_time) { @formatter.format(tokens, **context, tag: tag).html_safe }
     rescue Timeout::Error => e
-      add_highlight_timeout_metric
-
       Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
       highlight_plain(text)
     rescue StandardError
@@ -95,38 +87,5 @@ module Gitlab
     def link_dependencies(text, highlighted_text)
       Gitlab::DependencyLinker.link(blob_name, text, highlighted_text)
     end
-
-    def add_highlight_attempt_metric
-      return unless Feature.enabled?(:track_highlight_timeouts)
-
-      highlighting_attempt.increment(source: (@language || "undefined"))
-    end
-
-    def add_highlight_timeout_metric
-      return unless Feature.enabled?(:track_highlight_timeouts)
-
-      highlight_timeout.increment(source: Gitlab::Runtime.sidekiq? ? "background" : "foreground")
-    end
-
-    def highlighting_attempt
-      @highlight_attempt ||= Gitlab::Metrics.counter(
-        :file_highlighting_attempt,
-        'Counts the times highlighting has been attempted on a file'
-      )
-    end
-
-    def highlight_timeout
-      @highlight_timeout ||= Gitlab::Metrics.counter(
-        :highlight_timeout,
-        'Counts the times highlights have timed out'
-      )
-    end
-
-    def self.over_highlight_size_limit
-      @over_highlight_size_limit ||= Gitlab::Metrics.counter(
-        :over_highlight_size_limit,
-        'Count the times files have been over the highlight size limit'
-      )
-    end
   end
 end

spec/lib/gitlab/highlight_spec.rb

@@ -53,10 +53,6 @@ RSpec.describe Gitlab::Highlight do
         stub_config(extra: { 'maximum_text_highlight_size_kilobytes' => 0.0001 } ) # 1.024 bytes
       end
 
-      it 'increments the metric for oversized files' do
-        expect { result }.to change { over_highlight_size_limit('file size: 0.0001') }.by(1)
-      end
-
       it 'returns plain version for long content' do
         expect(result).to eq(%[<span id="LC1" class="line" lang="">(make-pathname :defaults name</span>\n<span id="LC2" class="line" lang="">:type "assem")</span>])
       end
@@ -126,79 +122,29 @@ RSpec.describe Gitlab::Highlight do
     end
 
     context 'timeout' do
-      subject { described_class.new('file.name', 'Contents') }
+      subject(:highlight) { described_class.new('file.rb', 'begin', language: 'ruby').highlight('Content') }
 
       it 'utilizes timeout for web' do
         expect(Timeout).to receive(:timeout).with(described_class::TIMEOUT_FOREGROUND).and_call_original
 
-        subject.highlight("Content")
+        highlight
       end
 
-      it 'utilizes longer timeout for sidekiq' do
-        allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true)
-        expect(Timeout).to receive(:timeout).with(described_class::TIMEOUT_BACKGROUND).and_call_original
+      it 'falls back to plaintext on timeout' do
+        allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
+        expect(Timeout).to receive(:timeout).and_raise(Timeout::Error)
 
-        subject.highlight("Content")
-      end
-    end
+        expect(Rouge::Lexers::PlainText).to receive(:lex).and_call_original
 
-    describe 'highlight timeouts' do
-      let(:result) { described_class.highlight(file_name, content, language: "ruby") }
-
-      context 'when there is an attempt' do
-        it "increments the attempt counter with a defined language" do
-          expect { result }.to change { highlight_attempt_total("ruby") }
-        end
-
-        it "increments the attempt counter with an undefined language" do
-          expect do
-            described_class.highlight(file_name, content)
-          end.to change { highlight_attempt_total("undefined") }
-        end
+        highlight
       end
 
-      context 'when there is a timeout error while highlighting' do
-        before do
-          allow(Timeout).to receive(:timeout).twice.and_raise(Timeout::Error)
-          # This is done twice because it's rescued first and then
-          # calls the original exception
-        end
-
-        it "increments the foreground counter if it's in the foreground" do
-          expect { result }
-            .to raise_error(Timeout::Error)
-            .and change { highlight_timeout_total('foreground') }.by(1)
-            .and not_change { highlight_timeout_total('background') }
-        end
-
-        it "increments the background counter if it's in the background" do
-          allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true)
+      it 'utilizes longer timeout for sidekiq' do
+        allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true)
+        expect(Timeout).to receive(:timeout).with(described_class::TIMEOUT_BACKGROUND).and_call_original
 
-          expect { result }
-            .to raise_error(Timeout::Error)
-            .and change { highlight_timeout_total('background') }.by(1)
-            .and not_change { highlight_timeout_total('foreground') }
-        end
+        highlight
       end
     end
   end
-
-  def highlight_timeout_total(source)
-    Gitlab::Metrics
-      .counter(:highlight_timeout, 'Counts the times highlights have timed out')
-      .get(source: source)
-  end
-
-  def highlight_attempt_total(source)
-    Gitlab::Metrics
-      .counter(:file_highlighting_attempt, 'Counts the times highlighting has been attempted on a file')
-      .get(source: source)
-  end
-
-  def over_highlight_size_limit(source)
-    Gitlab::Metrics
-      .counter(:over_highlight_size_limit,
-               'Count the times text has been over the highlight size limit')
-      .get(source: source)
-  end
 end