Commit 289e5ac0 authored by Matthias Käppler's avatar Matthias Käppler

Merge branch 'sy-instrument-error-tracking' into 'master'

Instrument error tracking metrics

See merge request gitlab-org/gitlab!82543
parents 4f713dc0 0337b92b
...@@ -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