From 7fd2d70f1c98571e0243c130216e2877df3ca998 Mon Sep 17 00:00:00 2001
From: Jonas Larsen <jlarsen@gitlab.com>
Date: Wed, 20 Dec 2023 14:15:06 +0100
Subject: [PATCH] Change i_code_review_create_mr metric to total counts
Now that we support total counts with 7d and 28d time frames, we can
stop using a distinct counter to implement the
redis_hll_counters.code_review.i_code_review_create_mr_*
metrics and instead count the number of i_code_review_user_create_mr
events triggered.
Changelog: changed
---
...182900_i_code_review_create_mr_monthly.yml | 8 ++--
...3183300_i_code_review_create_mr_weekly.yml | 7 ++--
..._create_mr_keys_from_redis_hll_to_redis.rb | 29 +++++++++++++
db/schema_migrations/20231204095802 | 1 +
.../merge_request_activity_unique_counter.rb | 8 +---
...ge_request_activity_unique_counter_spec.rb | 8 ----
...te_mr_keys_from_redis_hll_to_redis_spec.rb | 41 +++++++++++++++++++
7 files changed, 81 insertions(+), 21 deletions(-)
create mode 100644 db/post_migrate/20231204095802_change_i_code_review_create_mr_keys_from_redis_hll_to_redis.rb
create mode 100644 db/schema_migrations/20231204095802
create mode 100644 spec/migrations/20231204095802_change_i_code_review_create_mr_keys_from_redis_hll_to_redis_spec.rb
diff --git a/config/metrics/counts_28d/20221213182900_i_code_review_create_mr_monthly.yml b/config/metrics/counts_28d/20221213182900_i_code_review_create_mr_monthly.yml
index 740183d7286a..860d63f0a6bd 100644
--- a/config/metrics/counts_28d/20221213182900_i_code_review_create_mr_monthly.yml
+++ b/config/metrics/counts_28d/20221213182900_i_code_review_create_mr_monthly.yml
@@ -1,4 +1,5 @@
---
+# This metric used to be implemented as a RedisHLL counter. Therefor, the redis_hll_counters prefix.
key_path: redis_hll_counters.code_review.i_code_review_create_mr_monthly
description: Count of unique merge requests created per month
product_section: dev
@@ -9,14 +10,15 @@ status: active
milestone: "15.7"
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106869
time_frame: 28d
-data_source: redis_hll
+data_source: internal_events
data_category: operational
-instrumentation_class: RedisHLLMetric
performance_indicator_type:
- customer_health_score
options:
events:
- - i_code_review_create_mr
+ - i_code_review_user_create_mr
+events:
+ - name: i_code_review_user_create_mr
distribution:
- ce
- ee
diff --git a/config/metrics/counts_7d/20221213183300_i_code_review_create_mr_weekly.yml b/config/metrics/counts_7d/20221213183300_i_code_review_create_mr_weekly.yml
index 99dd27888259..b0f4e9b0dca2 100644
--- a/config/metrics/counts_7d/20221213183300_i_code_review_create_mr_weekly.yml
+++ b/config/metrics/counts_7d/20221213183300_i_code_review_create_mr_weekly.yml
@@ -9,13 +9,14 @@ status: active
milestone: "15.7"
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106869
time_frame: 7d
-data_source: redis_hll
+data_source: internal_events
data_category: operational
-instrumentation_class: RedisHLLMetric
performance_indicator_type: []
options:
events:
- - i_code_review_create_mr
+ - i_code_review_user_create_mr
+events:
+ - name: i_code_review_user_create_mr
distribution:
- ce
- ee
diff --git a/db/post_migrate/20231204095802_change_i_code_review_create_mr_keys_from_redis_hll_to_redis.rb b/db/post_migrate/20231204095802_change_i_code_review_create_mr_keys_from_redis_hll_to_redis.rb
new file mode 100644
index 000000000000..66b5ec0698ec
--- /dev/null
+++ b/db/post_migrate/20231204095802_change_i_code_review_create_mr_keys_from_redis_hll_to_redis.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+class ChangeICodeReviewCreateMrKeysFromRedisHllToRedis < Gitlab::Database::Migration[2.2]
+ milestone '16.8'
+
+ disable_ddl_transaction!
+ restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+ REDIS_HLL_PREFIX = '{hll_counters}_i_code_review_create_mr'
+ REDIS_PREFIX = '{event_counters}_i_code_review_user_create_mr'
+
+ def up
+ # For each old (redis_hll) counter we find the corresponding target (redis) counter and add
+ # old value to migrate a metric. If the Redis counter does not exist, it will get created.
+ # Since the RedisHLL keys expire after 6 weeks, we will migrate 6 keys at the most.
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.scan_each(match: "#{REDIS_HLL_PREFIX}-*") do |key|
+ redis_key = key.sub(REDIS_HLL_PREFIX, REDIS_PREFIX)
+ redis_hll_value = redis.pfcount(key)
+
+ redis.incrby(redis_key, redis_hll_value)
+ end
+ end
+ end
+
+ def down
+ # no-op
+ end
+end
diff --git a/db/schema_migrations/20231204095802 b/db/schema_migrations/20231204095802
new file mode 100644
index 000000000000..eee6ff01a860
--- /dev/null
+++ b/db/schema_migrations/20231204095802
@@ -0,0 +1 @@
+c2fc8e11eb2ac22bd4f37dcabd9468ddfce6285a9b796560c8ce9a21fa0047e1
\ No newline at end of file
diff --git a/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb
index d26b7ce951de..db48095ab744 100644
--- a/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb
+++ b/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb
@@ -6,7 +6,6 @@ module MergeRequestActivityUniqueCounter
MR_DIFFS_ACTION = 'i_code_review_mr_diffs'
MR_DIFFS_SINGLE_FILE_ACTION = 'i_code_review_mr_single_file_diffs'
MR_DIFFS_USER_SINGLE_FILE_ACTION = 'i_code_review_user_single_file_diffs'
- MR_CREATE_ACTION = 'i_code_review_create_mr'
MR_USER_CREATE_ACTION = 'i_code_review_user_create_mr'
MR_CLOSE_ACTION = 'i_code_review_user_close_mr'
MR_REOPEN_ACTION = 'i_code_review_user_reopen_mr'
@@ -64,15 +63,10 @@ def track_mr_diffs_single_file_action(merge_request:, user:)
end
def track_create_mr_action(user:, merge_request:)
- track_unique_action_by_merge_request(MR_CREATE_ACTION, merge_request)
-
- project = merge_request.target_project
-
Gitlab::InternalEvents.track_event(
MR_USER_CREATE_ACTION,
user: user,
- project: project,
- namespace: project.namespace
+ project: merge_request.target_project
)
end
diff --git a/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb
index c3a718e669ac..5c03ccb0d71c 100644
--- a/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb
+++ b/spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb
@@ -55,14 +55,6 @@
let(:merge_request) { create(:merge_request) }
let(:target_project) { merge_request.target_project }
- it_behaves_like 'a tracked merge request unique event' do
- let(:action) { described_class::MR_USER_CREATE_ACTION }
- end
-
- it_behaves_like 'a tracked merge request unique event' do
- let(:action) { described_class::MR_CREATE_ACTION }
- end
-
it_behaves_like 'internal event tracking' do
let(:event) { described_class::MR_USER_CREATE_ACTION }
let(:project) { target_project }
diff --git a/spec/migrations/20231204095802_change_i_code_review_create_mr_keys_from_redis_hll_to_redis_spec.rb b/spec/migrations/20231204095802_change_i_code_review_create_mr_keys_from_redis_hll_to_redis_spec.rb
new file mode 100644
index 000000000000..38d3af5a5253
--- /dev/null
+++ b/spec/migrations/20231204095802_change_i_code_review_create_mr_keys_from_redis_hll_to_redis_spec.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe ChangeICodeReviewCreateMrKeysFromRedisHllToRedis, :migration, :clean_gitlab_redis_cache, feature_category: :service_ping do
+ def set_redis_hll(key, value)
+ Gitlab::Redis::HLL.add(key: key, value: value, expiry: 6.weeks)
+ end
+
+ def get_int_from_redis(key)
+ Gitlab::Redis::SharedState.with { |redis| redis.get(key)&.to_i }
+ end
+
+ describe "#up" do
+ before do
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-16', value: 1)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-16', value: 2)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-47', value: 3)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-48', value: 1)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-49', value: 2)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-49', value: 4)
+ set_redis_hll('{hll_counters}_some_other_event-2023-49', value: 7)
+ end
+
+ it 'migrates all RedisHLL keys for i_code_review_create_mr', :aggregate_failures do
+ migrate!
+
+ expect(get_int_from_redis('{event_counters}_i_code_review_user_create_mr-2023-16')).to eq(2)
+ expect(get_int_from_redis('{event_counters}_i_code_review_user_create_mr-2023-47')).to eq(1)
+ expect(get_int_from_redis('{event_counters}_i_code_review_user_create_mr-2023-48')).to eq(1)
+ expect(get_int_from_redis('{event_counters}_i_code_review_user_create_mr-2023-49')).to eq(2)
+ end
+
+ it 'does not not migrate other RedisHLL keys' do
+ migrate!
+
+ expect(get_int_from_redis('{event_counters}_some_other_event-2023-16')).to be_nil
+ end
+ end
+end
--
GitLab