Merge branch 'dfrazao-gitlab/add-migration-name-to-artifacts' into 'master'

Add migration name to artifacts

See merge request gitlab-org/gitlab!66074

lib/gitlab/database/migrations/instrumentation.rb

@@ -14,8 +14,8 @@ module Gitlab
           @observations = []
         end
 
-        def observe(migration, &block)
-          observation = Observation.new(migration)
+        def observe(version:, name:, &block)
+          observation = Observation.new(version, name)
           observation.success = true
 
           exception = nil

lib/gitlab/database/migrations/observation.rb

@@ -4,7 +4,8 @@ module Gitlab
   module Database
     module Migrations
       Observation = Struct.new(
-        :migration,
+        :version,
+        :name,
         :walltime,
         :success,
         :total_database_size_change,

lib/gitlab/database/migrations/observers/query_details.rb

@@ -23,7 +23,7 @@ module Gitlab
           end
 
           def record(observation)
-            File.rename(@file_path, File.join(Instrumentation::RESULT_DIR, "#{observation.migration}-query-details.json"))
+            File.rename(@file_path, File.join(Instrumentation::RESULT_DIR, "#{observation.version}_#{observation.name}-query-details.json"))
           end
 
           def record_sql_event(_name, started, finished, _unique_id, payload)

lib/gitlab/database/migrations/observers/query_log.rb

@@ -18,7 +18,7 @@ module Gitlab
           end
 
           def record(observation)
-            File.rename(@log_file_path, File.join(Instrumentation::RESULT_DIR, "#{observation.migration}.log"))
+            File.rename(@log_file_path, File.join(Instrumentation::RESULT_DIR, "#{observation.version}_#{observation.name}.log"))
           end
         end
       end

lib/tasks/gitlab/db.rake

@@ -220,7 +220,7 @@ namespace :gitlab do
       instrumentation = Gitlab::Database::Migrations::Instrumentation.new
 
       pending_migrations.each do |migration|
-        instrumentation.observe(migration.version) do
+        instrumentation.observe(version: migration.version, name: migration.name) do
           ActiveRecord::Migrator.new(:up, ctx.migrations, ctx.schema_migration, migration.version).run
         end
       end

spec/lib/gitlab/database/migrations/instrumentation_spec.rb

@@ -5,14 +5,15 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
   describe '#observe' do
     subject { described_class.new }
 
-    let(:migration) { 1234 }
+    let(:migration_name) { 'test' }
+    let(:migration_version) { '12345' }
 
     it 'executes the given block' do
-      expect { |b| subject.observe(migration, &b) }.to yield_control
+      expect { |b| subject.observe(version: migration_version, name: migration_name, &b) }.to yield_control
     end
 
     context 'behavior with observers' do
-      subject { described_class.new(observers).observe(migration) {} }
+      subject { described_class.new(observers).observe(version: migration_version, name: migration_name) {} }
 
       let(:observers) { [observer] }
       let(:observer) { instance_double('Gitlab::Database::Migrations::Observers::MigrationObserver', before: nil, after: nil, record: nil) }
@@ -21,7 +22,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
         expect(observer).to receive(:before).ordered
         expect(observer).to receive(:after).ordered
         expect(observer).to receive(:record).ordered do |observation|
-          expect(observation.migration).to eq(migration)
+          expect(observation.version).to eq(migration_version)
         end
 
         subject
@@ -47,7 +48,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
     end
 
     context 'on successful execution' do
-      subject { described_class.new.observe(migration) {} }
+      subject { described_class.new.observe(version: migration_version, name: migration_name) {} }
 
       it 'records walltime' do
         expect(subject.walltime).not_to be_nil
@@ -58,12 +59,16 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
       end
 
       it 'records the migration version' do
-        expect(subject.migration).to eq(migration)
+        expect(subject.version).to eq(migration_version)
+      end
+
+      it 'records the migration name' do
+        expect(subject.name).to eq(migration_name)
       end
     end
 
     context 'upon failure' do
-      subject { described_class.new.observe(migration) { raise 'something went wrong' } }
+      subject { described_class.new.observe(version: migration_version, name: migration_name) { raise 'something went wrong' } }
 
       it 'raises the exception' do
         expect { subject }.to raise_error(/something went wrong/)
@@ -73,7 +78,7 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
         subject { instance.observations.first }
 
         before do
-          instance.observe(migration) { raise 'something went wrong' }
+          instance.observe(version: migration_version, name: migration_name) { raise 'something went wrong' }
         rescue StandardError
           # ignore
         end
@@ -89,7 +94,11 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
         end
 
         it 'records the migration version' do
-          expect(subject.migration).to eq(migration)
+          expect(subject.version).to eq(migration_version)
+        end
+
+        it 'records the migration name' do
+          expect(subject.name).to eq(migration_name)
         end
       end
     end
@@ -101,8 +110,8 @@ RSpec.describe Gitlab::Database::Migrations::Instrumentation do
       let(:migration2) { double('migration2', call: nil) }
 
       it 'records observations for all migrations' do
-        subject.observe('migration1') {}
-        subject.observe('migration2') { raise 'something went wrong' } rescue nil
+        subject.observe(version: migration_version, name: migration_name) {}
+        subject.observe(version: migration_version, name: migration_name) { raise 'something went wrong' } rescue nil
 
         expect(subject.observations.size).to eq(2)
       end

spec/lib/gitlab/database/migrations/observers/query_details_spec.rb

@@ -4,14 +4,15 @@ require 'spec_helper'
 RSpec.describe Gitlab::Database::Migrations::Observers::QueryDetails do
   subject { described_class.new }
 
-  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration) }
+  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) }
   let(:connection) { ActiveRecord::Base.connection }
   let(:query) { "select date_trunc('day', $1::timestamptz) + $2 * (interval '1 hour')" }
   let(:query_binds) { [Time.current, 3] }
   let(:directory_path) { Dir.mktmpdir }
