diff options
Diffstat (limited to 'spec/lib/gitlab/usage')
14 files changed, 392 insertions, 414 deletions
diff --git a/spec/lib/gitlab/usage/metric_definition_spec.rb b/spec/lib/gitlab/usage/metric_definition_spec.rb index a1bddcb3a47..931340947a2 100644 --- a/spec/lib/gitlab/usage/metric_definition_spec.rb +++ b/spec/lib/gitlab/usage/metric_definition_spec.rb @@ -74,13 +74,12 @@ RSpec.describe Gitlab::Usage::MetricDefinition do end describe '#with_instrumentation_class' do - let(:metric_status) { 'active' } let(:all_definitions) do metrics_definitions = [ - { key_path: 'metric1', instrumentation_class: 'RedisHLLMetric', status: 'data_available' }, - { key_path: 'metric2', instrumentation_class: 'RedisHLLMetric', status: 'implemented' }, - { key_path: 'metric3', instrumentation_class: 'RedisHLLMetric', status: 'deprecated' }, - { key_path: 'metric4', instrumentation_class: 'RedisHLLMetric', status: metric_status }, + { key_path: 'metric1', instrumentation_class: 'RedisHLLMetric', status: 'active' }, + { key_path: 'metric2', instrumentation_class: 'RedisHLLMetric', status: 'broken' }, + { key_path: 'metric3', instrumentation_class: 'RedisHLLMetric', status: 'active' }, + { key_path: 'metric4', instrumentation_class: 'RedisHLLMetric', status: 'removed' }, { key_path: 'metric5', status: 'active' }, { key_path: 'metric_missing_status' } ] @@ -92,7 +91,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do end it 'includes definitions with instrumentation_class' do - expect(described_class.with_instrumentation_class.count).to eq(4) + expect(described_class.with_instrumentation_class.count).to eq(3) end context 'with removed metric' do @@ -135,8 +134,9 @@ RSpec.describe Gitlab::Usage::MetricDefinition do :repair_issue_url | nil :removed_by_url | 1 - :instrumentation_class | 'Metric_Class' - :instrumentation_class | 'metricClass' + :performance_indicator_type | nil + :instrumentation_class | 'Metric_Class' + :instrumentation_class | 'metricClass' end with_them do @@ -201,9 +201,9 @@ RSpec.describe Gitlab::Usage::MetricDefinition do using RSpec::Parameterized::TableSyntax where(:status, :skip_validation?) do - 'deprecated' | true - 'removed' | true 'active' | false + 'broken' | false + 'removed' | true end with_them do diff --git a/spec/lib/gitlab/usage/metrics/aggregates/aggregate_spec.rb b/spec/lib/gitlab/usage/metrics/aggregates/aggregate_spec.rb index 76eec2755df..1f00f7bbec3 100644 --- a/spec/lib/gitlab/usage/metrics/aggregates/aggregate_spec.rb +++ b/spec/lib/gitlab/usage/metrics/aggregates/aggregate_spec.rb @@ -3,403 +3,133 @@ require 'spec_helper' RSpec.describe Gitlab::Usage::Metrics::Aggregates::Aggregate, :clean_gitlab_redis_shared_state do - let(:entity1) { 'dfb9d2d2-f56c-4c77-8aeb-6cddc4a1f857' } - let(:entity2) { '1dd9afb2-a3ee-4de1-8ae3-a405579c8584' } - let(:entity3) { '34rfjuuy-ce56-sa35-ds34-dfer567dfrf2' } - let(:entity4) { '8b9a2671-2abf-4bec-a682-22f6a8f7bf31' } let(:end_date) { Date.current } - let(:sources) { Gitlab::Usage::Metrics::Aggregates::Sources } let(:namespace) { described_class.to_s.deconstantize.constantize } + let(:sources) { Gitlab::Usage::Metrics::Aggregates::Sources } let_it_be(:recorded_at) { Time.current.to_i } - def aggregated_metric(name:, time_frame:, source: "redis", events: %w[event1 event2 event3], operator: "OR", feature_flag: nil) - { - name: name, - source: source, - events: events, - operator: operator, - time_frame: time_frame, - feature_flag: feature_flag - }.compact.with_indifferent_access - end - - context 'aggregated_metrics_data' do - shared_examples 'aggregated_metrics_data' do - context 'no aggregated metric is defined' do - it 'returns empty hash' do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics).and_return([]) - end - - expect(aggregated_metrics_data).to eq({}) - end + describe '.calculate_count_for_aggregation' do + using RSpec::Parameterized::TableSyntax + + context 'with valid configuration' do + where(:number_of_days, :operator, :datasource, :expected_method) do + 28 | 'AND' | 'redis_hll' | :calculate_metrics_intersections + 7 | 'AND' | 'redis_hll' | :calculate_metrics_intersections + 28 | 'AND' | 'database' | :calculate_metrics_intersections + 7 | 'AND' | 'database' | :calculate_metrics_intersections + 28 | 'OR' | 'redis_hll' | :calculate_metrics_union + 7 | 'OR' | 'redis_hll' | :calculate_metrics_union + 28 | 'OR' | 'database' | :calculate_metrics_union + 7 | 'OR' | 'database' | :calculate_metrics_union end - context 'there are aggregated metrics defined' do - let(:aggregated_metrics) do - [ - aggregated_metric(name: "gmau_1", source: datasource, time_frame: time_frame, operator: operator) - ] - end - - let(:results) { { 'gmau_1' => 5 } } + with_them do + let(:time_frame) { "#{number_of_days}d" } + let(:start_date) { number_of_days.days.ago.to_date } let(:params) { { start_date: start_date, end_date: end_date, recorded_at: recorded_at } } - - before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics).and_return(aggregated_metrics) - end - end - - context 'with OR operator' do - let(:operator) { Gitlab::Usage::Metrics::Aggregates::UNION_OF_AGGREGATED_METRICS } - - it 'returns the number of unique events occurred for any metric in aggregate', :aggregate_failures do - expect(namespace::SOURCES[datasource]).to receive(:calculate_metrics_union).with(params.merge(metric_names: %w[event1 event2 event3])).and_return(5) - expect(aggregated_metrics_data).to eq(results) - end + let(:aggregate) do + { + source: datasource, + operator: operator, + events: %w[event1 event2] + } end - context 'with AND operator' do - let(:operator) { Gitlab::Usage::Metrics::Aggregates::INTERSECTION_OF_AGGREGATED_METRICS } - - it 'returns the number of unique events that occurred for all of metrics in the aggregate', :aggregate_failures do - expect(namespace::SOURCES[datasource]).to receive(:calculate_metrics_intersections).with(params.merge(metric_names: %w[event1 event2 event3])).and_return(5) - expect(aggregated_metrics_data).to eq(results) - end - end - - context 'hidden behind feature flag' do - let(:enabled_feature_flag) { 'test_ff_enabled' } - let(:disabled_feature_flag) { 'test_ff_disabled' } - let(:aggregated_metrics) do - params = { source: datasource, time_frame: time_frame } - [ - # represents stable aggregated metrics that has been fully released - aggregated_metric(**params.merge(name: "gmau_without_ff")), - # represents new aggregated metric that is under performance testing on gitlab.com - aggregated_metric(**params.merge(name: "gmau_enabled", feature_flag: enabled_feature_flag)), - # represents aggregated metric that is under development and shouldn't be yet collected even on gitlab.com - aggregated_metric(**params.merge(name: "gmau_disabled", feature_flag: disabled_feature_flag)) - ] - end - - it 'does not calculate data for aggregates with ff turned off' do - skip_feature_flags_yaml_validation - skip_default_enabled_yaml_check - stub_feature_flags(enabled_feature_flag => true, disabled_feature_flag => false) - allow(namespace::SOURCES[datasource]).to receive(:calculate_metrics_union).and_return(6) - - expect(aggregated_metrics_data).to eq('gmau_without_ff' => 6, 'gmau_enabled' => 6) - end - end - end - - context 'error handling' do - context 'development and test environment' do - it 'raises error when unknown aggregation operator is used' do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics) - .and_return([aggregated_metric(name: 'gmau_1', source: datasource, operator: "SUM", time_frame: time_frame)]) - end - - expect { aggregated_metrics_data }.to raise_error namespace::UnknownAggregationOperator - end - - it 'raises error when unknown aggregation source is used' do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics) - .and_return([aggregated_metric(name: 'gmau_1', source: 'whoami', time_frame: time_frame)]) - end - - expect { aggregated_metrics_data }.to raise_error namespace::UnknownAggregationSource - end - - it 'raises error when union is missing' do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics) - .and_return([aggregated_metric(name: 'gmau_1', source: datasource, time_frame: time_frame)]) - end - allow(namespace::SOURCES[datasource]).to receive(:calculate_metrics_union).and_raise(sources::UnionNotAvailable) - - expect { aggregated_metrics_data }.to raise_error sources::UnionNotAvailable - end + subject(:calculate_count_for_aggregation) do + described_class + .new(recorded_at) + .calculate_count_for_aggregation(aggregation: aggregate, time_frame: time_frame) end - context 'production' do - before do - stub_rails_env('production') - end - - it 'rescues unknown aggregation operator error' do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics) - .and_return([aggregated_metric(name: 'gmau_1', source: datasource, operator: "SUM", time_frame: time_frame)]) - end - - expect(aggregated_metrics_data).to eq('gmau_1' => -1) - end - - it 'rescues unknown aggregation source error' do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics) - .and_return([aggregated_metric(name: 'gmau_1', source: 'whoami', time_frame: time_frame)]) - end - - expect(aggregated_metrics_data).to eq('gmau_1' => -1) - end - - it 'rescues error when union is missing' do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics) - .and_return([aggregated_metric(name: 'gmau_1', source: datasource, time_frame: time_frame)]) - end - allow(namespace::SOURCES[datasource]).to receive(:calculate_metrics_union).and_raise(sources::UnionNotAvailable) - - expect(aggregated_metrics_data).to eq('gmau_1' => -1) - end + it 'returns the number of unique events for aggregation', :aggregate_failures do + expect(namespace::SOURCES[datasource]) + .to receive(expected_method) + .with(params.merge(metric_names: %w[event1 event2])) + .and_return(5) + expect(calculate_count_for_aggregation).to eq(5) end end end - shared_examples 'database_sourced_aggregated_metrics' do - let(:datasource) { namespace::DATABASE_SOURCE } - - it_behaves_like 'aggregated_metrics_data' - end - - shared_examples 'redis_sourced_aggregated_metrics' do - let(:datasource) { namespace::REDIS_SOURCE } - - it_behaves_like 'aggregated_metrics_data' do - context 'error handling' do - let(:aggregated_metrics) { [aggregated_metric(name: 'gmau_1', source: datasource, time_frame: time_frame)] } - let(:error) { Gitlab::UsageDataCounters::HLLRedisCounter::EventError } - - before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics).and_return(aggregated_metrics) - end - allow(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:calculate_events_union).and_raise(error) - end - - context 'development and test environment' do - it 're raises Gitlab::UsageDataCounters::HLLRedisCounter::EventError' do - expect { aggregated_metrics_data }.to raise_error error - end - end - - context 'production' do - it 'rescues Gitlab::UsageDataCounters::HLLRedisCounter::EventError' do - stub_rails_env('production') - - expect(aggregated_metrics_data).to eq('gmau_1' => -1) - end - end - end + context 'with invalid configuration' do + where(:time_frame, :operator, :datasource, :expected_error) do + '28d' | 'SUM' | 'redis_hll' | namespace::UnknownAggregationOperator + '7d' | 'AND' | 'mongodb' | namespace::UnknownAggregationSource + 'all' | 'AND' | 'redis_hll' | namespace::DisallowedAggregationTimeFrame end - end - - describe '.aggregated_metrics_all_time_data' do - subject(:aggregated_metrics_data) { described_class.new(recorded_at).all_time_data } - let(:start_date) { nil } - let(:end_date) { nil } - let(:time_frame) { ['all'] } - - it_behaves_like 'database_sourced_aggregated_metrics' - - context 'redis sourced aggregated metrics' do - let(:aggregated_metrics) { [aggregated_metric(name: 'gmau_1', time_frame: time_frame)] } + with_them do + let(:aggregate) do + { + source: datasource, + operator: operator, + events: %w[event1 event2] + } + end - before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:aggregated_metrics).and_return(aggregated_metrics) - end + subject(:calculate_count_for_aggregation) do + described_class + .new(recorded_at) + .calculate_count_for_aggregation(aggregation: aggregate, time_frame: time_frame) end - context 'development and test environment' do - it 'raises Gitlab::Usage::Metrics::Aggregates::DisallowedAggregationTimeFrame' do - expect { aggregated_metrics_data }.to raise_error namespace::DisallowedAggregationTimeFrame + context 'with non prod environment' do + it 'raises error' do + expect { calculate_count_for_aggregation }.to raise_error expected_error end end - context 'production env' do - it 'returns fallback value for unsupported time frame' do + context 'with prod environment' do + before do stub_rails_env('production') + end - expect(aggregated_metrics_data).to eq('gmau_1' => -1) + it 'returns fallback value' do + expect(calculate_count_for_aggregation).to be(-1) end end end end - context 'legacy aggregated metrics configuration' do - let(:temp_dir) { Dir.mktmpdir } - let(:temp_file) { Tempfile.new(%w[common .yml], temp_dir) } - - before do - stub_const("#{namespace}::AGGREGATED_METRICS_PATH", File.expand_path('*.yml', temp_dir)) - File.open(temp_file.path, "w+b") do |file| - file.write [aggregated_metric(name: "gmau_1", time_frame: '7d')].to_yaml - end - end - - after do - temp_file.unlink - FileUtils.remove_entry(temp_dir) if Dir.exist?(temp_dir) + context 'when union data is not available' do + subject(:calculate_count_for_aggregation) do + described_class + .new(recorded_at) + .calculate_count_for_aggregation(aggregation: aggregate, time_frame: time_frame) end - it 'allows for YAML aliases in aggregated metrics configs' do - expect(YAML).to receive(:safe_load).with(kind_of(String), aliases: true).at_least(:once) - - described_class.new(recorded_at) + where(:time_frame, :operator, :datasource) do + '28d' | 'OR' | 'redis_hll' + '7d' | 'OR' | 'database' end - end - - describe '.aggregated_metrics_weekly_data' do - subject(:aggregated_metrics_data) { described_class.new(recorded_at).weekly_data } - let(:start_date) { 7.days.ago.to_date } - let(:time_frame) { ['7d'] } - - it_behaves_like 'database_sourced_aggregated_metrics' - it_behaves_like 'redis_sourced_aggregated_metrics' - end - - describe '.aggregated_metrics_monthly_data' do - subject(:aggregated_metrics_data) { described_class.new(recorded_at).monthly_data } - - let(:start_date) { 4.weeks.ago.to_date } - let(:time_frame) { ['28d'] } - - it_behaves_like 'database_sourced_aggregated_metrics' - it_behaves_like 'redis_sourced_aggregated_metrics' - end - - describe '.calculate_count_for_aggregation' do - using RSpec::Parameterized::TableSyntax - - context 'with valid configuration' do - where(:number_of_days, :operator, :datasource, :expected_method) do - 28 | 'AND' | 'redis' | :calculate_metrics_intersections - 7 | 'AND' | 'redis' | :calculate_metrics_intersections - 28 | 'AND' | 'database' | :calculate_metrics_intersections - 7 | 'AND' | 'database' | :calculate_metrics_intersections - 28 | 'OR' | 'redis' | :calculate_metrics_union - 7 | 'OR' | 'redis' | :calculate_metrics_union - 28 | 'OR' | 'database' | :calculate_metrics_union - 7 | 'OR' | 'database' | :calculate_metrics_union - end - - with_them do - let(:time_frame) { "#{number_of_days}d" } - let(:start_date) { number_of_days.days.ago.to_date } - let(:params) { { start_date: start_date, end_date: end_date, recorded_at: recorded_at } } - let(:aggregate) do - { - source: datasource, - operator: operator, - events: %w[event1 event2] - } - end - - subject(:calculate_count_for_aggregation) do - described_class - .new(recorded_at) - .calculate_count_for_aggregation(aggregation: aggregate, time_frame: time_frame) - end - - it 'returns the number of unique events for aggregation', :aggregate_failures do - expect(namespace::SOURCES[datasource]) - .to receive(expected_method) - .with(params.merge(metric_names: %w[event1 event2])) - .and_return(5) - expect(calculate_count_for_aggregation).to eq(5) - end + with_them do + before do + allow(namespace::SOURCES[datasource]).to receive(:calculate_metrics_union).and_raise(sources::UnionNotAvailable) end - end - context 'with invalid configuration' do - where(:time_frame, :operator, :datasource, :expected_error) do - '28d' | 'SUM' | 'redis' | namespace::UnknownAggregationOperator - '7d' | 'AND' | 'mongodb' | namespace::UnknownAggregationSource - 'all' | 'AND' | 'redis' | namespace::DisallowedAggregationTimeFrame + let(:aggregate) do + { + source: datasource, + operator: operator, + events: %w[event1 event2] + } end - with_them do - let(:aggregate) do - { - source: datasource, - operator: operator, - events: %w[event1 event2] - } - end - - subject(:calculate_count_for_aggregation) do - described_class - .new(recorded_at) - .calculate_count_for_aggregation(aggregation: aggregate, time_frame: time_frame) - end - - context 'with non prod environment' do - it 'raises error' do - expect { calculate_count_for_aggregation }.to raise_error expected_error - end - end - - context 'with prod environment' do - before do - stub_rails_env('production') - end - - it 'returns fallback value' do - expect(calculate_count_for_aggregation).to be(-1) - end + context 'with non prod environment' do + it 'raises error' do + expect { calculate_count_for_aggregation }.to raise_error sources::UnionNotAvailable end end - end - - context 'when union data is not available' do - subject(:calculate_count_for_aggregation) do - described_class - .new(recorded_at) - .calculate_count_for_aggregation(aggregation: aggregate, time_frame: time_frame) - end - where(:time_frame, :operator, :datasource) do - '28d' | 'OR' | 'redis' - '7d' | 'OR' | 'database' - end - - with_them do + context 'with prod environment' do before do - allow(namespace::SOURCES[datasource]).to receive(:calculate_metrics_union).and_raise(sources::UnionNotAvailable) - end - - let(:aggregate) do - { - source: datasource, - operator: operator, - events: %w[event1 event2] - } - end - - context 'with non prod environment' do - it 'raises error' do - expect { calculate_count_for_aggregation }.to raise_error sources::UnionNotAvailable - end + stub_rails_env('production') end - context 'with prod environment' do - before do - stub_rails_env('production') - end - - it 'returns fallback value' do - expect(calculate_count_for_aggregation).to be(-1) - end + it 'returns fallback value' do + expect(calculate_count_for_aggregation).to be(-1) end end end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_merge_request_authors_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_merge_request_authors_metric_spec.rb new file mode 100644 index 00000000000..92459e92eac --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_merge_request_authors_metric_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CountMergeRequestAuthorsMetric do + let(:expected_value) { 1 } + let(:start) { 30.days.ago.to_s(:db) } + let(:finish) { 2.days.ago.to_s(:db) } + + let(:expected_query) do + "SELECT COUNT(DISTINCT \"merge_requests\".\"author_id\") FROM \"merge_requests\"" \ + " WHERE \"merge_requests\".\"created_at\" BETWEEN '#{start}' AND '#{finish}'" + end + + before do + user = create(:user) + user2 = create(:user) + + create(:merge_request, created_at: 1.year.ago, author: user) + create(:merge_request, created_at: 1.week.ago, author: user2) + create(:merge_request, created_at: 1.week.ago, author: user2) + end + + it_behaves_like 'a correct instrumented metric value and query', { time_frame: '28d' } +end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/database_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/database_metric_spec.rb index f73155642d6..f1ecc8c8ab5 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/database_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/database_metric_spec.rb @@ -3,12 +3,14 @@ require 'spec_helper' RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do + let(:database_metric_class) { Class.new(described_class) } + subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation :count - metric_class.start { metric_class.relation.minimum(:id) } - metric_class.finish { metric_class.relation.maximum(:id) } + metric_class.start { Issue.minimum(:id) } + metric_class.finish { Issue.maximum(:id) } end.new(time_frame: 'all') end @@ -38,11 +40,11 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do context 'with metric options specified with custom batch_size' do subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation :count - metric_class.start { metric_class.relation.minimum(:id) } - metric_class.finish { metric_class.relation.maximum(:id) } + metric_class.start { Issue.minimum(:id) } + metric_class.finish { Issue.maximum(:id) } metric_class.metric_options { { batch_size: 12345 } } end.new(time_frame: 'all') end @@ -60,7 +62,7 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do context 'with start and finish not called' do subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation :count end.new(time_frame: 'all') @@ -73,7 +75,7 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do context 'with availability defined' do subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation :count metric_class.available? { false } @@ -87,7 +89,7 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do context 'with availability not defined' do subject do - Class.new(described_class) do + database_metric_class do relation { Issue } operation :count end.new(time_frame: 'all') @@ -100,11 +102,11 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do context 'with cache_start_and_finish_as called' do subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation :count - metric_class.start { metric_class.relation.minimum(:id) } - metric_class.finish { metric_class.relation.maximum(:id) } + metric_class.start { Issue.minimum(:id) } + metric_class.finish { Issue.maximum(:id) } metric_class.cache_start_and_finish_as :special_issue_count end.new(time_frame: 'all') end @@ -123,11 +125,11 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do context 'with estimate_batch_distinct_count' do subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation(:estimate_batch_distinct_count) - metric_class.start { metric_class.relation.minimum(:id) } - metric_class.finish { metric_class.relation.maximum(:id) } + metric_class.start { Issue.minimum(:id) } + metric_class.finish { Issue.maximum(:id) } end.new(time_frame: 'all') end @@ -139,13 +141,13 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do let(:buckets) { double('Buckets').as_null_object } subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation(:estimate_batch_distinct_count) do |result| result.foo end - metric_class.start { metric_class.relation.minimum(:id) } - metric_class.finish { metric_class.relation.maximum(:id) } + metric_class.start { Issue.minimum(:id) } + metric_class.finish { Issue.maximum(:id) } end.new(time_frame: 'all') end @@ -163,7 +165,7 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do context 'with custom timestamp column' do subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation :count metric_class.timestamp_column :last_edited_at @@ -171,6 +173,7 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do end it 'calculates a correct result' do + create(:issue, last_edited_at: 40.days.ago) create(:issue, last_edited_at: 5.days.ago) expect(subject.value).to eq(1) @@ -179,24 +182,40 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DatabaseMetric do context 'with default timestamp column' do subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation :count end.new(time_frame: '28d') end it 'calculates a correct result' do - create(:issue, last_edited_at: 5.days.ago) + create(:issue, created_at: 40.days.ago) create(:issue, created_at: 5.days.ago) expect(subject.value).to eq(1) end end + + context 'with additional parameters passed via options' do + subject do + database_metric_class.tap do |metric_class| + metric_class.relation ->(options) { Issue.where(confidential: options[:confidential]) } + metric_class.operation :count + end.new(time_frame: '28d', options: { confidential: true }) + end + + it 'calculates a correct result' do + create(:issue, created_at: 5.days.ago, confidential: true) + create(:issue, created_at: 5.days.ago, confidential: false) + + expect(subject.value).to eq(1) + end + end end context 'with unimplemented operation method used' do subject do - described_class.tap do |metric_class| + database_metric_class.tap do |metric_class| metric_class.relation { Issue } metric_class.operation :invalid_operation end.new(time_frame: 'all') diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/distinct_count_projects_with_expiration_policy_disabled_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/distinct_count_projects_with_expiration_policy_disabled_metric_spec.rb deleted file mode 100644 index 757adee6117..00000000000 --- a/spec/lib/gitlab/usage/metrics/instrumentations/distinct_count_projects_with_expiration_policy_disabled_metric_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DistinctCountProjectsWithExpirationPolicyDisabledMetric do - before_all do - create(:container_expiration_policy, enabled: false) - create(:container_expiration_policy, enabled: false, created_at: 29.days.ago) - create(:container_expiration_policy, enabled: true) - end - - it_behaves_like 'a correct instrumented metric value', { time_frame: '28d' } do - let(:expected_value) { 1 } - end - - it_behaves_like 'a correct instrumented metric value', { time_frame: 'all' } do - let(:expected_value) { 2 } - end -end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/distinct_count_projects_with_expiration_policy_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/distinct_count_projects_with_expiration_policy_metric_spec.rb new file mode 100644 index 00000000000..a1ca658a0d7 --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/distinct_count_projects_with_expiration_policy_metric_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DistinctCountProjectsWithExpirationPolicyMetric do + before_all do + create(:container_expiration_policy, enabled: false) + create(:container_expiration_policy, enabled: false, created_at: 29.days.ago) + create(:container_expiration_policy, keep_n: nil) + create(:container_expiration_policy, keep_n: 5, enabled: true) + create(:container_expiration_policy, keep_n: 5, enabled: true) + create(:container_expiration_policy, keep_n: 5, enabled: true) + create(:container_expiration_policy, older_than: '7d') + create(:container_expiration_policy, cadence: '14d') + create(:container_expiration_policy, enabled: true) + end + + it_behaves_like 'a correct instrumented metric value', { time_frame: '28d', options: { enabled: false } } do + let(:expected_value) { 1 } + end + + it_behaves_like 'a correct instrumented metric value', { time_frame: 'all', options: { enabled: false } } do + let(:expected_value) { 2 } + end + + it_behaves_like 'a correct instrumented metric value', { time_frame: 'all', options: { keep_n: 5, enabled: true } } do + let(:expected_value) { 3 } + end + + it_behaves_like 'a correct instrumented metric value', { time_frame: 'all', options: { cadence: '14d' } } do + let(:expected_value) { 1 } + end +end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/dormant_user_period_setting_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/dormant_user_period_setting_metric_spec.rb new file mode 100644 index 00000000000..a63616aeb48 --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/dormant_user_period_setting_metric_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DormantUserPeriodSettingMetric do + using RSpec::Parameterized::TableSyntax + + where(:deactivate_dormant_users_period_value, :expected_value) do + 90 | 90 # default + 365 | 365 + end + + with_them do + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + stub_application_setting(deactivate_dormant_users_period: deactivate_dormant_users_period_value) + end + + it_behaves_like 'a correct instrumented metric value', {} + end +end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/dormant_user_setting_enabled_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/dormant_user_setting_enabled_metric_spec.rb new file mode 100644 index 00000000000..5c8ca502f82 --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/dormant_user_setting_enabled_metric_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::DormantUserSettingEnabledMetric do + using RSpec::Parameterized::TableSyntax + + where(:deactivate_dormant_users_enabled, :expected_value) do + 1 | 1 + 0 | 0 + end + + with_them do + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + stub_application_setting(deactivate_dormant_users: deactivate_dormant_users_enabled) + end + + it_behaves_like 'a correct instrumented metric value', {} + end +end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/in_product_marketing_email_cta_clicked_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/in_product_marketing_email_cta_clicked_metric_spec.rb new file mode 100644 index 00000000000..cb94da11d58 --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/in_product_marketing_email_cta_clicked_metric_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::InProductMarketingEmailCtaClickedMetric do + using RSpec::Parameterized::TableSyntax + + let(:email_attributes) { { cta_clicked_at: Date.yesterday, track: 'verify', series: 0 } } + let(:options) { { track: 'verify', series: 0 } } + let(:expected_value) { 2 } + let(:expected_query) do + 'SELECT COUNT("in_product_marketing_emails"."id") FROM "in_product_marketing_emails"' \ + ' WHERE "in_product_marketing_emails"."cta_clicked_at" IS NOT NULL' \ + ' AND "in_product_marketing_emails"."series" = 0'\ + ' AND "in_product_marketing_emails"."track" = 1' + end + + before do + create_list :in_product_marketing_email, 2, email_attributes + + create :in_product_marketing_email, email_attributes.merge(cta_clicked_at: nil) + create :in_product_marketing_email, email_attributes.merge(track: 'team') + create :in_product_marketing_email, email_attributes.merge(series: 1) + end + + it_behaves_like 'a correct instrumented metric value and query', { + options: { track: 'verify', series: 0 }, + time_frame: 'all' + } + + where(:options_key, :valid_value, :invalid_value) do + :track | 'admin_verify' | 'invite_team' + :series | 1 | 5 + end + + with_them do + it "raises an exception if option is not present" do + expect do + described_class.new(options: options.except(options_key), time_frame: 'all') + end.to raise_error(ArgumentError, %r{#{options_key} .* must be one of}) + end + + it "raises an exception if option has invalid value" do + expect do + options[options_key] = invalid_value + described_class.new(options: options, time_frame: 'all') + end.to raise_error(ArgumentError, %r{#{options_key} .* must be one of}) + end + + it "doesn't raise exceptions if option has valid value" do + options[options_key] = valid_value + described_class.new(options: options, time_frame: 'all') + end + end +end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/in_product_marketing_email_sent_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/in_product_marketing_email_sent_metric_spec.rb new file mode 100644 index 00000000000..0cc82773d56 --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/in_product_marketing_email_sent_metric_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::InProductMarketingEmailSentMetric do + using RSpec::Parameterized::TableSyntax + + let(:email_attributes) { { track: 'verify', series: 0 } } + let(:expected_value) { 2 } + let(:expected_query) do + 'SELECT COUNT("in_product_marketing_emails"."id") FROM "in_product_marketing_emails"' \ + ' WHERE "in_product_marketing_emails"."series" = 0'\ + ' AND "in_product_marketing_emails"."track" = 1' + end + + before do + create_list :in_product_marketing_email, 2, email_attributes + + create :in_product_marketing_email, email_attributes.merge(track: 'team') + create :in_product_marketing_email, email_attributes.merge(series: 1) + end + + it_behaves_like 'a correct instrumented metric value and query', { + options: { track: 'verify', series: 0 }, + time_frame: 'all' + } + + where(:options_key, :valid_value, :invalid_value) do + :track | 'admin_verify' | 'invite_team' + :series | 1 | 5 + end + + with_them do + it "raises an exception if option is not present" do + expect do + described_class.new(options: email_attributes.except(options_key), time_frame: 'all') + end.to raise_error(ArgumentError, %r{#{options_key} .* must be one of}) + end + + it "raises an exception if option has invalid value" do + expect do + email_attributes[options_key] = invalid_value + described_class.new(options: email_attributes, time_frame: 'all') + end.to raise_error(ArgumentError, %r{#{options_key} .* must be one of}) + end + + it "doesn't raise exceptions if option has valid value" do + email_attributes[options_key] = valid_value + described_class.new(options: email_attributes, time_frame: 'all') + end + end +end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/work_items_activity_aggregated_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/work_items_activity_aggregated_metric_spec.rb index 3e315692d0a..35e5d7f2796 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/work_items_activity_aggregated_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/work_items_activity_aggregated_metric_spec.rb @@ -15,6 +15,8 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::WorkItemsActivityAggreg users_creating_work_items users_updating_work_item_title users_updating_work_item_dates + users_updating_work_item_labels + users_updating_work_item_milestone users_updating_work_item_iteration ] } @@ -44,16 +46,26 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::WorkItemsActivityAggreg describe '#value', :clean_gitlab_redis_shared_state do let(:counter) { Gitlab::UsageDataCounters::HLLRedisCounter } + let(:author1_id) { 1 } + let(:author2_id) { 2 } + let(:event_time) { 1.week.ago } before do - counter.track_event(:users_creating_work_items, values: 1, time: 1.week.ago) - counter.track_event(:users_updating_work_item_title, values: 1, time: 1.week.ago) - counter.track_event(:users_updating_work_item_dates, values: 2, time: 1.week.ago) - counter.track_event(:users_updating_work_item_iteration, values: 2, time: 1.week.ago) + counter.track_event(:users_creating_work_items, values: author1_id, time: event_time) end - it 'has correct value' do - expect(described_class.new(metric_definition).value).to eq 2 + it 'has correct value after events are tracked', :aggregate_failures do + expect do + counter.track_event(:users_updating_work_item_title, values: author1_id, time: event_time) + counter.track_event(:users_updating_work_item_dates, values: author1_id, time: event_time) + counter.track_event(:users_updating_work_item_labels, values: author1_id, time: event_time) + counter.track_event(:users_updating_work_item_milestone, values: author1_id, time: event_time) + end.to not_change { described_class.new(metric_definition).value } + + expect do + counter.track_event(:users_updating_work_item_iteration, values: author2_id, time: event_time) + counter.track_event(:users_updating_weight_estimate, values: author1_id, time: event_time) + end.to change { described_class.new(metric_definition).value }.from(1).to(2) end end end diff --git a/spec/lib/gitlab/usage/metrics/name_suggestion_spec.rb b/spec/lib/gitlab/usage/metrics/name_suggestion_spec.rb index f9cd6e88e0a..24107727a8e 100644 --- a/spec/lib/gitlab/usage/metrics/name_suggestion_spec.rb +++ b/spec/lib/gitlab/usage/metrics/name_suggestion_spec.rb @@ -63,7 +63,6 @@ RSpec.describe Gitlab::Usage::Metrics::NameSuggestion do context 'for sum metrics' do it_behaves_like 'name suggestion' do # corresponding metric is collected with sum(JiraImportState.finished, :imported_issues_count) - let(:key_path) { 'counts.jira_imports_total_imported_issues_count' } let(:operation) { :sum } let(:relation) { JiraImportState.finished } let(:column) { :imported_issues_count } @@ -74,7 +73,6 @@ RSpec.describe Gitlab::Usage::Metrics::NameSuggestion do context 'for average metrics' do it_behaves_like 'name suggestion' do # corresponding metric is collected with average(Ci::Pipeline, :duration) - let(:key_path) { 'counts.ci_pipeline_duration' } let(:operation) { :average } let(:relation) { Ci::Pipeline } let(:column) { :duration } @@ -100,5 +98,16 @@ RSpec.describe Gitlab::Usage::Metrics::NameSuggestion do let(:name_suggestion) { /<please fill metric name>/ } end end + + context 'for metrics with `having` keyword' do + it_behaves_like 'name suggestion' do + let(:operation) { :count } + let(:relation) { Issue.with_alert_management_alerts.having('COUNT(alert_management_alerts) > 1').group(:id) } + + let(:column) { nil } + let(:constraints) { /<adjective describing: '\(\(COUNT\(alert_management_alerts\) > 1\)\)'>/ } + let(:name_suggestion) { /count_#{constraints}_issues_<with>_alert_management_alerts/ } + end + end end end diff --git a/spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/having_constraints_spec.rb b/spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/having_constraints_spec.rb new file mode 100644 index 00000000000..492acf2a902 --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/having_constraints_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::HavingConstraints do + describe '#accept' do + let(:connection) { ApplicationRecord.connection } + let(:collector) { Arel::Collectors::SubstituteBinds.new(connection, Arel::Collectors::SQLString.new) } + + it 'builds correct constraints description' do + table = Arel::Table.new('records') + havings = table[:attribute].sum.eq(6).and(table[:attribute].count.gt(5)) + arel = table.from.project(table['id'].count).having(havings).group(table[:attribute2]) + described_class.new(connection).accept(arel, collector) + + expect(collector.value).to eql '(SUM(records.attribute) = 6 AND COUNT(records.attribute) > 5)' + end + end +end diff --git a/spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/constraints_spec.rb b/spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/where_constraints_spec.rb index 68016e760e4..42a776478a4 100644 --- a/spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/constraints_spec.rb +++ b/spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/where_constraints_spec.rb @@ -2,14 +2,15 @@ require 'spec_helper' -RSpec.describe Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::Constraints do +RSpec.describe Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::WhereConstraints do describe '#accept' do - let(:collector) { Arel::Collectors::SubstituteBinds.new(ActiveRecord::Base.connection, Arel::Collectors::SQLString.new) } + let(:connection) { ApplicationRecord.connection } + let(:collector) { Arel::Collectors::SubstituteBinds.new(connection, Arel::Collectors::SQLString.new) } it 'builds correct constraints description' do table = Arel::Table.new('records') arel = table.from.project(table['id'].count).where(table[:attribute].eq(true).and(table[:some_value].gt(5))) - described_class.new(ApplicationRecord.connection).accept(arel, collector) + described_class.new(connection).accept(arel, collector) expect(collector.value).to eql '(records.attribute = true AND records.some_value > 5)' end |