Add migration name to artifacts

The previous code was only using the migration version to generate
the filename of the artifacts.
With this change, we also add the migration name to the filename.
New filename format: #{MIGRATION_VERSION}_#{MIGRATION_NAME}

Related to https://gitlab.com/gitlab-org/database-team/gitlab-com-database-testing/-/issues/17

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