diff --git a/lib/gitlab/usage/metrics/aggregates.rb b/lib/gitlab/usage/metrics/aggregates.rb index a3410cbc39351a29691b51f37a93c2614e3b95f2..698d6f3f6b516ae435d26a84a46e4e23ab084aca 100644 --- a/lib/gitlab/usage/metrics/aggregates.rb +++ b/lib/gitlab/usage/metrics/aggregates.rb @@ -4,10 +4,7 @@ module Gitlab module Usage module Metrics module Aggregates - UNION_OF_AGGREGATED_METRICS = 'OR' - ALLOWED_METRICS_AGGREGATIONS = [UNION_OF_AGGREGATED_METRICS].freeze AggregatedMetricError = Class.new(StandardError) - UnknownAggregationOperator = Class.new(AggregatedMetricError) UnknownAggregationSource = Class.new(AggregatedMetricError) DisallowedAggregationTimeFrame = Class.new(AggregatedMetricError) UndefinedEvents = Class.new(AggregatedMetricError) diff --git a/lib/gitlab/usage/metrics/aggregates/aggregate.rb b/lib/gitlab/usage/metrics/aggregates/aggregate.rb index 079fb39851cc3dd36657559165864638ca6a71c1..1b78cebf6a801495dbb5d9a932d4bbfb3dd0741f 100644 --- a/lib/gitlab/usage/metrics/aggregates/aggregate.rb +++ b/lib/gitlab/usage/metrics/aggregates/aggregate.rb @@ -31,13 +31,6 @@ def calculate_count_for_aggregation(aggregation:, time_frame:) def with_validate_configuration(aggregation, time_frame) source = aggregation[:source] - unless ALLOWED_METRICS_AGGREGATIONS.include?(aggregation[:operator]) - return failure( - UnknownAggregationOperator - .new("Events should be aggregated with one of operators #{ALLOWED_METRICS_AGGREGATIONS}") - ) - end - unless SOURCES[source] return failure( UnknownAggregationSource diff --git a/lib/gitlab/usage/metrics/instrumentations/aggregated_metric.rb b/lib/gitlab/usage/metrics/instrumentations/aggregated_metric.rb index 15eb12b05df055daf64965b08e4f4d9a75afd787..1990aa46e6ecaf0d287a878eec475e6abec1e230 100644 --- a/lib/gitlab/usage/metrics/instrumentations/aggregated_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/aggregated_metric.rb @@ -12,7 +12,6 @@ module Instrumentations # data_source: redis_hll # options: # aggregate: - # operator: OR # attribute: user_id # events: # - 'incident_management_alert_status_changed' @@ -55,7 +54,6 @@ def aggregate_config { source: source, events: options[:events], - operator: aggregate[:operator], attribute: attribute } end diff --git a/spec/lib/gitlab/usage/metrics/aggregates/aggregate_spec.rb b/spec/lib/gitlab/usage/metrics/aggregates/aggregate_spec.rb index edace79ad7a82f9b7f3fb45b866d99227a11986c..b1d192608068fd9bfcf67212e5a8ec6ffe4c2d39 100644 --- a/spec/lib/gitlab/usage/metrics/aggregates/aggregate_spec.rb +++ b/spec/lib/gitlab/usage/metrics/aggregates/aggregate_spec.rb @@ -19,13 +19,13 @@ end context 'with valid configuration' do - where(:number_of_days, :operator, :datasource, :attribute, :expected_method) do - 28 | 'OR' | 'redis_hll' | 'user.id' | :calculate_metrics_union - 7 | 'OR' | 'redis_hll' | 'project.id' | :calculate_metrics_union - 28 | 'OR' | 'database' | 'user.id' | :calculate_metrics_union - 7 | 'OR' | 'database' | 'user.id' | :calculate_metrics_union - 28 | 'OR' | 'internal_events' | 'user.id' | :calculate_metrics_union - 7 | 'OR' | 'internal_events' | 'user.id' | :calculate_metrics_union + where(:number_of_days, :datasource, :attribute) do + 28 | 'redis_hll' | 'user.id' + 7 | 'redis_hll' | 'project.id' + 28 | 'database' | 'user.id' + 7 | 'database' | 'user.id' + 28 | 'internal_events' | 'user.id' + 7 | 'internal_events' | 'user.id' end with_them do @@ -35,7 +35,6 @@ let(:aggregate) do { source: datasource, - operator: operator, events: %w[event1 event2], attribute: attribute } @@ -49,7 +48,7 @@ it 'returns the number of unique events for aggregation', :aggregate_failures do expect(namespace::SOURCES[datasource]) - .to receive(expected_method) + .to receive(:calculate_metrics_union) .with(params.merge(metric_names: %w[event1 event2], property_name: attribute)) .and_return(5) expect(calculate_count_for_aggregation).to eq(5) @@ -66,9 +65,9 @@ allow(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:known_event?).with('event3').and_return(false) end - where(:operator, :datasource, :expected_method, :expected_events) do - 'OR' | 'redis_hll' | :calculate_metrics_union | %w[event1 event2] - 'OR' | 'database' | :calculate_metrics_union | %w[event1 event2 event3] + where(:datasource, :expected_events) do + 'redis_hll' | %w[event1 event2] + 'database' | %w[event1 event2 event3] end with_them do @@ -78,7 +77,6 @@ let(:aggregate) do { source: datasource, - operator: operator, events: %w[event1 event2 event3] } end @@ -91,7 +89,7 @@ it 'returns the number of unique events for aggregation', :aggregate_failures do expect(namespace::SOURCES[datasource]) - .to receive(expected_method) + .to receive(:calculate_metrics_union) .with(params.merge(metric_names: expected_events, property_name: nil)) .and_return(5) expect(calculate_count_for_aggregation).to eq(5) @@ -100,17 +98,15 @@ end context 'with invalid configuration' do - where(:time_frame, :operator, :datasource, :expected_error) do - '28d' | 'SUM' | 'redis_hll' | namespace::UnknownAggregationOperator - '7d' | 'OR' | 'mongodb' | namespace::UnknownAggregationSource - 'all' | 'OR' | 'redis_hll' | namespace::DisallowedAggregationTimeFrame + where(:time_frame, :datasource, :expected_error) do + '7d' | 'mongodb' | namespace::UnknownAggregationSource + 'all' | 'redis_hll' | namespace::DisallowedAggregationTimeFrame end with_them do let(:aggregate) do { source: datasource, - operator: operator, events: %w[event1 event2] } end @@ -146,10 +142,10 @@ .calculate_count_for_aggregation(aggregation: aggregate, time_frame: time_frame) end - where(:time_frame, :operator, :datasource) do - '28d' | 'OR' | 'redis_hll' - '7d' | 'OR' | 'database' - '28d' | 'OR' | 'internal_events' + where(:time_frame, :datasource) do + '28d' | 'redis_hll' + '7d' | 'database' + '28d' | 'internal_events' end with_them do @@ -160,7 +156,6 @@ let(:aggregate) do { source: datasource, - operator: operator, events: %w[event1 event2] } end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/aggregated_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/aggregated_metric_spec.rb index c3eddce4004ca21c2111ca86f56b3b85735210fe..d799fc1cfb5bf30e0f5da07e4b681e6824688822 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/aggregated_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/aggregated_metric_spec.rb @@ -43,13 +43,13 @@ ) end - where(:data_source, :time_frame, :operator, :attribute, :expected_value, :property_name_flag_enabled) do - 'redis_hll' | '28d' | 'OR' | 'user_id' | 3 | true - 'redis_hll' | '28d' | 'OR' | 'user_id' | 4 | false - 'redis_hll' | '28d' | 'OR' | 'project_id' | 4 | false - 'redis_hll' | '7d' | 'OR' | 'user_id' | 2 | true - 'redis_hll' | '7d' | 'OR' | 'project_id' | 1 | true - 'database' | '7d' | 'OR' | 'user_id' | 3.0 | true + where(:data_source, :time_frame, :attribute, :expected_value, :property_name_flag_enabled) do + 'redis_hll' | '28d' | 'user_id' | 3 | true + 'redis_hll' | '28d' | 'user_id' | 4 | false + 'redis_hll' | '28d' | 'project_id' | 4 | false + 'redis_hll' | '7d' | 'user_id' | 2 | true + 'redis_hll' | '7d' | 'project_id' | 1 | true + 'database' | '7d' | 'user_id' | 3.0 | true end with_them do @@ -60,7 +60,6 @@ time_frame: time_frame, options: { aggregate: { - operator: operator, attribute: attribute }, events: %w[ @@ -88,7 +87,6 @@ time_frame: '28d', options: { aggregate: { - operator: 'OR', attribute: 'project.name' }, events: %w[