Commit ecf93554 authored by Luis Mejia's avatar Luis Mejia Committed by Alper Akgun

Add track_and_raise_for_dev_exception to usage_data methods

And:

- Update spec for email campaign date
- Update utils specs to check raised error
parent 3bb0b5ab
...@@ -711,43 +711,62 @@ RSpec.describe Gitlab::UsageData do ...@@ -711,43 +711,62 @@ RSpec.describe Gitlab::UsageData do
) )
end end
it 'has to resort to 0 for counting license scan' do context 'when count fails' do
for_defined_days_back do subject { described_class.usage_activity_by_stage_secure(described_class.monthly_time_range_db_params) }
create(:security_scan)
before do
allow(Gitlab::Database::BatchCount).to receive(:batch_distinct_count).and_raise(ActiveRecord::StatementInvalid)
allow(Gitlab::Database::BatchCount).to receive(:batch_count).and_raise(ActiveRecord::StatementInvalid)
allow(Gitlab::Database::PostgresHll::BatchDistinctCounter).to receive(:new).and_raise(ActiveRecord::StatementInvalid)
allow(::Ci::Build).to receive(:distinct_count_by).and_raise(ActiveRecord::StatementInvalid)
allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev)
end end
allow(Gitlab::Database::BatchCount).to receive(:batch_distinct_count).and_raise(ActiveRecord::StatementInvalid) context 'with should_raise_for_dev? true' do
allow(Gitlab::Database::BatchCount).to receive(:batch_count).and_raise(ActiveRecord::StatementInvalid) let(:should_raise_for_dev) { true }
allow(Gitlab::Database::PostgresHll::BatchDistinctCounter).to receive(:new).and_raise(ActiveRecord::StatementInvalid)
allow(::Ci::Build).to receive(:distinct_count_by).and_raise(ActiveRecord::StatementInvalid)
expect(described_class.usage_activity_by_stage_secure(described_class.monthly_time_range_db_params)).to include( it 'raises an error' do
user_preferences_group_overview_security_dashboard: -1, expect { subject }.to raise_error(ActiveRecord::StatementInvalid)
user_api_fuzzing_jobs: -1, end
user_api_fuzzing_dnd_jobs: -1, end
user_container_scanning_jobs: -1,
user_coverage_fuzzing_jobs: -1, context 'with should_raise_for_dev? false' do
user_dast_jobs: -1, let(:should_raise_for_dev) { false }
user_dependency_scanning_jobs: -1,
user_license_management_jobs: -1, it 'has to resort to 0 for counting license scan' do
user_sast_jobs: -1, for_defined_days_back do
user_secret_detection_jobs: -1, create(:security_scan)
sast_pipeline: -1, end
sast_scans: -1,
dependency_scanning_pipeline: -1, expect(subject).to include(
dependency_scanning_scans: -1, user_preferences_group_overview_security_dashboard: -1,
container_scanning_pipeline: -1, user_api_fuzzing_jobs: -1,
container_scanning_scans: -1, user_api_fuzzing_dnd_jobs: -1,
dast_pipeline: -1, user_container_scanning_jobs: -1,
dast_scans: -1, user_coverage_fuzzing_jobs: -1,
secret_detection_pipeline: -1, user_dast_jobs: -1,
secret_detection_scans: -1, user_dependency_scanning_jobs: -1,
coverage_fuzzing_pipeline: -1, user_license_management_jobs: -1,
coverage_fuzzing_scans: -1, user_sast_jobs: -1,
api_fuzzing_pipeline: -1, user_secret_detection_jobs: -1,
api_fuzzing_scans: -1, sast_pipeline: -1,
user_unique_users_all_secure_scanners: -1 sast_scans: -1,
) dependency_scanning_pipeline: -1,
dependency_scanning_scans: -1,
container_scanning_pipeline: -1,
container_scanning_scans: -1,
dast_pipeline: -1,
dast_scans: -1,
secret_detection_pipeline: -1,
secret_detection_scans: -1,
coverage_fuzzing_pipeline: -1,
coverage_fuzzing_scans: -1,
api_fuzzing_pipeline: -1,
api_fuzzing_scans: -1,
user_unique_users_all_secure_scanners: -1
)
end
end
end end
it 'deprecates count for users who have run scans' do it 'deprecates count for users who have run scans' do
......
...@@ -406,7 +406,8 @@ module Gitlab ...@@ -406,7 +406,8 @@ module Gitlab
results[:projects_jira_cloud_active] = jira_integration_data_hash[:projects_jira_cloud_active] results[:projects_jira_cloud_active] = jira_integration_data_hash[:projects_jira_cloud_active]
results results
rescue ActiveRecord::StatementInvalid rescue ActiveRecord::StatementInvalid => error
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error)
{ projects_jira_server_active: FALLBACK, projects_jira_cloud_active: FALLBACK } { projects_jira_server_active: FALLBACK, projects_jira_cloud_active: FALLBACK }
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -56,7 +56,8 @@ module Gitlab ...@@ -56,7 +56,8 @@ module Gitlab
else else
relation.count relation.count
end end
rescue ActiveRecord::StatementInvalid rescue ActiveRecord::StatementInvalid => error
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error)
FALLBACK FALLBACK
end end
...@@ -66,7 +67,8 @@ module Gitlab ...@@ -66,7 +67,8 @@ module Gitlab
else else
relation.distinct_count_by(column) relation.distinct_count_by(column)
end end
rescue ActiveRecord::StatementInvalid rescue ActiveRecord::StatementInvalid => error
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error)
FALLBACK FALLBACK
end end
...@@ -78,7 +80,8 @@ module Gitlab ...@@ -78,7 +80,8 @@ module Gitlab
yield buckets if block_given? yield buckets if block_given?
buckets.estimated_distinct_count buckets.estimated_distinct_count
rescue ActiveRecord::StatementInvalid rescue ActiveRecord::StatementInvalid => error
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error)
FALLBACK FALLBACK
# catch all rescue should be removed as a part of feature flag rollout issue # catch all rescue should be removed as a part of feature flag rollout issue
# https://gitlab.com/gitlab-org/gitlab/-/issues/285485 # https://gitlab.com/gitlab-org/gitlab/-/issues/285485
...@@ -89,7 +92,8 @@ module Gitlab ...@@ -89,7 +92,8 @@ module Gitlab
def sum(relation, column, batch_size: nil, start: nil, finish: nil) def sum(relation, column, batch_size: nil, start: nil, finish: nil)
Gitlab::Database::BatchCount.batch_sum(relation, column, batch_size: batch_size, start: start, finish: finish) Gitlab::Database::BatchCount.batch_sum(relation, column, batch_size: batch_size, start: start, finish: finish)
rescue ActiveRecord::StatementInvalid rescue ActiveRecord::StatementInvalid => error
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error)
FALLBACK FALLBACK
end end
...@@ -155,7 +159,8 @@ module Gitlab ...@@ -155,7 +159,8 @@ module Gitlab
query: query.to_sql, query: query.to_sql,
message: e.message message: e.message
) )
# Raises error for dev env
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
HISTOGRAM_FALLBACK HISTOGRAM_FALLBACK
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -684,23 +684,50 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do ...@@ -684,23 +684,50 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
end end
end end
it 'works when queries time out' do context 'when queries time out' do
allow_any_instance_of(ActiveRecord::Relation) let(:metric_method) { :count }
.to receive(:count).and_raise(ActiveRecord::StatementInvalid.new(''))
before do
allow_any_instance_of(ActiveRecord::Relation).to receive(metric_method).and_raise(ActiveRecord::StatementInvalid)
allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev)
end
context 'with should_raise_for_dev? true' do
let(:should_raise_for_dev) { true }
it 'raises an error' do
expect { subject }.to raise_error(ActiveRecord::StatementInvalid)
end
context 'when metric calls find_in_batches' do
let(:metric_method) { :find_in_batches }
it 'raises an error for jira_usage' do
expect { described_class.jira_usage }.to raise_error(ActiveRecord::StatementInvalid)
end
end
end
context 'with should_raise_for_dev? false' do
let(:should_raise_for_dev) { false }
it 'does not raise an error' do
expect { subject }.not_to raise_error
end
expect { subject }.not_to raise_error context 'when metric calls find_in_batches' do
let(:metric_method) { :find_in_batches }
it 'does not raise an error for jira_usage' do
expect { described_class.jira_usage }.not_to raise_error
end
end
end
end end
it 'includes a recording_ce_finished_at timestamp' do it 'includes a recording_ce_finished_at timestamp' do
expect(subject[:recording_ce_finished_at]).to be_a(Time) expect(subject[:recording_ce_finished_at]).to be_a(Time)
end end
it 'jira usage works when queries time out' do
allow_any_instance_of(ActiveRecord::Relation)
.to receive(:find_in_batches).and_raise(ActiveRecord::StatementInvalid.new(''))
expect { described_class.jira_usage }.not_to raise_error
end
end end
describe '.system_usage_data_monthly' do describe '.system_usage_data_monthly' do
...@@ -1355,46 +1382,58 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do ...@@ -1355,46 +1382,58 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
context 'when queries time out' do context 'when queries time out' do
before do before do
allow_any_instance_of(ActiveRecord::Relation) allow_any_instance_of(ActiveRecord::Relation).to receive(:count).and_raise(ActiveRecord::StatementInvalid)
.to receive(:count).and_raise(ActiveRecord::StatementInvalid.new('')) allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev)
end end
it 'returns -1 for email campaign data' do context 'with should_raise_for_dev? true' do
expected_data = { let(:should_raise_for_dev) { true }
"in_product_marketing_email_create_0_sent" => -1,
"in_product_marketing_email_create_0_cta_clicked" => -1,
"in_product_marketing_email_create_1_sent" => -1,
"in_product_marketing_email_create_1_cta_clicked" => -1,
"in_product_marketing_email_create_2_sent" => -1,
"in_product_marketing_email_create_2_cta_clicked" => -1,
"in_product_marketing_email_team_short_0_sent" => -1,
"in_product_marketing_email_team_short_0_cta_clicked" => -1,
"in_product_marketing_email_trial_short_0_sent" => -1,
"in_product_marketing_email_trial_short_0_cta_clicked" => -1,
"in_product_marketing_email_admin_verify_0_sent" => -1,
"in_product_marketing_email_admin_verify_0_cta_clicked" => -1,
"in_product_marketing_email_verify_0_sent" => -1,
"in_product_marketing_email_verify_0_cta_clicked" => -1,
"in_product_marketing_email_verify_1_sent" => -1,
"in_product_marketing_email_verify_1_cta_clicked" => -1,
"in_product_marketing_email_verify_2_sent" => -1,
"in_product_marketing_email_verify_2_cta_clicked" => -1,
"in_product_marketing_email_trial_0_sent" => -1,
"in_product_marketing_email_trial_0_cta_clicked" => -1,
"in_product_marketing_email_trial_1_sent" => -1,
"in_product_marketing_email_trial_1_cta_clicked" => -1,
"in_product_marketing_email_trial_2_sent" => -1,
"in_product_marketing_email_trial_2_cta_clicked" => -1,
"in_product_marketing_email_team_0_sent" => -1,
"in_product_marketing_email_team_0_cta_clicked" => -1,
"in_product_marketing_email_team_1_sent" => -1,
"in_product_marketing_email_team_1_cta_clicked" => -1,
"in_product_marketing_email_team_2_sent" => -1,
"in_product_marketing_email_team_2_cta_clicked" => -1,
"in_product_marketing_email_experience_0_sent" => -1
}
expect(subject).to eq(expected_data) it 'raises an error' do
expect { subject }.to raise_error(ActiveRecord::StatementInvalid)
end
end
context 'with should_raise_for_dev? false' do
let(:should_raise_for_dev) { false }
it 'returns -1 for email campaign data' do
expected_data = {
"in_product_marketing_email_create_0_sent" => -1,
"in_product_marketing_email_create_0_cta_clicked" => -1,
"in_product_marketing_email_create_1_sent" => -1,
"in_product_marketing_email_create_1_cta_clicked" => -1,
"in_product_marketing_email_create_2_sent" => -1,
"in_product_marketing_email_create_2_cta_clicked" => -1,
"in_product_marketing_email_team_short_0_sent" => -1,
"in_product_marketing_email_team_short_0_cta_clicked" => -1,
"in_product_marketing_email_trial_short_0_sent" => -1,
"in_product_marketing_email_trial_short_0_cta_clicked" => -1,
"in_product_marketing_email_admin_verify_0_sent" => -1,
"in_product_marketing_email_admin_verify_0_cta_clicked" => -1,
"in_product_marketing_email_verify_0_sent" => -1,
"in_product_marketing_email_verify_0_cta_clicked" => -1,
"in_product_marketing_email_verify_1_sent" => -1,
"in_product_marketing_email_verify_1_cta_clicked" => -1,
"in_product_marketing_email_verify_2_sent" => -1,
"in_product_marketing_email_verify_2_cta_clicked" => -1,
"in_product_marketing_email_trial_0_sent" => -1,
"in_product_marketing_email_trial_0_cta_clicked" => -1,
"in_product_marketing_email_trial_1_sent" => -1,
"in_product_marketing_email_trial_1_cta_clicked" => -1,
"in_product_marketing_email_trial_2_sent" => -1,
"in_product_marketing_email_trial_2_cta_clicked" => -1,
"in_product_marketing_email_team_0_sent" => -1,
"in_product_marketing_email_team_0_cta_clicked" => -1,
"in_product_marketing_email_team_1_sent" => -1,
"in_product_marketing_email_team_1_cta_clicked" => -1,
"in_product_marketing_email_team_2_sent" => -1,
"in_product_marketing_email_team_2_cta_clicked" => -1,
"in_product_marketing_email_experience_0_sent" => -1
}
expect(subject).to eq(expected_data)
end
end end
end end
......
...@@ -5,6 +5,30 @@ require 'spec_helper' ...@@ -5,6 +5,30 @@ require 'spec_helper'
RSpec.describe Gitlab::Utils::UsageData do RSpec.describe Gitlab::Utils::UsageData do
include Database::DatabaseHelpers include Database::DatabaseHelpers
shared_examples 'failing hardening method' do
before do
allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev)
stub_const("Gitlab::Utils::UsageData::FALLBACK", fallback)
allow(failing_class).to receive(failing_method).and_raise(ActiveRecord::StatementInvalid)
end
context 'with should_raise_for_dev? false' do
let(:should_raise_for_dev) { false }
it 'returns the fallback' do
expect(subject).to eq(fallback)
end
end
context 'with should_raise_for_dev? true' do
let(:should_raise_for_dev) { true }
it 'raises an error' do
expect { subject }.to raise_error(ActiveRecord::StatementInvalid)
end
end
end
describe '#add_metric' do describe '#add_metric' do
let(:metric) { 'UuidMetric'} let(:metric) { 'UuidMetric'}
...@@ -22,11 +46,14 @@ RSpec.describe Gitlab::Utils::UsageData do ...@@ -22,11 +46,14 @@ RSpec.describe Gitlab::Utils::UsageData do
expect(described_class.count(relation, batch: false)).to eq(1) expect(described_class.count(relation, batch: false)).to eq(1)
end end
it 'returns the fallback value when counting fails' do context 'when counting fails' do
stub_const("Gitlab::Utils::UsageData::FALLBACK", 15) subject { described_class.count(relation, batch: false) }
allow(relation).to receive(:count).and_raise(ActiveRecord::StatementInvalid.new(''))
expect(described_class.count(relation, batch: false)).to eq(15) let(:fallback) { 15 }
let(:failing_class) { relation }
let(:failing_method) { :count }
it_behaves_like 'failing hardening method'
end end
end end
...@@ -39,11 +66,14 @@ RSpec.describe Gitlab::Utils::UsageData do ...@@ -39,11 +66,14 @@ RSpec.describe Gitlab::Utils::UsageData do
expect(described_class.distinct_count(relation, batch: false)).to eq(1) expect(described_class.distinct_count(relation, batch: false)).to eq(1)
end end
it 'returns the fallback value when counting fails' do context 'when counting fails' do
stub_const("Gitlab::Utils::UsageData::FALLBACK", 15) subject { described_class.distinct_count(relation, batch: false) }
allow(relation).to receive(:distinct_count_by).and_raise(ActiveRecord::StatementInvalid.new(''))
let(:fallback) { 15 }
let(:failing_class) { relation }
let(:failing_method) { :distinct_count_by }
expect(described_class.distinct_count(relation, batch: false)).to eq(15) it_behaves_like 'failing hardening method'
end end
end end
...@@ -155,14 +185,24 @@ RSpec.describe Gitlab::Utils::UsageData do ...@@ -155,14 +185,24 @@ RSpec.describe Gitlab::Utils::UsageData do
stub_const("Gitlab::Utils::UsageData::DISTRIBUTED_HLL_FALLBACK", 4) stub_const("Gitlab::Utils::UsageData::DISTRIBUTED_HLL_FALLBACK", 4)
end end
it 'returns fallback if counter raises WRONG_CONFIGURATION_ERROR' do context 'when counter raises WRONG_CONFIGURATION_ERROR' do
expect(described_class.estimate_batch_distinct_count(relation, 'id', start: 1, finish: 0)).to eq 3 subject { described_class.estimate_batch_distinct_count(relation, 'id', start: 1, finish: 0) }
let(:fallback) { 3 }
let(:failing_class) { Gitlab::Database::PostgresHll::BatchDistinctCounter }
let(:failing_method) { :new }
it_behaves_like 'failing hardening method'
end end
it 'returns default fallback value when counting fails due to database error' do context 'when counting fails due to database error' do
allow(Gitlab::Database::PostgresHll::BatchDistinctCounter).to receive(:new).and_raise(ActiveRecord::StatementInvalid.new('')) subject { described_class.estimate_batch_distinct_count(relation) }
expect(described_class.estimate_batch_distinct_count(relation)).to eq(3) let(:fallback) { 3 }
let(:failing_class) { Gitlab::Database::PostgresHll::BatchDistinctCounter }
let(:failing_method) { :new }
it_behaves_like 'failing hardening method'
end end
it 'logs error and returns DISTRIBUTED_HLL_FALLBACK value when counting raises any error', :aggregate_failures do it 'logs error and returns DISTRIBUTED_HLL_FALLBACK value when counting raises any error', :aggregate_failures do
...@@ -187,13 +227,14 @@ RSpec.describe Gitlab::Utils::UsageData do ...@@ -187,13 +227,14 @@ RSpec.describe Gitlab::Utils::UsageData do
expect(described_class.sum(relation, :column, batch_size: 100, start: 2, finish: 3)).to eq(1) expect(described_class.sum(relation, :column, batch_size: 100, start: 2, finish: 3)).to eq(1)
end end
it 'returns the fallback value when counting fails' do context 'when counting fails' do
stub_const("Gitlab::Utils::UsageData::FALLBACK", 15) subject { described_class.sum(relation, :column) }
allow(Gitlab::Database::BatchCount)
.to receive(:batch_sum) let(:fallback) { 15 }
.and_raise(ActiveRecord::StatementInvalid.new('')) let(:failing_class) { Gitlab::Database::BatchCount }
let(:failing_method) { :batch_sum }
expect(described_class.sum(relation, :column)).to eq(15) it_behaves_like 'failing hardening method'
end end
end end
...@@ -273,23 +314,45 @@ RSpec.describe Gitlab::Utils::UsageData do ...@@ -273,23 +314,45 @@ RSpec.describe Gitlab::Utils::UsageData do
expect(histogram).to eq('2' => 1) expect(histogram).to eq('2' => 1)
end end
it 'returns fallback and logs canceled queries' do context 'when query timeout' do
create(:alert_management_http_integration, :active, project: project1) subject do
with_statement_timeout(0.001) do
relation = AlertManagement::HttpIntegration.select('pg_sleep(0.002)')
described_class.histogram(relation, column, buckets: 1..100)
end
end
expect(Gitlab::AppJsonLogger).to receive(:error).with( before do
event: 'histogram', allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev)
relation: relation.table_name, create(:alert_management_http_integration, :active, project: project1)
operation: 'histogram', end
operation_args: [column, 1, 100, 99],
query: kind_of(String), context 'with should_raise_for_dev? false' do
message: /PG::QueryCanceled/ let(:should_raise_for_dev) { false }
)
it 'logs canceled queries' do
expect(Gitlab::AppJsonLogger).to receive(:error).with(
event: 'histogram',
relation: relation.table_name,
operation: 'histogram',
operation_args: [column, 1, 100, 99],
query: kind_of(String),
message: /PG::QueryCanceled/
)
subject
end
with_statement_timeout(0.001) do it 'returns fallback' do
relation = AlertManagement::HttpIntegration.select('pg_sleep(0.002)') expect(subject).to eq(fallback)
histogram = described_class.histogram(relation, column, buckets: 1..100) end
end
expect(histogram).to eq(fallback) context 'with should_raise_for_dev? true' do
let(:should_raise_for_dev) { true }
it 'raises error' do
expect { subject }.to raise_error(ActiveRecord::QueryCanceled)
end
end 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