From ce37fb0c2603ed203bb9314615f05f157aa2f5ac Mon Sep 17 00:00:00 2001
From: Jonas Larsen <jlarsen@gitlab.com>
Date: Mon, 19 Feb 2024 13:55:56 +0000
Subject: [PATCH] Remove "AND" operator - operator usage
---
lib/gitlab/usage/metrics/aggregates.rb | 3 --
.../usage/metrics/aggregates/aggregate.rb | 7 ---
.../instrumentations/aggregated_metric.rb | 2 -
.../metrics/aggregates/aggregate_spec.rb | 43 ++++++++-----------
.../aggregated_metric_spec.rb | 16 +++----
5 files changed, 26 insertions(+), 45 deletions(-)
diff --git a/lib/gitlab/usage/metrics/aggregates.rb b/lib/gitlab/usage/metrics/aggregates.rb
index a3410cbc39351..698d6f3f6b516 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 079fb39851cc3..1b78cebf6a801 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 15eb12b05df05..1990aa46e6eca 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 edace79ad7a82..b1d192608068f 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 c3eddce4004ca..d799fc1cfb5bf 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[
--
GitLab