Commit 55abaa52 authored by Imre Farkas's avatar Imre Farkas

Merge branch 'count-weekly-unique-visits' into 'master'

Track unique visits to analytics pages

See merge request gitlab-org/gitlab!33146
parents e47cc2a9 3cac5784
......@@ -3,11 +3,14 @@
class Dashboard::TodosController < Dashboard::ApplicationController
include ActionView::Helpers::NumberHelper
include PaginatedCollection
include Analytics::UniqueVisitsHelper
before_action :authorize_read_project!, only: :index
before_action :authorize_read_group!, only: :index
before_action :find_todos, only: [:index, :destroy_all]
track_unique_visits :index, target_id: 'u_analytics_todos'
def index
@sort = params[:sort]
@todos = @todos.page(params[:page])
......
# frozen_string_literal: true
class InstanceStatistics::CohortsController < InstanceStatistics::ApplicationController
include Analytics::UniqueVisitsHelper
before_action :authenticate_usage_ping_enabled_or_admin!
track_unique_visits :index, target_id: 'i_analytics_cohorts'
def index
if Gitlab::CurrentSettings.usage_ping_enabled
cohorts_results = Rails.cache.fetch('cohorts', expires_in: 1.day) do
......
# frozen_string_literal: true
class InstanceStatistics::DevOpsScoreController < InstanceStatistics::ApplicationController
include Analytics::UniqueVisitsHelper
track_unique_visits :index, target_id: 'i_analytics_dev_ops_score'
# rubocop: disable CodeReuse/ActiveRecord
def index
@metric = DevOpsScore::Metric.order(:created_at).last&.present
......
......@@ -4,10 +4,13 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController
include ActionView::Helpers::DateHelper
include ActionView::Helpers::TextHelper
include CycleAnalyticsParams
include Analytics::UniqueVisitsHelper
before_action :whitelist_query_limiting, only: [:show]
before_action :authorize_read_cycle_analytics!
track_unique_visits :show, target_id: 'p_analytics_valuestream'
def show
@cycle_analytics = ::CycleAnalytics::ProjectLevel.new(@project, options: options(cycle_analytics_project_params))
......
......@@ -2,12 +2,15 @@
class Projects::GraphsController < Projects::ApplicationController
include ExtractsPath
include Analytics::UniqueVisitsHelper
# Authorize
before_action :require_non_empty_project
before_action :assign_ref_vars
before_action :authorize_read_repository_graphs!
track_unique_visits :charts, target_id: 'p_analytics_repo'
def show
respond_to do |format|
format.html
......
......@@ -2,6 +2,7 @@
class Projects::PipelinesController < Projects::ApplicationController
include ::Gitlab::Utils::StrongMemoize
include Analytics::UniqueVisitsHelper
before_action :whitelist_query_limiting, only: [:create, :retry]
before_action :pipeline, except: [:index, :new, :create, :charts]
......@@ -20,6 +21,8 @@ class Projects::PipelinesController < Projects::ApplicationController
around_action :allow_gitaly_ref_name_caching, only: [:index, :show]
track_unique_visits :charts, target_id: 'p_analytics_pipelines'
wrap_parameters Ci::Pipeline
POLLING_INTERVAL = 10_000
......
# frozen_string_literal: true
module Analytics
module UniqueVisitsHelper
extend ActiveSupport::Concern
def visitor_id
return cookies[:visitor_id] if cookies[:visitor_id].present?
return unless current_user
uuid = SecureRandom.uuid
cookies[:visitor_id] = { value: uuid, expires: 24.months }
uuid
end
def track_visit(target_id)
return unless Feature.enabled?(:track_unique_visits)
return unless Gitlab::CurrentSettings.usage_ping_enabled?
return unless visitor_id
Gitlab::Analytics::UniqueVisits.new.track_visit(visitor_id, target_id)
end
class_methods do
def track_unique_visits(controller_actions, target_id:)
after_action only: controller_actions, if: -> { request.format.html? && request.headers['DNT'] != '1' } do
track_visit(target_id)
end
end
end
end
end
---
title: Add the unique visits data to the usage ping
merge_request: 33146
author:
type: changed
......@@ -597,6 +597,21 @@ appear to be associated to any of the services running, since they all appear to
| `sd` | `avg_cycle_analytics - production` | | | | |
| `missing` | `avg_cycle_analytics - production` | | | | |
| `total` | `avg_cycle_analytics` | | | | |
| `g_analytics_contribution` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/contribution_analytics |
| `g_analytics_insights` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/insights |
| `g_analytics_issues` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/issues_analytics |
| `g_analytics_productivity` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/analytics/productivity_analytics |
| `g_analytics_valuestream` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/analytics/value_stream_analytics |
| `p_analytics_pipelines` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/pipelines/charts |
| `p_analytics_code_reviews` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/-/analytics/code_reviews |
| `p_analytics_valuestream` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/-/value_stream_analytics |
| `p_analytics_insights` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/insights |
| `p_analytics_issues` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/-/analytics/issues_analytics |
| `p_analytics_repo` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/-/graphs/master/charts |
| `u_analytics_todos` | `analytics_unique_visits` | `manage` | | | Visits to /dashboard/todos |
| `i_analytics_cohorts` | `analytics_unique_visits` | `manage` | | | Visits to /-/instance_statistics/cohorts |
| `i_analytics_dev_ops_score` | `analytics_unique_visits` | `manage` | | | Visits to /-/instance_statistics/dev_ops_score |
| `analytics_unique_visits_for_any_target` | `analytics_unique_visits` | `manage` | | | Visits to any of the pages listed above |
| `clusters_applications_cert_managers` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with certificate managers enabled |
| `clusters_applications_helm` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with Helm enabled |
| `clusters_applications_ingress` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with Ingress enabled |
......@@ -766,6 +781,10 @@ The following is example content of the Usage Ping payload.
},
"total": 999
},
"analytics_unique_visits": {
"g_analytics_contribution": 999,
...
},
"usage_activity_by_stage": {
"configure": {
"project_clusters_enabled": 999,
......
# frozen_string_literal: true
class Groups::Analytics::CycleAnalyticsController < Analytics::CycleAnalyticsController
include Analytics::UniqueVisitsHelper
layout 'group'
before_action do
render_403 unless can?(current_user, :read_group_cycle_analytics, @group)
end
track_unique_visits :show, target_id: 'g_analytics_valuestream'
end
......@@ -24,6 +24,9 @@ class Groups::Analytics::ProductivityAnalyticsController < Groups::Analytics::Ap
before_action :validate_params, only: :show, if: -> { request.format.json? }
include IssuableCollections
include Analytics::UniqueVisitsHelper
track_unique_visits :show, target_id: 'g_analytics_productivity'
def show
respond_to do |format|
......
# frozen_string_literal: true
class Groups::ContributionAnalyticsController < Groups::ApplicationController
include Analytics::UniqueVisitsHelper
before_action :group
before_action :check_contribution_analytics_available!
before_action :authorize_read_contribution_analytics!
layout 'group'
track_unique_visits :show, target_id: 'g_analytics_contribution'
def show
@start_date = data_collector.from
......
......@@ -2,10 +2,13 @@
class Groups::InsightsController < Groups::ApplicationController
include InsightsActions
include Analytics::UniqueVisitsHelper
before_action :authorize_read_group!
before_action :authorize_read_insights_config_project!
track_unique_visits :show, target_id: 'g_analytics_insights'
private
def authorize_read_group!
......
......@@ -2,10 +2,13 @@
class Groups::IssuesAnalyticsController < Groups::ApplicationController
include IssuableCollections
include Analytics::UniqueVisitsHelper
before_action :authorize_read_group!
before_action :authorize_read_issue_analytics!
track_unique_visits :show, target_id: 'g_analytics_issues'
def show
respond_to do |format|
format.html
......
......@@ -3,12 +3,16 @@
module Projects
module Analytics
class CodeReviewsController < Projects::ApplicationController
include ::Analytics::UniqueVisitsHelper
before_action :authorize_read_code_review_analytics!
before_action do
push_frontend_feature_flag(:code_review_analytics_has_new_search)
push_frontend_feature_flag(:not_issuable_queries, @project, default_enabled: true)
end
track_unique_visits :index, target_id: 'p_analytics_code_reviews'
def index
end
end
......
......@@ -2,9 +2,12 @@
class Projects::Analytics::IssuesAnalyticsController < Projects::ApplicationController
include IssuableCollections
include ::Analytics::UniqueVisitsHelper
before_action :authorize_read_issue_analytics!
track_unique_visits :show, target_id: 'p_analytics_issues'
def show
respond_to do |format|
format.html
......
......@@ -2,11 +2,14 @@
class Projects::InsightsController < Projects::ApplicationController
include InsightsActions
include Analytics::UniqueVisitsHelper
helper_method :project_insights_config
before_action :authorize_read_project!
track_unique_visits :show, target_id: 'p_analytics_insights'
private
def authorize_read_project!
......
......@@ -74,6 +74,18 @@ RSpec.describe Groups::Analytics::ProductivityAnalyticsController do
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'when the feature is licensed' do
before do
stub_licensed_features(productivity_analytics: true)
group.add_owner(current_user)
end
it_behaves_like 'tracking unique visits', :show do
let(:request_params) { { group_id: group } }
let(:target_id) { 'g_analytics_productivity' }
end
end
end
describe 'GET show.json' do
......
......@@ -219,10 +219,15 @@ RSpec.describe Groups::ContributionAnalyticsController do
end
end
describe 'GET #index' do
describe 'GET #show' do
subject { get :show, params: { group_id: group.to_param } }
it_behaves_like 'disabled when using an external authorization service'
it_behaves_like 'tracking unique visits', :show do
let(:request_params) { { group_id: group.to_param } }
let(:target_id) { 'g_analytics_contribution' }
end
end
end
end
......@@ -117,6 +117,13 @@ RSpec.describe Groups::InsightsController do
it_behaves_like '200 status'
end
describe 'GET #show' do
it_behaves_like 'tracking unique visits', :show do
let(:request_params) { params.merge(group_id: parent_group.to_param) }
let(:target_id) { 'g_analytics_insights' }
end
end
end
context 'when the configuration is attached to a nested group' do
......
......@@ -18,4 +18,20 @@ RSpec.describe Groups::IssuesAnalyticsController do
let(:params) { { group_id: group.to_param } }
end
describe 'GET #show' do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
before do
group.add_owner(user)
sign_in(user)
stub_licensed_features(issues_analytics: true)
end
it_behaves_like 'tracking unique visits', :show do
let(:request_params) { { group_id: group.to_param } }
let(:target_id) { 'g_analytics_issues' }
end
end
end
......@@ -16,5 +16,16 @@ RSpec.describe Projects::Analytics::IssuesAnalyticsController do
end
let(:params) { { namespace_id: group.to_param, project_id: project1.to_param } }
describe 'GET #show' do
before do
stub_licensed_features(issues_analytics: true)
end
it_behaves_like 'tracking unique visits', :show do
let(:request_params) { { namespace_id: project1.namespace, project_id: project1 } }
let(:target_id) { 'p_analytics_issues' }
end
end
end
end
......@@ -50,3 +50,18 @@ RSpec.describe Projects::Analytics::CodeReviewsController, type: :request do
end
end
end
describe Projects::Analytics::CodeReviewsController, type: :controller do
let(:user) { create :user }
let(:project) { create(:project) }
before do
sign_in user
project.add_reporter(user)
end
it_behaves_like 'tracking unique visits', :index do
let(:request_params) { { namespace_id: project.namespace, project_id: project } }
let(:target_id) { 'p_analytics_code_reviews' }
end
end
# frozen_string_literal: true
module Gitlab
module Analytics
class UniqueVisits
TARGET_IDS = Set[
'g_analytics_contribution',
'g_analytics_insights',
'g_analytics_issues',
'g_analytics_productivity',
'g_analytics_valuestream',
'p_analytics_pipelines',
'p_analytics_code_reviews',
'p_analytics_valuestream',
'p_analytics_insights',
'p_analytics_issues',
'p_analytics_repo',
'u_analytics_todos',
'i_analytics_cohorts',
'i_analytics_dev_ops_score'
].freeze
KEY_EXPIRY_LENGTH = 28.days
def track_visit(visitor_id, target_id, time = Time.zone.now)
target_key = key(target_id, time)
Gitlab::Redis::SharedState.with do |redis|
redis.multi do |multi|
multi.pfadd(target_key, visitor_id)
multi.expire(target_key, KEY_EXPIRY_LENGTH)
end
end
end
def weekly_unique_visits_for_target(target_id, week_of: 7.days.ago)
Gitlab::Redis::SharedState.with do |redis|
redis.pfcount(key(target_id, week_of))
end
end
def weekly_unique_visits_for_any_target(week_of: 7.days.ago)
keys = TARGET_IDS.map { |target_id| key(target_id, week_of) }
Gitlab::Redis::SharedState.with do |redis|
redis.pfcount(*keys)
end
end
private
def key(target_id, time)
raise "Invalid target id #{target_id}" unless TARGET_IDS.include?(target_id.to_s)
year_week = time.strftime('%G-%V')
"#{target_id}-#{year_week}"
end
end
end
end
......@@ -27,7 +27,7 @@ module Gitlab
end
def uncached_data
clear_memoized_limits
clear_memoized
with_finished_at(:recording_ce_finished_at) do
license_usage_data
......@@ -39,6 +39,7 @@ module Gitlab
.merge(topology_usage_data)
.merge(usage_activity_by_stage)
.merge(usage_activity_by_stage(:usage_activity_by_stage_monthly, default_time_period))
.merge(analytics_unique_visits_data)
end
end
......@@ -511,8 +512,23 @@ module Gitlab
{}
end
def analytics_unique_visits_data
results = ::Gitlab::Analytics::UniqueVisits::TARGET_IDS.each_with_object({}) do |target_id, hash|
hash[target_id] = redis_usage_data { unique_visit_service.weekly_unique_visits_for_target(target_id) }
end
results['analytics_unique_visits_for_any_target'] = redis_usage_data { unique_visit_service.weekly_unique_visits_for_any_target }
{ analytics_unique_visits: results }
end
private
def unique_visit_service
strong_memoize(:unique_visit_service) do
::Gitlab::Analytics::UniqueVisits.new
end
end
def total_alert_issues
# Remove prometheus table queries once they are deprecated
# To be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/217407.
......@@ -535,9 +551,10 @@ module Gitlab
end
end
def clear_memoized_limits
def clear_memoized
clear_memoization(:user_minimum_id)
clear_memoization(:user_maximum_id)
clear_memoization(:unique_visit_service)
end
# rubocop: disable CodeReuse/ActiveRecord
......
......@@ -42,6 +42,15 @@ RSpec.describe Dashboard::TodosController do
expect(response).to have_gitlab_http_status(:ok)
end
context 'tracking visits' do
let_it_be(:authorized_project) { create(:project, :public) }
it_behaves_like 'tracking unique visits', :index do
let(:request_params) { { project_id: authorized_project.id } }
let(:target_id) { 'u_analytics_todos' }
end
end
end
context "with render_views" do
......
......@@ -18,4 +18,11 @@ RSpec.describe InstanceStatistics::CohortsController do
expect(response).to have_gitlab_http_status(:not_found)
end
describe 'GET #index' do
it_behaves_like 'tracking unique visits', :index do
let(:request_params) { {} }
let(:target_id) { 'i_analytics_cohorts' }
end
end
end
......@@ -4,4 +4,17 @@ require 'spec_helper'
RSpec.describe InstanceStatistics::DevOpsScoreController do
it_behaves_like 'instance statistics availability'
describe 'GET #index' do
let(:user) { create(:user) }
before do
sign_in(user)
end
it_behaves_like 'tracking unique visits', :index do
let(:request_params) { {} }
let(:target_id) { 'i_analytics_dev_ops_score' }
end
end
end
......@@ -25,6 +25,13 @@ RSpec.describe Projects::CycleAnalyticsController do
end
end
context 'tracking visits to html page' do
it_behaves_like 'tracking unique visits', :show do
let(:request_params) { { namespace_id: project.namespace, project_id: project } }
let(:target_id) { 'p_analytics_valuestream' }
end
end
describe 'cycle analytics not set up flag' do
context 'with no data' do
it 'is true' do
......
......@@ -80,6 +80,15 @@ RSpec.describe Projects::GraphsController do
expect(assigns[:daily_coverage_options]).to be_nil
end
end
it_behaves_like 'tracking unique visits', :charts do
before do
sign_in(user)
end
let(:request_params) { { namespace_id: project.namespace.path, project_id: project.path, id: 'master' } }
let(:target_id) { 'p_analytics_repo' }
end
end
context 'when languages were previously detected' do
......
......@@ -689,6 +689,15 @@ RSpec.describe Projects::PipelinesController do
end
end
describe 'GET #charts' do
let(:pipeline) { create(:ci_pipeline, project: project) }
it_behaves_like 'tracking unique visits', :charts do
let(:request_params) { { namespace_id: project.namespace, project_id: project, id: pipeline.id } }
let(:target_id) { 'p_analytics_pipelines' }
end
end
describe 'POST create' do
let(:project) { create(:project, :public, :repository) }
......
# frozen_string_literal: true
require "spec_helper"
describe Analytics::UniqueVisitsHelper do
include Devise::Test::ControllerHelpers
describe '#track_visit' do
let(:target_id) { 'p_analytics_valuestream' }
let(:current_user) { create(:user) }
before do
stub_feature_flags(track_unique_visits: true)
end
it 'does not track visits if feature flag disabled' do
stub_feature_flags(track_unique_visits: false)
sign_in(current_user)
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit)
helper.track_visit(target_id)
end
it 'does not track visits if usage ping is disabled' do
sign_in(current_user)
expect(Gitlab::CurrentSettings).to receive(:usage_ping_enabled?).and_return(false)
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit)
helper.track_visit(target_id)
end
it 'does not track visit if user is not logged in' do
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit)
helper.track_visit(target_id)
end
it 'tracks visit if user is logged in' do
sign_in(current_user)
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit)
helper.track_visit(target_id)
end
it 'tracks visit if user is not logged in, but has the cookie already' do
helper.request.cookies[:visitor_id] = { value: SecureRandom.uuid, expires: 24.months }
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit)
helper.track_visit(target_id)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state do
let(:unique_visits) { Gitlab::Analytics::UniqueVisits.new }
let(:target1_id) { 'g_analytics_contribution' }
let(:target2_id) { 'g_analytics_insights' }
let(:target3_id) { 'g_analytics_issues' }
let(:visitor1_id) { 'dfb9d2d2-f56c-4c77-8aeb-6cddc4a1f857' }
let(:visitor2_id) { '1dd9afb2-a3ee-4de1-8ae3-a405579c8584' }
describe '#track_visit' do
it 'tracks the unique weekly visits for targets' do
unique_visits.track_visit(visitor1_id, target1_id, 7.days.ago)
unique_visits.track_visit(visitor1_id, target1_id, 7.days.ago)
unique_visits.track_visit(visitor2_id, target1_id, 7.days.ago)
unique_visits.track_visit(visitor2_id, target2_id, 7.days.ago)
unique_visits.track_visit(visitor1_id, target2_id, 8.days.ago)
unique_visits.track_visit(visitor1_id, target2_id, 15.days.ago)
expect(unique_visits.weekly_unique_visits_for_target(target1_id)).to eq(2)
expect(unique_visits.weekly_unique_visits_for_target(target2_id)).to eq(1)
expect(unique_visits.weekly_unique_visits_for_target(target2_id, week_of: 15.days.ago)).to eq(1)
expect(unique_visits.weekly_unique_visits_for_target(target3_id)).to eq(0)
expect(unique_visits.weekly_unique_visits_for_any_target).to eq(2)
expect(unique_visits.weekly_unique_visits_for_any_target(week_of: 15.days.ago)).to eq(1)
expect(unique_visits.weekly_unique_visits_for_any_target(week_of: 30.days.ago)).to eq(0)
end
it 'sets the keys in Redis to expire automatically after 28 days' do
unique_visits.track_visit(visitor1_id, target1_id)
Gitlab::Redis::SharedState.with do |redis|
redis.scan_each(match: "#{target1_id}-*").each do |key|
expect(redis.ttl(key)).to be_within(5.seconds).of(28.days)
end
end
end
it 'raises an error if an invalid target id is given' do
invalid_target_id = "x_invalid"
expect do
unique_visits.track_visit(visitor1_id, invalid_target_id)
end.to raise_error("Invalid target id #{invalid_target_id}")
end
end
end
......@@ -672,4 +672,36 @@ describe Gitlab::UsageData, :aggregate_failures do
end
end
end
describe '.analytics_unique_visits_data' do
subject { described_class.analytics_unique_visits_data }
it 'returns the number of unique visits to pages with analytics features' do
::Gitlab::Analytics::UniqueVisits::TARGET_IDS.each do |target_id|
expect_any_instance_of(::Gitlab::Analytics::UniqueVisits).to receive(:weekly_unique_visits_for_target).with(target_id).and_return(123)
end
expect_any_instance_of(::Gitlab::Analytics::UniqueVisits).to receive(:weekly_unique_visits_for_any_target).and_return(543)
expect(subject).to eq({
analytics_unique_visits: {
'g_analytics_contribution' => 123,
'g_analytics_insights' => 123,
'g_analytics_issues' => 123,
'g_analytics_productivity' => 123,
'g_analytics_valuestream' => 123,
'p_analytics_pipelines' => 123,
'p_analytics_code_reviews' => 123,
'p_analytics_valuestream' => 123,
'p_analytics_insights' => 123,
'p_analytics_issues' => 123,
'p_analytics_repo' => 123,
'u_analytics_todos' => 123,
'i_analytics_cohorts' => 123,
'i_analytics_dev_ops_score' => 123,
'analytics_unique_visits_for_any_target' => 543
}
})
end
end
end
# frozen_string_literal: true
RSpec.shared_examples 'tracking unique visits' do |method|
it 'tracks unique visit if the format is HTML' do
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit).with(instance_of(String), target_id)
get method, params: request_params, format: :html
end
it 'tracks unique visit if DNT is not enabled' do
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit).with(instance_of(String), target_id)
request.headers['DNT'] = '0'
get method, params: request_params, format: :html
end
it 'does not track unique visit if DNT is enabled' do
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit)
request.headers['DNT'] = '1'
get method, params: request_params, format: :html
end
it 'does not track unique visit if the format is JSON' do
expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit)
get method, params: request_params, format: :json
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