Commit 94a0e08e authored by Peter Leitzen's avatar Peter Leitzen

Sync metric dashboards from repository to database only if needed

This reduces the amount of scheduled sidekiq SyncDashboardsWorker jobs
which do not import metrics and thus unnecessary.
parent 06655b7f
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module Git module Git
class BranchHooksService < ::Git::BaseHooksService class BranchHooksService < ::Git::BaseHooksService
extend ::Gitlab::Utils::Override
def execute def execute
execute_branch_hooks execute_branch_hooks
...@@ -41,9 +43,14 @@ module Git ...@@ -41,9 +43,14 @@ module Git
super super
end end
override :invalidated_file_types
def invalidated_file_types def invalidated_file_types
return super unless default_branch? && !creating_branch? return super unless default_branch? && !creating_branch?
modified_file_types
end
def modified_file_types
paths = commit_paths.values.reduce(&:merge) || Set.new paths = commit_paths.values.reduce(&:merge) || Set.new
Gitlab::FileDetector.types_in_paths(paths) Gitlab::FileDetector.types_in_paths(paths)
...@@ -82,6 +89,7 @@ module Git ...@@ -82,6 +89,7 @@ module Git
def enqueue_metrics_dashboard_sync def enqueue_metrics_dashboard_sync
return unless default_branch? return unless default_branch?
return unless modified_file_types.include?(:metrics_dashboard)
::Metrics::Dashboard::SyncDashboardsWorker.perform_async(project.id) ::Metrics::Dashboard::SyncDashboardsWorker.perform_async(project.id)
end end
...@@ -210,10 +218,10 @@ module Git ...@@ -210,10 +218,10 @@ module Git
def commit_paths def commit_paths
strong_memoize(:commit_paths) do strong_memoize(:commit_paths) do
limited_commits.map do |commit| limited_commits.to_h do |commit|
paths = Set.new(commit.raw_deltas.map(&:new_path)) paths = Set.new(commit.raw_deltas.map(&:new_path))
[commit, paths] [commit, paths]
end.to_h end
end end
end end
end end
......
...@@ -11,7 +11,8 @@ RSpec.describe Git::BranchHooksService do ...@@ -11,7 +11,8 @@ RSpec.describe Git::BranchHooksService do
let(:branch) { project.default_branch } let(:branch) { project.default_branch }
let(:ref) { "refs/heads/#{branch}" } let(:ref) { "refs/heads/#{branch}" }
let(:commit) { project.commit(sample_commit.id) } let(:commit_id) { sample_commit.id }
let(:commit) { project.commit(commit_id) }
let(:oldrev) { commit.parent_id } let(:oldrev) { commit.parent_id }
let(:newrev) { commit.id } let(:newrev) { commit.id }
...@@ -227,7 +228,6 @@ RSpec.describe Git::BranchHooksService do ...@@ -227,7 +228,6 @@ RSpec.describe Git::BranchHooksService do
) )
end end
let(:commit) { project.repository.commit(commit_id) }
let(:blank_sha) { Gitlab::Git::BLANK_SHA } let(:blank_sha) { Gitlab::Git::BLANK_SHA }
def clears_cache(extended: []) def clears_cache(extended: [])
...@@ -508,11 +508,7 @@ RSpec.describe Git::BranchHooksService do ...@@ -508,11 +508,7 @@ RSpec.describe Git::BranchHooksService do
end end
describe 'Metrics dashboard sync' do describe 'Metrics dashboard sync' do
context 'with feature flag enabled' do shared_examples 'trigger dashboard sync' do
before do
Feature.enable(:metrics_dashboards_sync)
end
it 'imports metrics to database' do it 'imports metrics to database' do
expect(Metrics::Dashboard::SyncDashboardsWorker).to receive(:perform_async) expect(Metrics::Dashboard::SyncDashboardsWorker).to receive(:perform_async)
...@@ -520,12 +516,95 @@ RSpec.describe Git::BranchHooksService do ...@@ -520,12 +516,95 @@ RSpec.describe Git::BranchHooksService do
end end
end end
context 'with feature flag disabled' do shared_examples 'no dashboard sync' do
it 'imports metrics to database' do it 'does not sync metrics to database' do
expect(Metrics::Dashboard::SyncDashboardsWorker).to receive(:perform_async) expect(Metrics::Dashboard::SyncDashboardsWorker).not_to receive(:perform_async)
service.execute service.execute
end end
end end
def change_repository(**changes)
actions = changes.flat_map do |(action, paths)|
Array(paths).flat_map do |file_path|
{ action: action, file_path: file_path, content: SecureRandom.hex }
end
end
project.repository.multi_action(
user, message: 'message', branch_name: branch, actions: actions
)
end
let(:charts) { '.gitlab/dashboards/charts.yml' }
let(:readme) { 'README.md' }
let(:commit_id) { change_repository(**commit_changes) }
context 'with default branch' do
context 'when adding files' do
let(:new_file) { 'somenewfile.md' }
context 'also related' do
let(:commit_changes) { { create: [charts, new_file] } }
include_examples 'trigger dashboard sync'
end
context 'only unrelated' do
let(:commit_changes) { { create: new_file } }
include_examples 'no dashboard sync'
end
end
context 'when deleting files' do
before do
change_repository(create: charts)
end
context 'also related' do
let(:commit_changes) { { delete: [charts, readme] } }
include_examples 'trigger dashboard sync'
end
context 'only unrelated' do
let(:commit_changes) { { delete: readme } }
include_examples 'no dashboard sync'
end
end
context 'when updating files' do
before do
change_repository(create: charts)
end
context 'also related' do
let(:commit_changes) { { update: [charts, readme] } }
include_examples 'trigger dashboard sync'
end
context 'only unrelated' do
let(:commit_changes) { { update: readme } }
include_examples 'no dashboard sync'
end
end
context 'without changes' do
let(:commit_changes) { {} }
include_examples 'no dashboard sync'
end
end
context 'with other branch' do
let(:branch) { 'fix' }
let(:commit_changes) { { create: charts } }
include_examples 'no dashboard sync'
end
end end
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