From c640d72a911e8216d204ba40baa7cd38acca2a80 Mon Sep 17 00:00:00 2001
From: Jonas Larsen <jlarsen@gitlab.com>
Date: Wed, 12 Mar 2025 11:22:53 +0100
Subject: [PATCH] Migrate DiffsCounter to Internal Events
---
app/assets/javascripts/diffs/components/app.vue | 2 --
.../counts_all/20210723075525_diff_searches.yml | 8 +++-----
lib/gitlab/usage_data_counters.rb | 1 -
lib/gitlab/usage_data_counters/diffs_counter.rb | 10 ----------
.../total_counter_redis_key_overrides.yml | 1 +
spec/frontend/diffs/components/app_spec.js | 7 -------
spec/lib/gitlab/usage_data_counters_spec.rb | 5 +++--
spec/lib/gitlab/utils/usage_data_spec.rb | 12 ++++++------
spec/requests/api/usage_data_spec.rb | 4 ++--
9 files changed, 15 insertions(+), 35 deletions(-)
delete mode 100644 lib/gitlab/usage_data_counters/diffs_counter.rb
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 7f649d0b3751..8f89788cb61d 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -8,7 +8,6 @@ import { mapState as mapPiniaState, mapActions as mapPiniaActions } from 'pinia'
import FindingsDrawer from 'ee_component/diffs/components/shared/findings_drawer.vue';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { convertToGraphQLId } from '~/graphql_shared/utils';
-import api from '~/api';
import {
keysFor,
MR_PREVIOUS_FILE_IN_DIFF,
@@ -675,7 +674,6 @@ export default {
this.disableVirtualScroller();
this.trackEvent('i_code_review_user_searches_diff');
- api.trackRedisCounterEvent('diff_searches');
}
},
jumpToFile(step) {
diff --git a/config/metrics/counts_all/20210723075525_diff_searches.yml b/config/metrics/counts_all/20210723075525_diff_searches.yml
index 08865d01f2fc..a64eb7577ce9 100644
--- a/config/metrics/counts_all/20210723075525_diff_searches.yml
+++ b/config/metrics/counts_all/20210723075525_diff_searches.yml
@@ -10,11 +10,9 @@ status: active
milestone: '14.2'
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66522
time_frame: all
-data_source: redis
-instrumentation_class: RedisMetric
-options:
- prefix: diff
- event: searches
+data_source: internal_events
+events:
+ - name: i_code_review_user_searches_diff
tiers:
- free
- premium
diff --git a/lib/gitlab/usage_data_counters.rb b/lib/gitlab/usage_data_counters.rb
index 3964c4830af5..a208fd943b31 100644
--- a/lib/gitlab/usage_data_counters.rb
+++ b/lib/gitlab/usage_data_counters.rb
@@ -3,7 +3,6 @@
module Gitlab
module UsageDataCounters
COUNTERS = [
- DiffsCounter,
MergeRequestWidgetExtensionCounter
].freeze
diff --git a/lib/gitlab/usage_data_counters/diffs_counter.rb b/lib/gitlab/usage_data_counters/diffs_counter.rb
deleted file mode 100644
index b06c7e26f540..000000000000
--- a/lib/gitlab/usage_data_counters/diffs_counter.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
- module UsageDataCounters
- class DiffsCounter < BaseCounter
- KNOWN_EVENTS = %w[searches].freeze
- PREFIX = 'diff'
- end
- end
-end
diff --git a/lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml b/lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml
index 3914d1577098..6a1a32e8b8df 100644
--- a/lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml
+++ b/lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml
@@ -144,3 +144,4 @@
'{event_counters}_request_api_proxy_access_via_user': USAGE_KUBERNETES_AGENT_K8S_API_PROXY_REQUESTS_VIA_USER_ACCESS
'{event_counters}_request_api_proxy_access_via_pat': USAGE_KUBERNETES_AGENT_K8S_API_PROXY_REQUESTS_VIA_PAT_ACCESS
'{event_counters}_delete_manifest_from_container_registry': USAGE_CONTAINER_REGISTRY_EVENTS_I_CONTAINER_REGISTRY_DELETE_MANIFEST
+'{event_counters}_i_code_review_user_searches_diff': USAGE_DIFF_SEARCHES
diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js
index 0233a3a90494..9b76ce55251e 100644
--- a/spec/frontend/diffs/components/app_spec.js
+++ b/spec/frontend/diffs/components/app_spec.js
@@ -6,7 +6,6 @@ import VueApollo from 'vue-apollo';
// eslint-disable-next-line no-restricted-imports
import Vuex from 'vuex';
import { createTestingPinia } from '@pinia/testing';
-import api from '~/api';
import getMRCodequalityAndSecurityReports from 'ee_else_ce/diffs/components/graphql/get_mr_codequality_and_security_reports.query.graphql';
import createMockApollo from 'helpers/mock_apollo_helper';
import setWindowLocation from 'helpers/set_window_location_helper';
@@ -51,8 +50,6 @@ const UPDATED_COMMIT_URL = `${TEST_HOST}/COMMIT/NEW`;
const ENDPOINT_BATCH_URL = `${TEST_HOST}/diff/endpointBatch`;
const ENDPOINT_METADATA_URL = `${TEST_HOST}/diff/endpointMetadata`;
-jest.mock('~/api.js');
-
Vue.use(Vuex);
Vue.use(VueApollo);
@@ -1184,7 +1181,6 @@ describe('diffs/components/app', () => {
window.dispatchEvent(new Event('blur'));
expect(trackEventSpy).not.toHaveBeenCalled();
- expect(api.trackRedisCounterEvent).not.toHaveBeenCalled();
});
it('should track metrics if delta is between 0 and 1000ms', async () => {
@@ -1199,7 +1195,6 @@ describe('diffs/components/app', () => {
window.dispatchEvent(new Event('blur'));
expect(trackEventSpy).toHaveBeenCalledWith('i_code_review_user_searches_diff', {}, undefined);
- expect(api.trackRedisCounterEvent).toHaveBeenCalledWith('diff_searches');
});
it('should not track metrics if delta is greater than or equal to 1000ms', async () => {
@@ -1214,7 +1209,6 @@ describe('diffs/components/app', () => {
window.dispatchEvent(new Event('blur'));
expect(trackEventSpy).not.toHaveBeenCalled();
- expect(api.trackRedisCounterEvent).not.toHaveBeenCalled();
});
it('should not track metrics if delta is negative', async () => {
@@ -1229,7 +1223,6 @@ describe('diffs/components/app', () => {
window.dispatchEvent(new Event('blur'));
expect(trackEventSpy).not.toHaveBeenCalled();
- expect(api.trackRedisCounterEvent).not.toHaveBeenCalled();
});
});
});
diff --git a/spec/lib/gitlab/usage_data_counters_spec.rb b/spec/lib/gitlab/usage_data_counters_spec.rb
index 8d7e497dc1f7..958ac77bb2de 100644
--- a/spec/lib/gitlab/usage_data_counters_spec.rb
+++ b/spec/lib/gitlab/usage_data_counters_spec.rb
@@ -13,10 +13,11 @@
describe '.count' do
subject { described_class.count(event_name) }
- let(:event_name) { 'diff_searches' }
+ let(:event_name) { 'i_code_review_merge_request_widget_code_quality_count_view' }
it 'increases a searches counter' do
- expect(Gitlab::UsageDataCounters::DiffsCounter).to receive(:count).with('searches')
+ expect(Gitlab::UsageDataCounters::MergeRequestWidgetExtensionCounter)
+ .to receive(:count).with('code_quality_count_view')
subject
end
diff --git a/spec/lib/gitlab/utils/usage_data_spec.rb b/spec/lib/gitlab/utils/usage_data_spec.rb
index b235b4612159..e03220f62e29 100644
--- a/spec/lib/gitlab/utils/usage_data_spec.rb
+++ b/spec/lib/gitlab/utils/usage_data_spec.rb
@@ -502,18 +502,18 @@ def expect_error(exception, message, &block)
context 'with counter given' do
context 'when gets an error' do
- subject { described_class.redis_usage_data(::Gitlab::UsageDataCounters::DiffsCounter) }
+ subject { described_class.redis_usage_data(::Gitlab::UsageDataCounters::MergeRequestWidgetExtensionCounter) }
- let(:fallback) { ::Gitlab::UsageDataCounters::DiffsCounter.fallback_totals }
- let(:failing_class) { ::Gitlab::UsageDataCounters::DiffsCounter }
+ let(:fallback) { ::Gitlab::UsageDataCounters::MergeRequestWidgetExtensionCounter.fallback_totals }
+ let(:failing_class) { ::Gitlab::UsageDataCounters::MergeRequestWidgetExtensionCounter }
let(:failing_method) { :totals }
it_behaves_like 'failing hardening method', ::Redis::CommandError
end
- it 'returns the totals when couter is given' do
- allow(::Gitlab::UsageDataCounters::DiffsCounter).to receive(:totals).and_return({ merge_request_create: 2 })
- expect(described_class.redis_usage_data(::Gitlab::UsageDataCounters::DiffsCounter)).to eql({ merge_request_create: 2 })
+ it 'returns the totals when counter is given' do
+ allow(::Gitlab::UsageDataCounters::MergeRequestWidgetExtensionCounter).to receive(:totals).and_return({ merge_request_create: 2 })
+ expect(described_class.redis_usage_data(::Gitlab::UsageDataCounters::MergeRequestWidgetExtensionCounter)).to eql({ merge_request_create: 2 })
end
end
end
diff --git a/spec/requests/api/usage_data_spec.rb b/spec/requests/api/usage_data_spec.rb
index 8a25f5cb79e2..ef84e1c1a6a2 100644
--- a/spec/requests/api/usage_data_spec.rb
+++ b/spec/requests/api/usage_data_spec.rb
@@ -85,7 +85,7 @@
describe 'POST /usage_data/increment_counter' do
let(:endpoint) { '/usage_data/increment_counter' }
- let(:known_event) { "diff_searches" }
+ let(:known_event) { "i_code_review_merge_request_widget_code_quality_count_view" }
let(:unknown_event) { 'unknown' }
context 'without authentication' do
@@ -113,7 +113,7 @@
context 'with correct params' do
it 'returns status :ok' do
- expect(Gitlab::UsageDataCounters::BaseCounter).to receive(:count).with("searches")
+ expect(Gitlab::UsageDataCounters::BaseCounter).to receive(:count).with("code_quality_count_view")
post api(endpoint, user), params: { event: known_event }
--
GitLab