From 00ea927d0356143bca404e9c040ded048d5d43c8 Mon Sep 17 00:00:00 2001
From: Allison Browne <abrowne@gitlab.com>
Date: Mon, 26 Jul 2021 13:58:16 -0400
Subject: [PATCH] Remove build trace section related application code

Remove the application code that populates the ci_build_trace_sections
and ci_build_trace_section_names. This data is not read by the
application currently and we can save on space/maintaince and solve
problems with background migration for self-managed users by dropping
the table is subsequent MRs.
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66674

This table was originally created to collect metrics on jobs but was
never used and an expiration policy of was never enforced.
https://gitlab.com/gitlab-org/gitlab-runner/-/issues/2505

There is the possibility that we will need to build out features that
need this data in the future, based on the future vision, but at this
time it is not being used, and we can re-add it while re-considering
the architecture at that time (likely in 2022).
https://gitlab.com/gitlab-org/gitlab/-/issues/32565
---
 app/models/ci/build.rb                        |  7 ---
 app/models/ci/build_trace_section.rb          | 17 ------
 app/models/ci/build_trace_section_name.rb     | 13 -----
 app/models/project.rb                         |  1 -
 ...tract_sections_from_build_trace_service.rb | 34 -----------
 app/workers/ci/build_finished_worker.rb       |  1 -
 rubocop/rubocop-usage-data.yml                |  1 -
 .../factories/ci/build_trace_section_names.rb |  8 ---
 spec/lib/gitlab/import_export/all_models.yml  |  1 -
 spec/models/ci/build_spec.rb                  | 12 ----
 .../ci/build_trace_section_name_spec.rb       | 14 -----
 spec/models/ci/build_trace_section_spec.rb    | 13 -----
 spec/models/project_spec.rb                   |  1 -
 .../create_table_with_foreign_keys_spec.rb    |  1 -
 ..._sections_from_build_trace_service_spec.rb | 57 -------------------
 spec/workers/build_finished_worker_spec.rb    |  1 -
 spec/workers/ci/build_finished_worker_spec.rb |  1 -
 17 files changed, 183 deletions(-)
 delete mode 100644 app/models/ci/build_trace_section.rb
 delete mode 100644 app/models/ci/build_trace_section_name.rb
 delete mode 100644 app/services/ci/extract_sections_from_build_trace_service.rb
 delete mode 100644 spec/factories/ci/build_trace_section_names.rb
 delete mode 100644 spec/models/ci/build_trace_section_name_spec.rb
 delete mode 100644 spec/models/ci/build_trace_section_spec.rb
 delete mode 100644 spec/services/ci/extract_sections_from_build_trace_service_spec.rb

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 4328f3f7a4b8a..93bce1b672770 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -39,7 +39,6 @@ class Build < Ci::Processable
     has_one :pending_state, class_name: 'Ci::BuildPendingState', inverse_of: :build
     has_one :queuing_entry, class_name: 'Ci::PendingBuild', foreign_key: :build_id
     has_one :runtime_metadata, class_name: 'Ci::RunningBuild', foreign_key: :build_id
-    has_many :trace_sections, class_name: 'Ci::BuildTraceSection'
     has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id, inverse_of: :build
     has_many :report_results, class_name: 'Ci::BuildReportResult', inverse_of: :build
 
@@ -648,12 +647,6 @@ def update_coverage
       update(coverage: coverage) if coverage.present?
     end
 
-    # rubocop: disable CodeReuse/ServiceClass
-    def parse_trace_sections!
-      ExtractSectionsFromBuildTraceService.new(project, user).execute(self)
-    end
-    # rubocop: enable CodeReuse/ServiceClass
-
     def trace
       Gitlab::Ci::Trace.new(self)
     end
diff --git a/app/models/ci/build_trace_section.rb b/app/models/ci/build_trace_section.rb
deleted file mode 100644
index 036f611a61c77..0000000000000
--- a/app/models/ci/build_trace_section.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-# frozen_string_literal: true
-
-module Ci
-  class BuildTraceSection < ApplicationRecord
-    extend SuppressCompositePrimaryKeyWarning
-    extend Gitlab::Ci::Model
-    include IgnorableColumns
-
-    belongs_to :build, class_name: 'Ci::Build'
-    belongs_to :project
-    belongs_to :section_name, class_name: 'Ci::BuildTraceSectionName'
-
-    validates :section_name, :build, :project, presence: true, allow_blank: false
-
-    ignore_column :build_id_convert_to_bigint, remove_with: '14.2', remove_after: '2021-08-22'
-  end
-end
diff --git a/app/models/ci/build_trace_section_name.rb b/app/models/ci/build_trace_section_name.rb
deleted file mode 100644
index c065cfea14ea4..0000000000000
--- a/app/models/ci/build_trace_section_name.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-# frozen_string_literal: true
-
-module Ci
-  class BuildTraceSectionName < ApplicationRecord
-    extend Gitlab::Ci::Model
-
-    belongs_to :project
-    has_many :trace_sections, class_name: 'Ci::BuildTraceSection', foreign_key: :section_name_id
-
-    validates :name, :project, presence: true, allow_blank: false
-    validates :name, uniqueness: { scope: :project_id }
-  end
-end
diff --git a/app/models/project.rb b/app/models/project.rb
index 6d99fa31c3d45..afd8cdf8a205d 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -318,7 +318,6 @@ def self.integration_association_name(name)
   # still using `dependent: :destroy` here.
   has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :processables, class_name: 'Ci::Processable', inverse_of: :project
