Commit 0337b92b authored by Sarah Yasonik's avatar Sarah Yasonik Committed by Matthias Käppler

Instrument error tracking metrics

Track unique users navigating to the error tracking list and details views
parent 1f6c3d73
...@@ -79,7 +79,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle ...@@ -79,7 +79,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
end end
def list_issues_params def list_issues_params
params.permit(:search_term, :sort, :cursor, :issue_status) params.permit(:search_term, :sort, :cursor, :issue_status).merge(tracking_event: :error_tracking_view_list)
end end
def issue_update_params def issue_update_params
...@@ -87,7 +87,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle ...@@ -87,7 +87,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
end end
def issue_details_params def issue_details_params
params.permit(:issue_id) params.permit(:issue_id).merge(tracking_event: :error_tracking_view_details)
end end
def set_issue_id def set_issue_id
......
# frozen_string_literal: true # frozen_string_literal: true
module ErrorTracking module ErrorTracking
class BaseService < ::BaseService class BaseService < ::BaseProjectService
include Gitlab::Utils::UsageData
def initialize(project, user = nil, params = {})
super(project: project, current_user: user, params: params.dup)
end
def execute def execute
return unauthorized if unauthorized return unauthorized if unauthorized
...@@ -21,6 +27,8 @@ module ErrorTracking ...@@ -21,6 +27,8 @@ module ErrorTracking
yield if block_given? yield if block_given?
track_usage_event(params[:tracking_event], current_user.id) if params[:tracking_event]
success(parse_response(response)) success(parse_response(response))
end end
......
---
name: track_error_tracking_activity
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82543
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/355112
milestone: '14.9'
type: development
group: group::respond
default_enabled: false
---
key_path: redis_hll_counters.error_tracking.error_tracking_view_details_monthly
description: Unique users viewing the error details page
product_section: ops
product_stage: monitor
product_group: respond
product_category: error_tracking
value_type: number
status: active
milestone: "14.9"
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82543
time_frame: 28d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- error_tracking_view_details
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
---
key_path: redis_hll_counters.error_tracking.error_tracking_view_list_monthly
description: Unique users viewing the list of errors in the project
product_section: ops
product_stage: monitor
product_group: respond
product_category: error_tracking
value_type: number
status: active
milestone: "14.9"
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82543
time_frame: 28d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- error_tracking_view_list
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
---
key_path: redis_hll_counters.error_tracking.error_tracking_total_unique_counts_monthly
description: Total unique users accessing error tracking routes
product_section: ops
product_stage: monitor
product_group: respond
product_category: error_tracking
value_type: number
status: active
milestone: "14.9"
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82543
time_frame: 28d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- error_tracking_view_list
- error_tracking_view_details
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
---
key_path: redis_hll_counters.error_tracking.error_tracking_view_details_weekly
description: Unique users viewing the error details page
product_section: ops
product_stage: monitor
product_group: respond
product_category: error_tracking
value_type: number
status: active
milestone: "14.9"
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82543
time_frame: 7d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- error_tracking_view_details
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
---
key_path: redis_hll_counters.error_tracking.error_tracking_view_list_weekly
description: Unique users viewing the list of errors in the project
product_section: ops
product_stage: monitor
product_group: respond
product_category: error_tracking
value_type: number
status: active
milestone: "14.9"
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82543
time_frame: 7d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- error_tracking_view_list
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
---
key_path: redis_hll_counters.error_tracking.error_tracking_total_unique_counts_weekly
description: Total unique users accessing error tracking routes
product_section: ops
product_stage: monitor
product_group: respond
product_category: error_tracking
value_type: number
status: active
milestone: "14.9"
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82543
time_frame: 7d
data_source: redis_hll
data_category: optional
instrumentation_class: RedisHLLMetric
options:
events:
- error_tracking_view_list
- error_tracking_view_details
distribution:
- ce
- ee
tier:
- free
- premium
- ultimate
...@@ -20,8 +20,8 @@ RSpec.describe SearchController, :elastic do ...@@ -20,8 +20,8 @@ RSpec.describe SearchController, :elastic do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
subject(:request) { get :show, params: { scope: 'projects', search: 'term' } } subject(:request) { get :show, params: { scope: 'projects', search: 'term' } }
let(:target_id) { 'i_search_advanced' } let(:target_event) { 'i_search_advanced' }
let(:expected_type) { instance_of(String) } let(:expected_value) { instance_of(String) }
end end
end end
...@@ -29,7 +29,7 @@ RSpec.describe SearchController, :elastic do ...@@ -29,7 +29,7 @@ RSpec.describe SearchController, :elastic do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:request_params) { { group_id: group.id, scope: 'blobs', search: 'term' } } let(:request_params) { { group_id: group.id, scope: 'blobs', search: 'term' } }
let(:target_id) { 'i_search_paid' } let(:target_event) { 'i_search_paid' }
context 'on Gitlab.com' do context 'on Gitlab.com' do
before do before do
...@@ -40,7 +40,7 @@ RSpec.describe SearchController, :elastic do ...@@ -40,7 +40,7 @@ RSpec.describe SearchController, :elastic do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
subject(:request) { get :show, params: request_params } subject(:request) { get :show, params: request_params }
let(:expected_type) { instance_of(String) } let(:expected_value) { instance_of(String) }
end end
end end
...@@ -57,13 +57,13 @@ RSpec.describe SearchController, :elastic do ...@@ -57,13 +57,13 @@ RSpec.describe SearchController, :elastic do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
subject(:request) { get :show, params: request_params } subject(:request) { get :show, params: request_params }
let(:expected_type) { instance_of(String) } let(:expected_value) { instance_of(String) }
end end
end end
it 'does not track if there is no license available' do it 'does not track if there is no license available' do
stub_licensed_features(elastic_search: false) stub_licensed_features(elastic_search: false)
expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event).with(target_id, values: instance_of(String)) expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event).with(target_event, values: instance_of(String))
get :show, params: request_params, format: :html get :show, params: request_params, format: :html
end end
......
...@@ -114,8 +114,8 @@ RSpec.describe Groups::EpicBoardsController do ...@@ -114,8 +114,8 @@ RSpec.describe Groups::EpicBoardsController do
let_it_be(:board) { create(:epic_board, group: group) } let_it_be(:board) { create(:epic_board, group: group) }
subject(:request) { list_boards } subject(:request) { list_boards }
let(:target_id) { 'g_project_management_users_viewing_epic_boards' } let(:target_event) { 'g_project_management_users_viewing_epic_boards' }
let(:expected_type) { instance_of(String) } let(:expected_value) { instance_of(String) }
end end
def list_boards(format: :html) def list_boards(format: :html)
...@@ -215,8 +215,8 @@ RSpec.describe Groups::EpicBoardsController do ...@@ -215,8 +215,8 @@ RSpec.describe Groups::EpicBoardsController do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
subject(:request) { read_board(board: board) } subject(:request) { read_board(board: board) }
let(:target_id) { 'g_project_management_users_viewing_epic_boards' } let(:target_event) { 'g_project_management_users_viewing_epic_boards' }
let(:expected_type) { instance_of(String) } let(:expected_value) { instance_of(String) }
end end
def read_board(board:, format: :html) def read_board(board:, format: :html)
......
...@@ -51,8 +51,8 @@ RSpec.describe NetworkPolicies::ResourcesService do ...@@ -51,8 +51,8 @@ RSpec.describe NetworkPolicies::ResourcesService do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
subject(:request) { service.execute } subject(:request) { service.execute }
let(:target_id) { 'clusters_using_network_policies_ui' } let(:target_event) { 'clusters_using_network_policies_ui' }
let(:expected_type) { instance_of(Integer) } let(:expected_value) { instance_of(Integer) }
before do before do
allow(kubeclient).to receive(:get_network_policies) allow(kubeclient).to receive(:get_network_policies)
......
...@@ -26,6 +26,7 @@ module Gitlab ...@@ -26,6 +26,7 @@ module Gitlab
ecosystem ecosystem
epic_boards_usage epic_boards_usage
epics_usage epics_usage
error_tracking
ide_edit ide_edit
incident_management incident_management
issues_edit issues_edit
......
---
- name: error_tracking_view_details
category: error_tracking
redis_slot: error_tracking
aggregation: weekly
feature_flag: track_error_tracking_activity
- name: error_tracking_view_list
category: error_tracking
redis_slot: error_tracking
aggregation: weekly
feature_flag: track_error_tracking_activity
...@@ -366,8 +366,8 @@ RSpec.describe Projects::BlobController do ...@@ -366,8 +366,8 @@ RSpec.describe Projects::BlobController do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
subject(:request) { put :update, params: default_params } subject(:request) { put :update, params: default_params }
let(:target_id) { 'g_edit_by_sfe' } let(:target_event) { 'g_edit_by_sfe' }
let(:expected_type) { instance_of(Integer) } let(:expected_value) { instance_of(Integer) }
end end
end end
...@@ -516,8 +516,8 @@ RSpec.describe Projects::BlobController do ...@@ -516,8 +516,8 @@ RSpec.describe Projects::BlobController do
subject(:request) { post :create, params: default_params } subject(:request) { post :create, params: default_params }
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
let(:target_id) { 'g_edit_by_sfe' } let(:target_event) { 'g_edit_by_sfe' }
let(:expected_type) { instance_of(Integer) } let(:expected_value) { instance_of(Integer) }
end end
it 'redirects to blob' do it 'redirects to blob' do
......
...@@ -50,9 +50,7 @@ RSpec.describe Projects::ErrorTrackingController do ...@@ -50,9 +50,7 @@ RSpec.describe Projects::ErrorTrackingController do
let(:external_url) { 'http://example.com' } let(:external_url) { 'http://example.com' }
context 'no data' do context 'no data' do
let(:permitted_params) do let(:permitted_params) { permit_index_parameters!({}) }
ActionController::Parameters.new({}).permit!
end
before do before do
expect(ErrorTracking::ListIssuesService) expect(ErrorTracking::ListIssuesService)
...@@ -75,9 +73,7 @@ RSpec.describe Projects::ErrorTrackingController do ...@@ -75,9 +73,7 @@ RSpec.describe Projects::ErrorTrackingController do
let(:search_term) { 'something' } let(:search_term) { 'something' }
let(:sort) { 'last_seen' } let(:sort) { 'last_seen' }
let(:params) { project_params(format: :json, search_term: search_term, sort: sort, cursor: cursor) } let(:params) { project_params(format: :json, search_term: search_term, sort: sort, cursor: cursor) }
let(:permitted_params) do let(:permitted_params) { permit_index_parameters!(search_term: search_term, sort: sort, cursor: cursor) }
ActionController::Parameters.new(search_term: search_term, sort: sort, cursor: cursor).permit!
end
before do before do
expect(ErrorTracking::ListIssuesService) expect(ErrorTracking::ListIssuesService)
...@@ -114,7 +110,7 @@ RSpec.describe Projects::ErrorTrackingController do ...@@ -114,7 +110,7 @@ RSpec.describe Projects::ErrorTrackingController do
context 'without extra params' do context 'without extra params' do
before do before do
expect(ErrorTracking::ListIssuesService) expect(ErrorTracking::ListIssuesService)
.to receive(:new).with(project, user, {}) .to receive(:new).with(project, user, permit_index_parameters!({}))
.and_return(list_issues_service) .and_return(list_issues_service)
end end
...@@ -179,6 +175,15 @@ RSpec.describe Projects::ErrorTrackingController do ...@@ -179,6 +175,15 @@ RSpec.describe Projects::ErrorTrackingController do
end end
end end
end end
private
def permit_index_parameters!(params)
ActionController::Parameters.new(
**params,
tracking_event: :error_tracking_view_list
).permit!
end
end end
describe 'GET #issue_details' do describe 'GET #issue_details' do
...@@ -188,7 +193,8 @@ RSpec.describe Projects::ErrorTrackingController do ...@@ -188,7 +193,8 @@ RSpec.describe Projects::ErrorTrackingController do
let(:permitted_params) do let(:permitted_params) do
ActionController::Parameters.new( ActionController::Parameters.new(
{ issue_id: issue_id.to_s } issue_id: issue_id.to_s,
tracking_event: :error_tracking_view_details
).permit! ).permit!
end end
......
...@@ -251,8 +251,8 @@ RSpec.describe SearchController do ...@@ -251,8 +251,8 @@ RSpec.describe SearchController do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
subject(:request) { get :show, params: { scope: 'projects', search: 'term' } } subject(:request) { get :show, params: { scope: 'projects', search: 'term' } }
let(:target_id) { 'i_search_total' } let(:target_event) { 'i_search_total' }
let(:expected_type) { instance_of(String) } let(:expected_value) { instance_of(String) }
end end
end end
......
...@@ -176,8 +176,8 @@ RSpec.describe SnippetsController do ...@@ -176,8 +176,8 @@ RSpec.describe SnippetsController do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
subject(:request) { get :show, params: { id: public_snippet.to_param } } subject(:request) { get :show, params: { id: public_snippet.to_param } }
let(:target_id) { 'i_snippets_show' } let(:target_event) { 'i_snippets_show' }
let(:expected_type) { instance_of(String) } let(:expected_value) { instance_of(String) }
end end
end end
......
...@@ -52,7 +52,8 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s ...@@ -52,7 +52,8 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
'geo', 'geo',
'growth', 'growth',
'work_items', 'work_items',
'ci_users' 'ci_users',
'error_tracking'
) )
end end
end end
......
...@@ -36,8 +36,8 @@ RSpec.describe API::Terraform::State do ...@@ -36,8 +36,8 @@ RSpec.describe API::Terraform::State do
let(:current_user) { maintainer } let(:current_user) { maintainer }
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
let(:target_id) { 'p_terraform_state_api_unique_users' } let(:target_event) { 'p_terraform_state_api_unique_users' }
let(:expected_type) { instance_of(Integer) } let(:expected_value) { instance_of(Integer) }
end end
end end
end end
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe ErrorTracking::BaseService do RSpec.describe ErrorTracking::BaseService do
describe '#compose_response' do describe '#compose_response' do
let(:project) { double('project') } let(:project) { double('project') }
let(:user) { double('user') } let(:user) { double('user', id: non_existing_record_id) }
let(:service) { described_class.new(project, user) } let(:service) { described_class.new(project, user) }
it 'returns bad_request error when response has an error key' do it 'returns bad_request error when response has an error key' do
...@@ -68,6 +68,16 @@ RSpec.describe ErrorTracking::BaseService do ...@@ -68,6 +68,16 @@ RSpec.describe ErrorTracking::BaseService do
expect(result[:animal]).to eq(:fish) expect(result[:animal]).to eq(:fish)
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
end end
context 'when tracking_event is provided' do
let(:service) { described_class.new(project, user, tracking_event: :error_tracking_view_list) }
it_behaves_like 'tracking unique hll events' do
let(:target_event) { 'error_tracking_view_list' }
let(:expected_value) { non_existing_record_id }
let(:request) { service.send(:compose_response, data) }
end
end
end end
end end
end end
......
...@@ -2,14 +2,14 @@ ...@@ -2,14 +2,14 @@
# #
# Requires a context containing: # Requires a context containing:
# - request # - request
# - expected_type # - expected_value
# - target_id # - target_event
RSpec.shared_examples 'tracking unique hll events' do RSpec.shared_examples 'tracking unique hll events' do
it 'tracks unique event' do it 'tracks unique event' do
expect(Gitlab::UsageDataCounters::HLLRedisCounter).to( expect(Gitlab::UsageDataCounters::HLLRedisCounter).to(
receive(:track_event) receive(:track_event)
.with(target_id, values: expected_type) .with(target_event, values: expected_value)
.and_call_original # we call original to trigger additional validations; otherwise the method is stubbed .and_call_original # we call original to trigger additional validations; otherwise the method is stubbed
) )
......
...@@ -220,8 +220,8 @@ RSpec.shared_examples 'wiki controller actions' do ...@@ -220,8 +220,8 @@ RSpec.shared_examples 'wiki controller actions' do
context 'page view tracking' do context 'page view tracking' do
it_behaves_like 'tracking unique hll events' do it_behaves_like 'tracking unique hll events' do
let(:target_id) { 'wiki_action' } let(:target_event) { 'wiki_action' }
let(:expected_type) { instance_of(String) } let(:expected_value) { instance_of(String) }
end end
it 'increases the page view counter' do it 'increases the page view counter' do
......
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