diff --git a/app/presenters/blob_presenter.rb b/app/presenters/blob_presenter.rb
index c52fc168c55e9057cd52f20b4ced4ebf45d6f0a7..087fb8bf201fccfc443a99b40e565c183330f1a0 100644
--- a/app/presenters/blob_presenter.rb
+++ b/app/presenters/blob_presenter.rb
@@ -22,14 +22,15 @@ def highlight_and_trim(ellipsis_svg:, trim_length:, plain: nil)
)
end
- def highlight(to: nil, plain: nil)
+ def highlight(to: nil, plain: nil, used_on: :blob)
load_all_blob_data
Gitlab::Highlight.highlight(
blob.path,
blob_data(to),
language: blob_language,
- plain: plain
+ plain: plain,
+ used_on: used_on
)
end
diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md
index 80121c7c235040e23bb5acdea0d5d8ab60caa882..f1d1b504c9d1eaa682c567d989ce8a77a04713ee 100644
--- a/doc/administration/monitoring/prometheus/gitlab_metrics.md
+++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md
@@ -180,6 +180,8 @@ The following metrics are available:
| `gitlab_connection_pool_size` | Gauge | 16.7 | Size of connection pool |
| `gitlab_connection_pool_available_count` | Gauge | 16.7 | Number of available connections in the pool |
| `gitlab_security_policies_scan_result_process_duration_seconds` | Histogram | 16.7 | The amount of time to process scan result policies |
+| `gitlab_highlight_usage` | Counter | 16.8 | The number of times `Gitlab::Highlight` is used | `used_on` |
+| `dependency_linker_usage` | Counter | 16.8 | The number of times dependency linker is used | `used_on` |
## Metrics controlled by a feature flag
diff --git a/lib/gitlab/dependency_linker.rb b/lib/gitlab/dependency_linker.rb
index db60128b97924af02a9629f47b0e0e66344574d1..d4178173e037ffa7a8e76d17db1a1fec47ccc580 100644
--- a/lib/gitlab/dependency_linker.rb
+++ b/lib/gitlab/dependency_linker.rb
@@ -22,11 +22,19 @@ def self.linker(blob_name)
LINKERS.find { |linker| linker.support?(blob_name) }
end
- def self.link(blob_name, plain_text, highlighted_text)
+ def self.link(blob_name, plain_text, highlighted_text, used_on: :blob)
linker = linker(blob_name)
return highlighted_text unless linker
+ usage_counter.increment(used_on: used_on)
linker.link(plain_text, highlighted_text)
end
+
+ def self.usage_counter
+ Gitlab::Metrics.counter(
+ :dependency_linker_usage,
+ 'The number of times dependency linker is used'
+ )
+ end
end
end
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index d2524ae17617e8f342f4d94df7fa79bced2284cb..39da9c4e7c8876c1e0f71500a62879e5e420ce3b 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -90,7 +90,8 @@ def diff_line_highlighting(diff_line, plain: false)
rich_line = syntax_highlighter(diff_line).highlight(
diff_line.text(prefix: false),
plain: plain,
- context: { line_number: diff_line.line }
+ context: { line_number: diff_line.line },
+ used_on: :diff
)
# Only update text if line is found. This will prevent
@@ -143,7 +144,7 @@ def highlighted_blob_lines(blob)
blob.load_all_data!
- blob.present.highlight.lines
+ blob.present.highlight(used_on: :diff).lines
end
def blobs_too_large?
diff --git a/lib/gitlab/highlight.rb b/lib/gitlab/highlight.rb
index 1a85c57e6b155d348ba5f1de6346aeadf63d6062..40bca7993ce8726d1d427aca0c533a975bfa8188 100644
--- a/lib/gitlab/highlight.rb
+++ b/lib/gitlab/highlight.rb
@@ -2,9 +2,9 @@
module Gitlab
class Highlight
- def self.highlight(blob_name, blob_content, language: nil, plain: false, context: {})
+ def self.highlight(blob_name, blob_content, language: nil, plain: false, context: {}, used_on: :blob)
new(blob_name, blob_content, language: language)
- .highlight(blob_content, continue: false, plain: plain, context: context)
+ .highlight(blob_content, continue: false, plain: plain, context: context, used_on: used_on)
end
def self.too_large?(size)
@@ -18,15 +18,19 @@ def initialize(blob_name, blob_content, language: nil)
@language = language
@blob_name = blob_name
@blob_content = blob_content
+ @gitlab_highlight_usage_counter = Gitlab::Metrics.counter(
+ :gitlab_highlight_usage,
+ 'The number of times Gitlab::Highlight is used'
+ )
end
- def highlight(text, continue: false, plain: false, context: {})
+ def highlight(text, continue: false, plain: false, context: {}, used_on: :blob)
@context = context
plain ||= self.class.too_large?(text.length)
- highlighted_text = highlight_text(text, continue: continue, plain: plain)
- highlighted_text = link_dependencies(text, highlighted_text) if blob_name
+ highlighted_text = highlight_text(text, continue: continue, plain: plain, used_on: used_on)
+ highlighted_text = link_dependencies(text, highlighted_text, used_on: used_on) if blob_name
highlighted_text
end
@@ -54,7 +58,9 @@ def custom_language
Rouge::Lexer.find_fancy(@language)
end
- def highlight_text(text, continue: true, plain: false)
+ def highlight_text(text, continue: true, plain: false, used_on: :blob)
+ @gitlab_highlight_usage_counter.increment(used_on: used_on)
+
if plain
highlight_plain(text)
else
@@ -77,8 +83,8 @@ def highlight_rich(text, continue: true)
highlight_plain(text)
end
- def link_dependencies(text, highlighted_text)
- Gitlab::DependencyLinker.link(blob_name, text, highlighted_text)
+ def link_dependencies(text, highlighted_text, used_on: :blob)
+ Gitlab::DependencyLinker.link(blob_name, text, highlighted_text, used_on: used_on)
end
end
end
diff --git a/spec/lib/gitlab/dependency_linker_spec.rb b/spec/lib/gitlab/dependency_linker_spec.rb
index 8feab0f8017c9adefeb04e4c8ecf873e79fd0670..4da0b9d8c0da88d6096510c8cd08ea4c6c887b08 100644
--- a/spec/lib/gitlab/dependency_linker_spec.rb
+++ b/spec/lib/gitlab/dependency_linker_spec.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+require 'spec_helper'
RSpec.describe Gitlab::DependencyLinker do
describe '.link' do
@@ -107,5 +107,16 @@
described_class.link(blob_name, nil, nil)
end
+
+ it 'increments usage counter based on specified used_on', :prometheus do
+ allow(described_class::GemfileLinker).to receive(:link)
+
+ described_class.link('Gemfile', nil, nil, used_on: :diff)
+
+ dependency_linker_usage_counter = Gitlab::Metrics.registry.get(:dependency_linker_usage)
+
+ expect(dependency_linker_usage_counter.get(used_on: :diff)).to eq(1)
+ expect(dependency_linker_usage_counter.get(used_on: :blob)).to eq(0)
+ end
end
end
diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb
index e65f5a618a5ee117ecc6358f63e316fbfd1af2a2..e9e65f648872f9e0e43a79bff51003ed504e0a80 100644
--- a/spec/lib/gitlab/diff/highlight_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_spec.rb
@@ -25,119 +25,131 @@
end
describe '#highlight' do
- context "with a diff file" do
- let(:subject) { described_class.new(diff_file, repository: project.repository).highlight }
+ shared_examples_for 'diff highlighter' do
+ context "with a diff file" do
+ let(:subject) { described_class.new(diff_file, repository: project.repository).highlight }
- it 'returns Gitlab::Diff::Line elements' do
- expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
- end
+ it 'returns Gitlab::Diff::Line elements' do
+ expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
+ end
- it 'does not modify "match" lines' do
- expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
- expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
- end
+ it 'does not modify "match" lines' do
+ expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
+ expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
+ end
- it 'highlights and marks unchanged lines' do
- code = %{ <span id="LC7" class="line" lang="ruby"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>\n}
+ it 'highlights and marks unchanged lines' do
+ code = %{ <span id="LC7" class="line" lang="ruby"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>\n}
- expect(subject[2].rich_text).to eq(code)
- end
+ expect(subject[2].rich_text).to eq(code)
+ end
- it 'highlights and marks removed lines' do
- code = %(-<span id="LC9" class="line" lang="ruby"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span>\n)
+ it 'highlights and marks removed lines' do
+ code = %(-<span id="LC9" class="line" lang="ruby"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span>\n)
- expect(subject[4].rich_text).to eq(code)
- end
+ expect(subject[4].rich_text).to eq(code)
+ end
- it 'highlights and marks added lines' do
- code = %(+<span id="LC9" class="line" lang="ruby"> <span class="k">raise</span> <span class="no"><span class="idiff left addition">RuntimeError</span></span><span class="p"><span class="idiff addition">,</span></span><span class="idiff right addition"> </span><span class="s2">"System commands must be given as an array of strings"</span></span>\n)
+ it 'highlights and marks added lines' do
+ code = %(+<span id="LC9" class="line" lang="ruby"> <span class="k">raise</span> <span class="no"><span class="idiff left addition">RuntimeError</span></span><span class="p"><span class="idiff addition">,</span></span><span class="idiff right addition"> </span><span class="s2">"System commands must be given as an array of strings"</span></span>\n)
+
+ expect(subject[5].rich_text).to eq(code)
+ end
- expect(subject[5].rich_text).to eq(code)
+ context 'when no diff_refs' do
+ before do
+ allow(diff_file).to receive(:diff_refs).and_return(nil)
+ end
+
+ context 'when no inline diffs' do
+ it_behaves_like 'without inline diffs'
+ end
+ end
end
- context 'when no diff_refs' do
- before do
- allow(diff_file).to receive(:diff_refs).and_return(nil)
+ context "with diff lines" do
+ let(:subject) { described_class.new(diff_file.diff_lines, repository: project.repository).highlight }
+
+ it 'returns Gitlab::Diff::Line elements' do
+ expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
end
- context 'when no inline diffs' do
- it_behaves_like 'without inline diffs'
+ it 'does not modify "match" lines' do
+ expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
+ expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
end
- end
- end
- context "with diff lines" do
- let(:subject) { described_class.new(diff_file.diff_lines, repository: project.repository).highlight }
+ it 'marks unchanged lines' do
+ code = %q{ def popen(cmd, path=nil)}
- it 'returns Gitlab::Diff::Line elements' do
- expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
- end
+ expect(subject[2].text).to eq(code)
+ expect(subject[2].text).not_to be_html_safe
+ end
- it 'does not modify "match" lines' do
- expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
- expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
- end
+ it 'marks removed lines' do
+ code = %q(- raise "System commands must be given as an array of strings")
- it 'marks unchanged lines' do
- code = %q{ def popen(cmd, path=nil)}
+ expect(subject[4].text).to eq(code)
+ expect(subject[4].text).not_to be_html_safe
+ end
- expect(subject[2].text).to eq(code)
- expect(subject[2].text).not_to be_html_safe
- end
+ it 'marks added lines' do
+ code = %q(+ raise <span class="idiff left right addition">RuntimeError, </span>"System commands must be given as an array of strings")
- it 'marks removed lines' do
- code = %q(- raise "System commands must be given as an array of strings")
+ expect(subject[5].rich_text).to eq(code)
+ expect(subject[5].rich_text).to be_html_safe
+ end
- expect(subject[4].text).to eq(code)
- expect(subject[4].text).not_to be_html_safe
- end
+ context 'when the inline diff marker has an invalid range' do
+ before do
+ allow_any_instance_of(Gitlab::Diff::InlineDiffMarker).to receive(:mark).and_raise(RangeError)
+ end
- it 'marks added lines' do
- code = %q(+ raise <span class="idiff left right addition">RuntimeError, </span>"System commands must be given as an array of strings")
+ it 'keeps the original rich line' do
+ allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
- expect(subject[5].rich_text).to eq(code)
- expect(subject[5].rich_text).to be_html_safe
- end
+ code = %q(+ raise RuntimeError, "System commands must be given as an array of strings")
- context 'when the inline diff marker has an invalid range' do
- before do
- allow_any_instance_of(Gitlab::Diff::InlineDiffMarker).to receive(:mark).and_raise(RangeError)
- end
+ expect(subject[5].text).to eq(code)
+ expect(subject[5].text).not_to be_html_safe
+ end
- it 'keeps the original rich line' do
- allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
+ it 'reports to Sentry if configured' do
+ expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original
- code = %q(+ raise RuntimeError, "System commands must be given as an array of strings")
+ expect { subject }.to raise_exception(RangeError)
+ end
+ end
- expect(subject[5].text).to eq(code)
- expect(subject[5].text).not_to be_html_safe
+ context 'when no inline diffs' do
+ it_behaves_like 'without inline diffs'
end
+ end
- it 'reports to Sentry if configured' do
- expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original
+ context 'when blob is too large' do
+ let(:subject) { described_class.new(diff_file, repository: project.repository).highlight }
- expect { subject }.to raise_exception(RangeError)
+ before do
+ allow(Gitlab::Highlight).to receive(:too_large?).and_return(true)
end
- end
- context 'when no inline diffs' do
- it_behaves_like 'without inline diffs'
+ it 'blobs are highlighted as plain text without loading all data' do
+ expect(diff_file.blob).not_to receive(:load_all_data!)
+
+ expect(subject[2].rich_text).to eq(%{ <span id="LC7" class="line" lang=""> def popen(cmd, path=nil)</span>\n})
+ expect(subject[2].rich_text).to be_html_safe
+ end
end
end
- context 'when blob is too large' do
- let(:subject) { described_class.new(diff_file, repository: project.repository).highlight }
+ it_behaves_like 'diff highlighter'
+ context 'when diff_line_syntax_highlighting feature flag is disabled' do
before do
- allow(Gitlab::Highlight).to receive(:too_large?).and_return(true)
+ stub_feature_flags(diff_line_syntax_highlighting: false)
end
- it 'blobs are highlighted as plain text without loading all data' do
- expect(diff_file.blob).not_to receive(:load_all_data!)
-
- expect(subject[2].rich_text).to eq(%{ <span id="LC7" class="line" lang=""> def popen(cmd, path=nil)</span>\n})
- expect(subject[2].rich_text).to be_html_safe
- end
+ it_behaves_like 'diff highlighter'
end
end
end
diff --git a/spec/lib/gitlab/highlight_spec.rb b/spec/lib/gitlab/highlight_spec.rb
index ef3765e479f0ddada1a35bf9837cf2630cc5881f..cd5965551071e192bfdb92b4db9859e333b675e1 100644
--- a/spec/lib/gitlab/highlight_spec.rb
+++ b/spec/lib/gitlab/highlight_spec.rb
@@ -116,7 +116,7 @@ def test(input):
it 'links dependencies via DependencyLinker' do
expect(Gitlab::DependencyLinker).to receive(:link)
- .with('file.name', 'Contents', anything).and_call_original
+ .with('file.name', 'Contents', anything, used_on: :blob).and_call_original
described_class.highlight('file.name', 'Contents')
end
@@ -133,5 +133,32 @@ def test(input):
highlight
end
end
+
+ it 'increments usage counter', :prometheus do
+ described_class.highlight(file_name, content)
+
+ gitlab_highlight_usage_counter = Gitlab::Metrics.registry.get(:gitlab_highlight_usage)
+
+ expect(gitlab_highlight_usage_counter.get(used_on: :blob)).to eq(1)
+ expect(gitlab_highlight_usage_counter.get(used_on: :diff)).to eq(0)
+ end
+
+ context 'when used_on is specified' do
+ it 'increments usage counter', :prometheus do
+ described_class.highlight(file_name, content, used_on: :diff)
+
+ gitlab_highlight_usage_counter = Gitlab::Metrics.registry.get(:gitlab_highlight_usage)
+
+ expect(gitlab_highlight_usage_counter.get(used_on: :diff)).to eq(1)
+ expect(gitlab_highlight_usage_counter.get(used_on: :blob)).to eq(0)
+ end
+
+ it 'links dependencies via DependencyLinker' do
+ expect(Gitlab::DependencyLinker).to receive(:link)
+ .with(file_name, content, anything, used_on: :diff).and_call_original
+
+ described_class.highlight(file_name, content, used_on: :diff)
+ end
+ end
end
end
diff --git a/spec/presenters/blob_presenter_spec.rb b/spec/presenters/blob_presenter_spec.rb
index eed39c7a4043e8c42dd9febef52c5324f1191e28..958bf222e2a4267a1af18d035a5014bc413f95c3 100644
--- a/spec/presenters/blob_presenter_spec.rb
+++ b/spec/presenters/blob_presenter_spec.rb
@@ -312,13 +312,13 @@
let(:git_blob) { blob.__getobj__ }
it 'returns highlighted content' do
- expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: nil, language: 'ruby')
+ expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: nil, language: 'ruby', used_on: :blob)
presenter.highlight
end
it 'returns plain content when :plain is true' do
- expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: true, language: 'ruby')
+ expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: true, language: 'ruby', used_on: :blob)
presenter.highlight(plain: true)
end
@@ -331,7 +331,7 @@
end
it 'returns limited highlighted content' do
- expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', "line one\n", plain: nil, language: 'ruby')
+ expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', "line one\n", plain: nil, language: 'ruby', used_on: :blob)
presenter.highlight(to: 1)
end
@@ -343,11 +343,19 @@
end
it 'passes language to inner call' do
- expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: nil, language: 'ruby')
+ expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: nil, language: 'ruby', used_on: :blob)
presenter.highlight
end
end
+
+ context 'when used_on param is present' do
+ it 'returns highlighted content' do
+ expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: nil, language: 'ruby', used_on: :diff)
+
+ presenter.highlight(used_on: :diff)
+ end
+ end
end
describe '#highlight_and_trim' do
diff --git a/spec/presenters/blobs/notebook_presenter_spec.rb b/spec/presenters/blobs/notebook_presenter_spec.rb
index 2f05dc98fb9507a5e2262fcde058f30db3302cf1..4caecccf538a295cb19f47d0d044c2d3699a5067 100644
--- a/spec/presenters/blobs/notebook_presenter_spec.rb
+++ b/spec/presenters/blobs/notebook_presenter_spec.rb
@@ -14,7 +14,7 @@
subject(:presenter) { described_class.new(blob, current_user: user) }
it 'highlight receives markdown' do
- expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: nil, language: 'md')
+ expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: nil, language: 'md', used_on: :blob)
presenter.highlight
end