-  has_many :build_trace_section_names, class_name: 'Ci::BuildTraceSectionName'
   has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks
   has_many :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project
   has_many :job_artifacts, class_name: 'Ci::JobArtifact'
diff --git a/app/services/ci/extract_sections_from_build_trace_service.rb b/app/services/ci/extract_sections_from_build_trace_service.rb
deleted file mode 100644
index c756e376901ec..0000000000000
--- a/app/services/ci/extract_sections_from_build_trace_service.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-module Ci
-  class ExtractSectionsFromBuildTraceService < BaseService
-    def execute(build)
-      return false unless build.trace_sections.empty?
-
-      Gitlab::Database.bulk_insert(BuildTraceSection.table_name, extract_sections(build)) # rubocop:disable Gitlab/BulkInsert
-      true
-    end
-
-    private
-
-    # rubocop: disable CodeReuse/ActiveRecord
-    def find_or_create_name(name)
-      project.build_trace_section_names.find_or_create_by!(name: name)
-    rescue ActiveRecord::RecordInvalid
-      project.build_trace_section_names.find_by!(name: name)
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    def extract_sections(build)
-      build.trace.extract_sections.map do |attr|
-        name = attr.delete(:name)
-        name_record = find_or_create_name(name)
-
-        attr.merge(
-          build_id: build.id,
-          project_id: project.id,
-          section_name_id: name_record.id)
-      end
-    end
-  end
-end
diff --git a/app/workers/ci/build_finished_worker.rb b/app/workers/ci/build_finished_worker.rb
index 1d6e3b1fa3cc9..d294e92dbc1e7 100644
--- a/app/workers/ci/build_finished_worker.rb
+++ b/app/workers/ci/build_finished_worker.rb
@@ -31,7 +31,6 @@ def perform(build_id)
     # @param [Ci::Build] build The build to process.
     def process_build(build)
       # We execute these in sync to reduce IO.
-      build.parse_trace_sections!
       build.update_coverage
       Ci::BuildReportResultService.new.execute(build)
 
diff --git a/rubocop/rubocop-usage-data.yml b/rubocop/rubocop-usage-data.yml
index 2da7b056c2e62..c79a5a8ed7f29 100644
--- a/rubocop/rubocop-usage-data.yml
+++ b/rubocop/rubocop-usage-data.yml
@@ -46,7 +46,6 @@ UsageData/HistogramWithLargeTable:
     - 'ee/lib/ee/gitlab/usage_data.rb'
   HighTrafficModels: &high_traffic_models # models for all high traffic tables in Migration/UpdateLargeTable
     - 'AuditEvent'
-    - 'Ci::BuildTraceSection'
     - 'CommitStatus'
     - 'Ci::Processable'
     - 'Ci::Bridge'
diff --git a/spec/factories/ci/build_trace_section_names.rb b/spec/factories/ci/build_trace_section_names.rb
deleted file mode 100644
index b9b66b4931725..0000000000000
--- a/spec/factories/ci/build_trace_section_names.rb
+++ /dev/null
@@ -1,8 +0,0 @@
-# frozen_string_literal: true
-
-FactoryBot.define do
-  factory :ci_build_trace_section_name, class: 'Ci::BuildTraceSectionName' do
-    sequence(:name) { |n| "section_#{n}" }
-    project factory: :project
-  end
-end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 78805cea66a91..f3ebb24022813 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -461,7 +461,6 @@ project:
 - file_uploads
 - import_state
 - members_and_requesters
-- build_trace_section_names
 - build_trace_chunks
 - job_artifacts
 - root_of_fork_network
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 0c344270e0bee..972b4743591ef 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -20,7 +20,6 @@
   it { is_expected.to belong_to(:trigger_request) }
   it { is_expected.to belong_to(:erased_by) }
 
-  it { is_expected.to have_many(:trace_sections) }
   it { is_expected.to have_many(:needs) }
   it { is_expected.to have_many(:sourced_pipelines) }
   it { is_expected.to have_many(:job_variables) }
@@ -1105,17 +1104,6 @@
     end
   end
 
-  describe '#parse_trace_sections!' do
-    it 'calls ExtractSectionsFromBuildTraceService' do
-      expect(Ci::ExtractSectionsFromBuildTraceService)
-          .to receive(:new).with(project, build.user).once.and_call_original
-      expect_any_instance_of(Ci::ExtractSectionsFromBuildTraceService)
-        .to receive(:execute).with(build).once
-
-      build.parse_trace_sections!
-    end
-  end
-
   describe '#trace' do
     subject { build.trace }
 
diff --git a/spec/models/ci/build_trace_section_name_spec.rb b/spec/models/ci/build_trace_section_name_spec.rb
deleted file mode 100644
index b220e67d48eaa..0000000000000
--- a/spec/models/ci/build_trace_section_name_spec.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Ci::BuildTraceSectionName, model: true do
-  subject { build(:ci_build_trace_section_name) }
-
-  it { is_expected.to belong_to(:project) }
-  it { is_expected.to have_many(:trace_sections)}
-
-  it { is_expected.to validate_presence_of(:project) }
-  it { is_expected.to validate_presence_of(:name) }
-  it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) }
-end
diff --git a/spec/models/ci/build_trace_section_spec.rb b/spec/models/ci/build_trace_section_spec.rb
deleted file mode 100644
index 640bd202b3ae6..0000000000000
--- a/spec/models/ci/build_trace_section_spec.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Ci::BuildTraceSection, model: true do
-  it { is_expected.to belong_to(:build)}
-  it { is_expected.to belong_to(:project)}
-  it { is_expected.to belong_to(:section_name)}
-
-  it { is_expected.to validate_presence_of(:section_name) }
-  it { is_expected.to validate_presence_of(:build) }
-  it { is_expected.to validate_presence_of(:project) }
-end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index efa269cdb5c83..1ae0ec2c1f423 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -86,7 +86,6 @@
     it { is_expected.to have_many(:ci_pipelines) }
     it { is_expected.to have_many(:ci_refs) }
     it { is_expected.to have_many(:builds) }
-    it { is_expected.to have_many(:build_trace_section_names)}
     it { is_expected.to have_many(:build_report_results) }
     it { is_expected.to have_many(:runner_projects) }
     it { is_expected.to have_many(:runners) }
diff --git a/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb
index 7bcaf36b0148c..6a8df2b507d4f 100644
--- a/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb
+++ b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb
@@ -116,7 +116,6 @@ def up
         shared_context 'when there is a target to a high traffic table' do |dsl_method|
           %w[
             audit_events
-            ci_build_trace_sections
             ci_builds
             ci_builds_metadata
             ci_job_artifacts
diff --git a/spec/services/ci/extract_sections_from_build_trace_service_spec.rb b/spec/services/ci/extract_sections_from_build_trace_service_spec.rb
deleted file mode 100644
index c6ffcdcc6a8aa..0000000000000
--- a/spec/services/ci/extract_sections_from_build_trace_service_spec.rb
+++ /dev/null
@@ -1,57 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Ci::ExtractSectionsFromBuildTraceService, '#execute' do
-  let(:user) { create(:user) }
-  let(:project) { create(:project) }
-  let(:build) { create(:ci_build, project: project) }
-
-  subject { described_class.new(project, user) }
-
-  shared_examples 'build trace has sections markers' do
-    before do
-      build.trace.set(File.read(expand_fixture_path('trace/trace_with_sections')))
-    end
-
-    it 'saves the correct extracted sections' do
-      expect(build.trace_sections).to be_empty
-      expect(subject.execute(build)).to be(true)
-      expect(build.trace_sections).not_to be_empty
-    end
-
-    it "fails if trace_sections isn't empty" do
-      expect(subject.execute(build)).to be(true)
-      expect(build.trace_sections).not_to be_empty
-
-      expect(subject.execute(build)).to be(false)
-      expect(build.trace_sections).not_to be_empty
-    end
-  end
-
-  shared_examples 'build trace has no sections markers' do
-    before do
-      build.trace.set('no markerts')
-    end
-
-    it 'extracts no sections' do
-      expect(build.trace_sections).to be_empty
-      expect(subject.execute(build)).to be(true)
-      expect(build.trace_sections).to be_empty
-    end
-  end
-
-  context 'when the build has no user' do
-    it_behaves_like 'build trace has sections markers'
-    it_behaves_like 'build trace has no sections markers'
-  end
-
-  context 'when the build has a valid user' do
-    before do
-      build.user = user
-    end
-
-    it_behaves_like 'build trace has sections markers'
-    it_behaves_like 'build trace has no sections markers'
-  end
-end
diff --git a/spec/workers/build_finished_worker_spec.rb b/spec/workers/build_finished_worker_spec.rb
index 6b7162ee886dd..4e34d2348d6dc 100644
--- a/spec/workers/build_finished_worker_spec.rb
+++ b/spec/workers/build_finished_worker_spec.rb
@@ -15,7 +15,6 @@
       end
 
       it 'calculates coverage and calls hooks', :aggregate_failures do
-        expect(build).to receive(:parse_trace_sections!).ordered
         expect(build).to receive(:update_coverage).ordered
 
         expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service|
diff --git a/spec/workers/ci/build_finished_worker_spec.rb b/spec/workers/ci/build_finished_worker_spec.rb
index 374ecd8619f98..9096b0d2ba968 100644
--- a/spec/workers/ci/build_finished_worker_spec.rb
+++ b/spec/workers/ci/build_finished_worker_spec.rb
@@ -15,7 +15,6 @@
       end
 
       it 'calculates coverage and calls hooks', :aggregate_failures do
-        expect(build).to receive(:parse_trace_sections!).ordered
         expect(build).to receive(:update_coverage).ordered
 
         expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service|
-- 
GitLab