Commit 00ea927d authored by Allison Browne's avatar Allison Browne

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
parent e5508129
...@@ -39,7 +39,6 @@ module Ci ...@@ -39,7 +39,6 @@ module Ci
has_one :pending_state, class_name: 'Ci::BuildPendingState', inverse_of: :build 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 :queuing_entry, class_name: 'Ci::PendingBuild', foreign_key: :build_id
has_one :runtime_metadata, class_name: 'Ci::RunningBuild', 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 :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id, inverse_of: :build
has_many :report_results, class_name: 'Ci::BuildReportResult', inverse_of: :build has_many :report_results, class_name: 'Ci::BuildReportResult', inverse_of: :build
...@@ -648,12 +647,6 @@ module Ci ...@@ -648,12 +647,6 @@ module Ci
update(coverage: coverage) if coverage.present? update(coverage: coverage) if coverage.present?
end end
# rubocop: disable CodeReuse/ServiceClass
def parse_trace_sections!
ExtractSectionsFromBuildTraceService.new(project, user).execute(self)
end
# rubocop: enable CodeReuse/ServiceClass
def trace def trace
Gitlab::Ci::Trace.new(self) Gitlab::Ci::Trace.new(self)
end end
......
# 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
# 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
...@@ -318,7 +318,6 @@ class Project < ApplicationRecord ...@@ -318,7 +318,6 @@ class Project < ApplicationRecord
# still using `dependent: :destroy` here. # still using `dependent: :destroy` here.
has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent 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 :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_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 :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project
has_many :job_artifacts, class_name: 'Ci::JobArtifact' has_many :job_artifacts, class_name: 'Ci::JobArtifact'
......
# 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
...@@ -31,7 +31,6 @@ module Ci ...@@ -31,7 +31,6 @@ module Ci
# @param [Ci::Build] build The build to process. # @param [Ci::Build] build The build to process.
def process_build(build) def process_build(build)
# We execute these in sync to reduce IO. # We execute these in sync to reduce IO.
build.parse_trace_sections!
build.update_coverage build.update_coverage
Ci::BuildReportResultService.new.execute(build) Ci::BuildReportResultService.new.execute(build)
......
...@@ -46,7 +46,6 @@ UsageData/HistogramWithLargeTable: ...@@ -46,7 +46,6 @@ UsageData/HistogramWithLargeTable:
- 'ee/lib/ee/gitlab/usage_data.rb' - 'ee/lib/ee/gitlab/usage_data.rb'
HighTrafficModels: &high_traffic_models # models for all high traffic tables in Migration/UpdateLargeTable HighTrafficModels: &high_traffic_models # models for all high traffic tables in Migration/UpdateLargeTable
- 'AuditEvent' - 'AuditEvent'
- 'Ci::BuildTraceSection'
- 'CommitStatus' - 'CommitStatus'
- 'Ci::Processable' - 'Ci::Processable'
- 'Ci::Bridge' - 'Ci::Bridge'
......
# 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
...@@ -461,7 +461,6 @@ project: ...@@ -461,7 +461,6 @@ project:
- file_uploads - file_uploads
- import_state - import_state
- members_and_requesters - members_and_requesters
- build_trace_section_names
- build_trace_chunks - build_trace_chunks
- job_artifacts - job_artifacts
- root_of_fork_network - root_of_fork_network
......
...@@ -20,7 +20,6 @@ RSpec.describe Ci::Build do ...@@ -20,7 +20,6 @@ RSpec.describe Ci::Build do
it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:trigger_request) }
it { is_expected.to belong_to(:erased_by) } 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(:needs) }
it { is_expected.to have_many(:sourced_pipelines) } it { is_expected.to have_many(:sourced_pipelines) }
it { is_expected.to have_many(:job_variables) } it { is_expected.to have_many(:job_variables) }
...@@ -1105,17 +1104,6 @@ RSpec.describe Ci::Build do ...@@ -1105,17 +1104,6 @@ RSpec.describe Ci::Build do
end end
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 describe '#trace' do
subject { build.trace } subject { build.trace }
......
# 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
# 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
...@@ -86,7 +86,6 @@ RSpec.describe Project, factory_default: :keep do ...@@ -86,7 +86,6 @@ RSpec.describe Project, factory_default: :keep do
it { is_expected.to have_many(:ci_pipelines) } it { is_expected.to have_many(:ci_pipelines) }
it { is_expected.to have_many(:ci_refs) } it { is_expected.to have_many(:ci_refs) }
it { is_expected.to have_many(:builds) } 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(:build_report_results) }
it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runner_projects) }
it { is_expected.to have_many(:runners) } it { is_expected.to have_many(:runners) }
......
...@@ -116,7 +116,6 @@ RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do ...@@ -116,7 +116,6 @@ RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys do
shared_context 'when there is a target to a high traffic table' do |dsl_method| shared_context 'when there is a target to a high traffic table' do |dsl_method|
%w[ %w[
audit_events audit_events
ci_build_trace_sections
ci_builds ci_builds
ci_builds_metadata ci_builds_metadata
ci_job_artifacts ci_job_artifacts
......
# 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
...@@ -15,7 +15,6 @@ RSpec.describe BuildFinishedWorker do ...@@ -15,7 +15,6 @@ RSpec.describe BuildFinishedWorker do
end end
it 'calculates coverage and calls hooks', :aggregate_failures do 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(build).to receive(:update_coverage).ordered
expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service| expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service|
......
...@@ -15,7 +15,6 @@ RSpec.describe Ci::BuildFinishedWorker do ...@@ -15,7 +15,6 @@ RSpec.describe Ci::BuildFinishedWorker do
end end
it 'calculates coverage and calls hooks', :aggregate_failures do 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(build).to receive(:update_coverage).ordered
expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service| expect_next_instance_of(Ci::BuildReportResultService) do |build_report_result_service|
......
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