Commit 07fa965c authored by Marc Shaw's avatar Marc Shaw

Code Review: Wrap redis_usage_data and return {} when flag is false

Issue: gitlab.com/gitlab-org/gitlab/-/issues/219956
Merge Request: gitlab.com/gitlab-org/gitlab/-/merge_requests/35580
parent 121486a8
...@@ -577,24 +577,33 @@ module Gitlab ...@@ -577,24 +577,33 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def action_monthly_active_users(time_period) def action_monthly_active_users(time_period)
return {} unless Feature.enabled?(Gitlab::UsageDataCounters::TrackUniqueActions::FEATURE_FLAG)
counter = Gitlab::UsageDataCounters::TrackUniqueActions.new counter = Gitlab::UsageDataCounters::TrackUniqueActions.new
project_count = counter.count_unique_events( project_count = redis_usage_data do
event_action: Gitlab::UsageDataCounters::TrackUniqueActions::PUSH_ACTION, counter.count_unique_events(
date_from: time_period[:created_at].first, event_action: Gitlab::UsageDataCounters::TrackUniqueActions::PUSH_ACTION,
date_to: time_period[:created_at].last date_from: time_period[:created_at].first,
) date_to: time_period[:created_at].last
)
end
design_count = counter.count_unique_events( design_count = redis_usage_data do
event_action: Gitlab::UsageDataCounters::TrackUniqueActions::DESIGN_ACTION, counter.count_unique_events(
date_from: time_period[:created_at].first, event_action: Gitlab::UsageDataCounters::TrackUniqueActions::DESIGN_ACTION,
date_to: time_period[:created_at].last date_from: time_period[:created_at].first,
) date_to: time_period[:created_at].last
)
end
wiki_count = counter.count_unique_events( wiki_count = redis_usage_data do
event_action: Gitlab::UsageDataCounters::TrackUniqueActions::WIKI_ACTION, counter.count_unique_events(
date_from: time_period[:created_at].first, event_action: Gitlab::UsageDataCounters::TrackUniqueActions::WIKI_ACTION,
date_to: time_period[:created_at].last) date_from: time_period[:created_at].first,
date_to: time_period[:created_at].last
)
end
{ {
action_monthly_active_users_project_repo: project_count, action_monthly_active_users_project_repo: project_count,
......
...@@ -4,6 +4,7 @@ module Gitlab ...@@ -4,6 +4,7 @@ module Gitlab
module UsageDataCounters module UsageDataCounters
class TrackUniqueActions class TrackUniqueActions
KEY_EXPIRY_LENGTH = 29.days KEY_EXPIRY_LENGTH = 29.days
FEATURE_FLAG = :track_unique_actions
WIKI_ACTION = :wiki_action WIKI_ACTION = :wiki_action
DESIGN_ACTION = :design_action DESIGN_ACTION = :design_action
...@@ -35,7 +36,7 @@ module Gitlab ...@@ -35,7 +36,7 @@ module Gitlab
def track_action(event_action:, event_target:, author_id:, time: Time.zone.now) def track_action(event_action:, event_target:, author_id:, time: Time.zone.now)
return unless Gitlab::CurrentSettings.usage_ping_enabled return unless Gitlab::CurrentSettings.usage_ping_enabled
return unless Feature.enabled?(:track_push_events_with_redis) return unless Feature.enabled?(FEATURE_FLAG)
return unless valid_target?(event_target) return unless valid_target?(event_target)
return unless valid_action?(event_action) return unless valid_action?(event_action)
......
...@@ -20,7 +20,7 @@ RSpec.describe Gitlab::UsageDataCounters::TrackUniqueActions, :clean_gitlab_redi ...@@ -20,7 +20,7 @@ RSpec.describe Gitlab::UsageDataCounters::TrackUniqueActions, :clean_gitlab_redi
context 'when the feature flag and the application setting is enabled' do context 'when the feature flag and the application setting is enabled' do
context 'when the target and the action is valid' do context 'when the target and the action is valid' do
before do before do
stub_feature_flags(cache_diff_stats_merge_request: true) stub_feature_flags(described_class::FEATURE_FLAG => true)
stub_application_setting(usage_ping_enabled: true) stub_application_setting(usage_ping_enabled: true)
end end
...@@ -60,16 +60,16 @@ RSpec.describe Gitlab::UsageDataCounters::TrackUniqueActions, :clean_gitlab_redi ...@@ -60,16 +60,16 @@ RSpec.describe Gitlab::UsageDataCounters::TrackUniqueActions, :clean_gitlab_redi
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:feature_flag, :application_setting, :target, :action) do where(:feature_flag, :application_setting, :target, :action) do
true | true | :project | :invalid_action true | true | Project | :invalid_action
false | true | :project | :pushed false | true | Project | :pushed
true | false | :project | :pushed true | false | Project | :pushed
true | true | :invalid_target | :pushed true | true | :invalid_target | :pushed
end end
with_them do with_them do
before do before do
stub_application_setting(usage_ping_enabled: application_setting) stub_application_setting(usage_ping_enabled: application_setting)
stub_feature_flags(cache_diff_stats_merge_request: feature_flag) stub_feature_flags(described_class::FEATURE_FLAG => feature_flag)
end end
it 'returns the expected values' do it 'returns the expected values' do
......
...@@ -898,29 +898,45 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do ...@@ -898,29 +898,45 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
let(:time) { Time.zone.now } let(:time) { Time.zone.now }
before do before do
counter = Gitlab::UsageDataCounters::TrackUniqueActions.new stub_feature_flags(Gitlab::UsageDataCounters::TrackUniqueActions::FEATURE_FLAG => feature_flag)
project = Event::TARGET_TYPES[:project] end
wiki = Event::TARGET_TYPES[:wiki]
design = Event::TARGET_TYPES[:design] context 'when the feature flag is enabled' do
let(:feature_flag) { true }
counter.track_action(event_action: :pushed, event_target: project, author_id: 1)
counter.track_action(event_action: :pushed, event_target: project, author_id: 1) before do
counter.track_action(event_action: :pushed, event_target: project, author_id: 2) counter = Gitlab::UsageDataCounters::TrackUniqueActions.new
counter.track_action(event_action: :pushed, event_target: project, author_id: 3) project = Event::TARGET_TYPES[:project]
counter.track_action(event_action: :pushed, event_target: project, author_id: 4, time: time - 3.days) wiki = Event::TARGET_TYPES[:wiki]
counter.track_action(event_action: :created, event_target: project, author_id: 5, time: time - 3.days) design = Event::TARGET_TYPES[:design]
counter.track_action(event_action: :created, event_target: wiki, author_id: 3)
counter.track_action(event_action: :created, event_target: design, author_id: 3) counter.track_action(event_action: :pushed, event_target: project, author_id: 1)
end counter.track_action(event_action: :pushed, event_target: project, author_id: 1)
counter.track_action(event_action: :pushed, event_target: project, author_id: 2)
it 'returns the distinct count of user actions within the specified time period' do counter.track_action(event_action: :pushed, event_target: project, author_id: 3)
expect(described_class.action_monthly_active_users(time_period)).to eq( counter.track_action(event_action: :pushed, event_target: project, author_id: 4, time: time - 3.days)
{ counter.track_action(event_action: :created, event_target: project, author_id: 5, time: time - 3.days)
action_monthly_active_users_design_management: 1, counter.track_action(event_action: :created, event_target: wiki, author_id: 3)
action_monthly_active_users_project_repo: 3, counter.track_action(event_action: :created, event_target: design, author_id: 3)
action_monthly_active_users_wiki_repo: 1 end
}
) it 'returns the distinct count of user actions within the specified time period' do
expect(described_class.action_monthly_active_users(time_period)).to eq(
{
action_monthly_active_users_design_management: 1,
action_monthly_active_users_project_repo: 3,
action_monthly_active_users_wiki_repo: 1
}
)
end
end
context 'when the feature flag is disabled' do
let(:feature_flag) { false }
it 'returns an empty hash' do
expect(described_class.action_monthly_active_users(time_period)).to eq({})
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment