diff --git a/app/controllers/concerns/record_user_last_activity.rb b/app/controllers/concerns/record_user_last_activity.rb
index 6ac87d8f27b0eaf85eea7f2f600c8027376616d0..501590d33d992f9d29260df3a9b399dd5ef4110b 100644
--- a/app/controllers/concerns/record_user_last_activity.rb
+++ b/app/controllers/concerns/record_user_last_activity.rb
@@ -20,6 +20,7 @@ def set_user_last_activity
return if Gitlab::Database.read_only?
return unless current_user && current_user.last_activity_on != Date.today
- Users::ActivityService.new(current_user).execute
+ # TODO: add namespace & project - https://gitlab.com/gitlab-org/gitlab/-/issues/387952
+ Users::ActivityService.new(author: current_user).execute
end
end
diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb
index 942cb9beed445f9e78c30a27fe7cb085f2e84e25..2f01bdecd2395de9019d03580618ded90b4b4348 100644
--- a/app/controllers/graphql_controller.rb
+++ b/app/controllers/graphql_controller.rb
@@ -144,7 +144,8 @@ def disable_query_limiting
def set_user_last_activity
return unless current_user
- Users::ActivityService.new(current_user).execute
+ # TODO: add namespace & project - https://gitlab.com/gitlab-org/gitlab/-/issues/387951
+ Users::ActivityService.new(author: current_user).execute
end
def track_vs_code_usage
diff --git a/app/controllers/repositories/git_http_controller.rb b/app/controllers/repositories/git_http_controller.rb
index 144ec4c0de9ba5dfde173b14fb1645852438e217..bd3461d83318396a7de0faa867f436fd77e9fcac 100644
--- a/app/controllers/repositories/git_http_controller.rb
+++ b/app/controllers/repositories/git_http_controller.rb
@@ -116,7 +116,7 @@ def access_klass
end
def log_user_activity
- Users::ActivityService.new(user).execute
+ Users::ActivityService.new(author: user, project: project, namespace: project&.namespace).execute
end
end
end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 699dcf1adac43fff7e017bada28060a34f80f5e6..b6aba04c877eaac052864fb04e5bbffbeec172ea 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -289,7 +289,7 @@ def log_audit_event(user, resource, options = {})
def log_user_activity(user)
login_counter.increment
- Users::ActivityService.new(user).execute
+ Users::ActivityService.new(author: user).execute
end
def load_recaptcha
diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb
index bf4a26400e1b950c5587c50c57d799bfc5aa5857..acc5176059d35534f222c20f4f89064e1641bf4d 100644
--- a/app/services/event_create_service.rb
+++ b/app/services/event_create_service.rb
@@ -245,7 +245,7 @@ def create_push_event(service_class, project, current_user, push_data)
Users::LastPushEventService.new(current_user)
.cache_last_push_event(event)
- Users::ActivityService.new(current_user).execute
+ Users::ActivityService.new(author: current_user, namespace: namespace, project: project).execute
end
def create_event(resource_parent, current_user, status, attributes = {})
diff --git a/app/services/users/activity_service.rb b/app/services/users/activity_service.rb
index 4978f778870b6c7f1876b0226dfeb61d3dd0daba..c8f9c28061f06e04d96f7f9414c4d5aa4e8aa376 100644
--- a/app/services/users/activity_service.rb
+++ b/app/services/users/activity_service.rb
@@ -4,38 +4,56 @@ module Users
class ActivityService
LEASE_TIMEOUT = 1.minute.to_i
- def initialize(author)
+ def initialize(author:, namespace: nil, project: nil)
@user = if author.respond_to?(:username)
author
elsif author.respond_to?(:user)
author.user
end
- @user = nil unless @user.is_a?(User)
+ @user = nil unless user.is_a?(User)
+ @namespace = namespace
+ @project = project
end
def execute
- return unless @user
+ return unless user
::Gitlab::Database::LoadBalancing::Session.without_sticky_writes { record_activity }
end
private
+ attr_reader :user, :namespace, :project
+
def record_activity
return if Gitlab::Database.read_only?
today = Date.today
- return if @user.last_activity_on == today
+ return if user.last_activity_on == today
- lease = Gitlab::ExclusiveLease.new("activity_service:#{@user.id}",
+ lease = Gitlab::ExclusiveLease.new("activity_service:#{user.id}",
timeout: LEASE_TIMEOUT)
return unless lease.try_obtain
- @user.update_attribute(:last_activity_on, today)
+ user.update_attribute(:last_activity_on, today)
+
+ Gitlab::UsageDataCounters::HLLRedisCounter.track_event('unique_active_user', values: user.id)
+
+ return unless Feature.enabled?(:route_hll_to_snowplow_phase3)
- Gitlab::UsageDataCounters::HLLRedisCounter.track_event('unique_active_user', values: @user.id)
+ Gitlab::Tracking.event(
+ 'Users::ActivityService',
+ 'perform_action',
+ user: user,
+ namespace: namespace,
+ project: project,
+ label: 'redis_hll_counters.manage.unique_active_users_monthly',
+ context: [
+ Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: 'unique_active_user').to_context
+ ]
+ )
end
end
end
diff --git a/config/events/1671713111_Users__ActivityService_perform_action.yml b/config/events/1671713111_Users__ActivityService_perform_action.yml
new file mode 100644
index 0000000000000000000000000000000000000000..4b1f90d42b48486089398e689a27f84299e13087
--- /dev/null
+++ b/config/events/1671713111_Users__ActivityService_perform_action.yml
@@ -0,0 +1,23 @@
+---
+description: User's last_activity_on gets updated
+category: Users::ActivityService
+action: perform_action
+label_description: key_path of Service Ping metric redis_hll_counters.manage.unique_active_users_monthly
+property_description:
+value_description:
+extra_properties:
+identifiers:
+product_section: dev
+product_stage: manage
+product_group: manage
+product_category:
+milestone: "15.8"
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108108
+distributions:
+- ce
+- ee
+tiers:
+- free
+- premium
+- ultimate
+
diff --git a/ee/spec/controllers/repositories/git_http_controller_spec.rb b/ee/spec/controllers/repositories/git_http_controller_spec.rb
index 1044e36dd239606bd33558de5b866ed3ca930e69..250cddee5e7195660651d68ed012f98621bf14d5 100644
--- a/ee/spec/controllers/repositories/git_http_controller_spec.rb
+++ b/ee/spec/controllers/repositories/git_http_controller_spec.rb
@@ -8,6 +8,7 @@
let_it_be(:group) { create(:group, :wiki_repo) }
let_it_be(:user) { create(:user) }
+ let_it_be(:project) { nil }
before_all do
group.add_owner(user)
diff --git a/lib/api/api.rb b/lib/api/api.rb
index 5e44902267693912d74be411e3787f461f340037..81b96207863e39dae754a3b7d6cda7003aa6911e 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -164,7 +164,7 @@ class API < ::API::Base
namespace do
after do
- ::Users::ActivityService.new(@current_user).execute
+ ::Users::ActivityService.new(author: @current_user, project: @project, namespace: @group).execute
end
# Mount endpoints to include in the OpenAPI V2 documentation here
diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb
index 56db6ee4c5cb90f845c69df1b0a7e4bc4f0c6bf6..816b8deb4618a11b6da7ea02825125efe141e1f3 100644
--- a/lib/api/helpers/internal_helpers.rb
+++ b/lib/api/helpers/internal_helpers.rb
@@ -58,7 +58,9 @@ def parse_env
def log_user_activity(actor)
commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS
- ::Users::ActivityService.new(actor).execute if commands.include?(params[:action])
+ return unless commands.include?(params[:action])
+
+ ::Users::ActivityService.new(author: actor, namespace: project&.namespace, project: project).execute
end
def redis_ping
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 78b3cc63b085e3c25b09dbed9712490e29dd2e52..1f7d169bae565dad00f77ecead201dafd1d9c853 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -221,7 +221,7 @@ def succesful_login(user_params, sesion_params: {})
expect(Gitlab::Metrics).to receive(:counter)
.with(:successful_login_captcha_total, anything)
.and_return(counter)
- expect(Gitlab::Metrics).to receive(:counter).and_call_original
+ expect(Gitlab::Metrics).to receive(:counter).at_least(1).time.and_call_original
post(:create, params: { user: user_params }, session: sesion_params)
end
diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb
index 1ad0f20b77e1ae8d403450c50cfd66ca129782a0..35851fff6c883a74994c50073832b1e36f0b1c76 100644
--- a/spec/requests/api/api_spec.rb
+++ b/spec/requests/api/api_spec.rb
@@ -12,8 +12,22 @@
let(:user) { create(:user, last_activity_on: Date.yesterday) }
it 'updates the users last_activity_on to the current date' do
+ expect(Users::ActivityService).to receive(:new).with(author: user, project: nil, namespace: nil).and_call_original
+
expect { get api('/groups', user) }.to change { user.reload.last_activity_on }.to(Date.today)
end
+
+ context "with a project-specific path" do
+ let_it_be(:project) { create(:project, :public) }
+ let_it_be(:user) { project.first_owner }
+
+ it "passes correct arguments to ActivityService" do
+ activity_args = { author: user, project: project, namespace: project.group }
+ expect(Users::ActivityService).to receive(:new).with(activity_args).and_call_original
+
+ get(api("/projects/#{project.id}/issues", user))
+ end
+ end
end
describe 'User with only read_api scope personal access token' do
diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb
index 767f3e8b5b5382cfc415780a12a22788ed660af1..ca32271f5738c34788ad7ff19168144261db094c 100644
--- a/spec/requests/api/internal/base_spec.rb
+++ b/spec/requests/api/internal/base_spec.rb
@@ -651,6 +651,12 @@ def request
headers: gitlab_shell_internal_api_request_header
)
end
+
+ it "updates user's activity data" do
+ expect(::Users::ActivityService).to receive(:new).with(author: user, namespace: project.namespace, project: project)
+
+ request
+ end
end
end
end
diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb
index 47a4b943d8313e081ce4b2acd8c634a8d49c172b..6c0d93f568a7dd18007bce1ccd1a83c67f0e9ee1 100644
--- a/spec/services/users/activity_service_spec.rb
+++ b/spec/services/users/activity_service_spec.rb
@@ -7,9 +7,21 @@
let(:user) { create(:user, last_activity_on: last_activity_on) }
- subject { described_class.new(user) }
+ subject { described_class.new(author: user) }
describe '#execute', :clean_gitlab_redis_shared_state do
+ shared_examples 'does not update last_activity_on' do
+ it 'does not update user attribute' do
+ expect { subject.execute }.not_to change(user, :last_activity_on)
+ end
+
+ it 'does not track Snowplow event' do
+ subject.execute
+
+ expect_no_snowplow_event
+ end
+ end
+
context 'when last activity is nil' do
let(:last_activity_on) { nil }
@@ -41,13 +53,29 @@
subject.execute
end
+
+ it_behaves_like 'Snowplow event tracking with RedisHLL context' do
+ subject(:record_activity) { described_class.new(author: user, namespace: namespace, project: project).execute }
+
+ let(:feature_flag_name) { :route_hll_to_snowplow_phase3 }
+ let(:category) { described_class.name }
+ let(:action) { 'perform_action' }
+ let(:label) { 'redis_hll_counters.manage.unique_active_users_monthly' }
+ let(:namespace) { build(:group) }
+ let(:project) { build(:project) }
+ let(:context) do
+ payload = Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll,
+ event: 'unique_active_user').to_context
+ [Gitlab::Json.dump(payload)]
+ end
+ end
end
context 'when a bad object is passed' do
let(:fake_object) { double(username: 'hello') }
it 'does not record activity' do
- service = described_class.new(fake_object)
+ service = described_class.new(author: fake_object)
expect(service).not_to receive(:record_activity)
@@ -58,9 +86,7 @@
context 'when last activity is today' do
let(:last_activity_on) { Date.today }
- it 'does not update last_activity_on' do
- expect { subject.execute }.not_to change(user, :last_activity_on)
- end
+ it_behaves_like 'does not update last_activity_on'
it 'does not try to obtain ExclusiveLease' do
expect(Gitlab::ExclusiveLease).not_to receive(:new).with("activity_service:#{user.id}", anything)
@@ -76,19 +102,17 @@
allow(Gitlab::Database).to receive(:read_only?).and_return(true)
end
- it 'does not update last_activity_on' do
- expect { subject.execute }.not_to change(user, :last_activity_on)
- end
+ it_behaves_like 'does not update last_activity_on'
end
context 'when a lease could not be obtained' do
let(:last_activity_on) { nil }
- it 'does not update last_activity_on' do
+ before do
stub_exclusive_lease_taken("activity_service:#{user.id}", timeout: 1.minute.to_i)
-
- expect { subject.execute }.not_to change(user, :last_activity_on)
end
+
+ it_behaves_like 'does not update last_activity_on'
end
end
@@ -104,7 +128,7 @@
end
let(:service) do
- service = described_class.new(user)
+ service = described_class.new(author: user)
::Gitlab::Database::LoadBalancing::Session.clear_session
@@ -123,7 +147,7 @@
end
context 'database load balancing is not configured' do
- let(:service) { described_class.new(user) }
+ let(:service) { described_class.new(author: user) }
it 'updates user without error' do
service.execute
diff --git a/spec/support/shared_examples/controllers/repositories/git_http_controller_shared_examples.rb b/spec/support/shared_examples/controllers/repositories/git_http_controller_shared_examples.rb
index 3a7588a5cc93b692006d3e659553a854b04f01ed..cc28a79b4ca48eb5820471bf8d1e4824a5d083d3 100644
--- a/spec/support/shared_examples/controllers/repositories/git_http_controller_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/repositories/git_http_controller_shared_examples.rb
@@ -61,9 +61,14 @@
end
it 'updates the user activity' do
- expect_next_instance_of(Users::ActivityService) do |activity_service|
- expect(activity_service).to receive(:execute)
- end
+ activity_project = container.is_a?(PersonalSnippet) ? nil : project
+
+ activity_service = instance_double(Users::ActivityService)
+
+ args = { author: user, project: activity_project, namespace: activity_project&.namespace }
+ expect(Users::ActivityService).to receive(:new).with(args).and_return(activity_service)
+
+ expect(activity_service).to receive(:execute)
get :info_refs, params: params
end