-  let(:log_file) { "#{directory_path}/#{migration}-query-details.json" }
+  let(:log_file) { "#{directory_path}/#{migration_version}_#{migration_name}-query-details.json" }
   let(:query_details) { Gitlab::Json.parse(File.read(log_file)) }
-  let(:migration) { 20210422152437 }
+  let(:migration_version) { 20210422152437 }
+  let(:migration_name) { 'test' }
 
   before do
     stub_const('Gitlab::Database::Migrations::Instrumentation::RESULT_DIR', directory_path)

spec/lib/gitlab/database/migrations/observers/query_log_spec.rb

@@ -4,12 +4,13 @@ require 'spec_helper'
 RSpec.describe Gitlab::Database::Migrations::Observers::QueryLog do
   subject { described_class.new }
 
-  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration) }
+  let(:observation) { Gitlab::Database::Migrations::Observation.new(migration_version, migration_name) }
   let(:connection) { ActiveRecord::Base.connection }
   let(:query) { 'select 1' }
   let(:directory_path) { Dir.mktmpdir }
   let(:log_file) { "#{directory_path}/current.log" }
-  let(:migration) { 20210422152437 }
+  let(:migration_version) { 20210422152437 }
+  let(:migration_name) { 'test' }
 
   before do
     stub_const('Gitlab::Database::Migrations::Instrumentation::RESULT_DIR', directory_path)
@@ -22,7 +23,7 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryLog do
   it 'writes a file with the query log' do
     observe
 
-    expect(File.read("#{directory_path}/#{migration}.log")).to include(query)
+    expect(File.read("#{directory_path}/#{migration_version}_#{migration_name}.log")).to include(query)
   end
 
   it 'does not change the default logger' do

spec/tasks/gitlab/db_rake_spec.rb

@@ -276,8 +276,8 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout do
     let(:ctx) { double('ctx', migrations: all_migrations, schema_migration: double, get_all_versions: existing_versions) }
     let(:instrumentation) { instance_double(Gitlab::Database::Migrations::Instrumentation, observations: observations) }
     let(:existing_versions) { [1] }
-    let(:all_migrations) { [double('migration1', version: 1), pending_migration] }
-    let(:pending_migration) { double('migration2', version: 2) }
+    let(:all_migrations) { [double('migration1', version: 1, name: 'test'), pending_migration] }
+    let(:pending_migration) { double('migration2', version: 2, name: 'test') }
     let(:filename) { Gitlab::Database::Migrations::Instrumentation::STATS_FILENAME }
     let(:result_dir) { Dir.mktmpdir }
     let(:observations) { %w[some data] }
@@ -303,7 +303,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout do
     end
 
     it 'instruments the pending migration' do
-      expect(instrumentation).to receive(:observe).with(2).and_yield
+      expect(instrumentation).to receive(:observe).with(version: 2, name: 'test').and_yield
 
       subject
     end