Commit 62d1c9ed authored by Peter Leitzen's avatar Peter Leitzen

Implement error tracking controller

The frontend part will be done in a different MR.
parent 51b3ee89
# frozen_string_literal: true
class Projects::ErrorTrackingController < Projects::ApplicationController
before_action :check_feature_flag!
before_action :authorize_read_sentry_issue!
before_action :push_feature_flag_to_frontend
POLLING_INTERVAL = 10_000
def index
respond_to do |format|
format.html
format.json do
set_polling_interval
render_index_json
end
end
end
private
def render_index_json
service = ErrorTracking::ListIssuesService.new(project, current_user)
result = service.execute
unless result[:status] == :success
return render json: { message: result[:message] },
status: result[:http_status] || :bad_request
end
render json: {
errors: serialize_errors(result[:issues]),
external_url: service.external_url
}
end
def set_polling_interval
Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL)
end
def serialize_errors(errors)
ErrorTracking::ErrorSerializer
.new(project: project, user: current_user)
.represent(errors)
end
def check_feature_flag!
render_404 unless Feature.enabled?(:error_tracking, project)
end
def push_feature_flag_to_frontend
push_frontend_feature_flag(:error_tracking, current_user)
end
end
...@@ -2,6 +2,10 @@ ...@@ -2,6 +2,10 @@
module ErrorTracking module ErrorTracking
class ProjectErrorTrackingSetting < ActiveRecord::Base class ProjectErrorTrackingSetting < ActiveRecord::Base
include ReactiveCaching
self.reactive_cache_key = ->(setting) { [setting.class.model_name.singular, setting.project_id] }
belongs_to :project belongs_to :project
validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true } validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true }
...@@ -13,23 +17,38 @@ module ErrorTracking ...@@ -13,23 +17,38 @@ module ErrorTracking
key: Settings.attr_encrypted_db_key_base_truncated, key: Settings.attr_encrypted_db_key_base_truncated,
algorithm: 'aes-256-gcm' algorithm: 'aes-256-gcm'
after_save :clear_reactive_cache!
def sentry_client def sentry_client
Sentry::Client.new(api_url, token) Sentry::Client.new(api_url, token)
end end
def sentry_external_url def sentry_external_url
extract_external_url self.class.extract_sentry_external_url(api_url)
end end
private def list_sentry_issues(opts = {})
with_reactive_cache('list_issues', opts.stringify_keys) do |result|
{ issues: result }
end
end
def calculate_reactive_cache(request, opts)
case request
when 'list_issues'
sentry_client.list_issues(**opts.symbolize_keys)
end
end
# http://HOST/api/0/projects/ORG/PROJECT # http://HOST/api/0/projects/ORG/PROJECT
# -> # ->
# http://HOST/ORG/PROJECT # http://HOST/ORG/PROJECT
def extract_external_url def self.extract_sentry_external_url(url)
api_url.sub('api/0/projects/', '') url.sub('api/0/projects/', '')
end end
private
def validate_api_url_path def validate_api_url_path
unless URI(api_url).path.starts_with?('/api/0/projects') unless URI(api_url).path.starts_with?('/api/0/projects')
errors.add(:api_url, 'path needs to start with /api/0/projects') errors.add(:api_url, 'path needs to start with /api/0/projects')
......
# frozen_string_literal: true
module ErrorTracking
class ErrorEntity < Grape::Entity
expose :id, :title, :type, :user_count, :count,
:first_seen, :last_seen, :message, :culprit,
:external_url, :project_id, :project_name, :project_slug,
:short_id, :status, :frequency
end
end
# frozen_string_literal: true
module ErrorTracking
class ErrorSerializer < BaseSerializer
entity ErrorEntity
end
end
...@@ -2,15 +2,22 @@ ...@@ -2,15 +2,22 @@
module ErrorTracking module ErrorTracking
class ListIssuesService < ::BaseService class ListIssuesService < ::BaseService
DEFAULT_ISSUE_STATUS = 'unresolved'.freeze DEFAULT_ISSUE_STATUS = 'unresolved'
DEFAULT_LIMIT = 20 DEFAULT_LIMIT = 20
def execute def execute
return error('not enabled') unless valid? return error('not enabled') unless enabled?
return error('access denied') unless can?(current_user, :read_sentry_issue, project) return error('access denied') unless can_read?
issues = sentry_client.list_issues(issue_status: issue_status, limit: limit) result = project_error_tracking_setting
success(issues: issues) .list_sentry_issues(issue_status: issue_status, limit: limit)
# our results are not yet ready
unless result
return error('not ready', :no_content)
end
success(issues: result[:issues])
end end
def external_url def external_url
...@@ -31,12 +38,12 @@ module ErrorTracking ...@@ -31,12 +38,12 @@ module ErrorTracking
params[:limit] || DEFAULT_LIMIT params[:limit] || DEFAULT_LIMIT
end end
def valid? def enabled?
project_error_tracking_setting&.enabled? project_error_tracking_setting&.enabled?
end end
def sentry_client def can_read?
project_error_tracking_setting&.sentry_client can?(current_user, :read_sentry_issue, project)
end end
end end
end end
...@@ -538,6 +538,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -538,6 +538,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end end
end end
resources :error_tracking, only: [:index], controller: :error_tracking
# Since both wiki and repository routing contains wildcard characters # Since both wiki and repository routing contains wildcard characters
# its preferable to keep it below all other project routes # its preferable to keep it below all other project routes
draw :wiki draw :wiki
......
...@@ -61,7 +61,13 @@ module Sentry ...@@ -61,7 +61,13 @@ module Sentry
end end
def issue_url(id) def issue_url(id)
"#{issues_api_url}#{id}/" issues_url = @url + "/issues/#{id}"
issues_url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(issues_url)
uri = URI(issues_url)
uri.path.squeeze!('/')
uri.to_s
end end
def map_to_error(issue) def map_to_error(issue)
......
# frozen_string_literal: true
require 'rails_helper'
describe Projects::ErrorTrackingController do
set(:project) { create(:project) }
set(:user) { create(:user) }
before do
sign_in(user)
project.add_maintainer(user)
end
describe 'GET #index' do
describe 'html' do
it 'renders index with 200 status code' do
get :index, params: project_params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template(:index)
end
context 'with feature flag disabled' do
before do
stub_feature_flags(error_tracking: false)
end
it 'returns 404' do
get :index, params: project_params
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'with insufficient permissions' do
before do
project.add_guest(user)
end
it 'returns 404' do
get :index, params: project_params
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'with an anonymous user' do
before do
sign_out(user)
end
it 'redirects to sign-in page' do
get :index, params: project_params
expect(response).to redirect_to(new_user_session_path)
end
end
end
describe 'format json' do
shared_examples 'no data' do
it 'returns no data' do
get :index, params: project_params(format: :json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('error_tracking/index')
expect(json_response['external_url']).to be_nil
expect(json_response['errors']).to eq([])
end
end
let(:list_issues_service) { spy(:list_issues_service) }
let(:external_url) { 'http://example.com' }
before do
expect(ErrorTracking::ListIssuesService)
.to receive(:new).with(project, user)
.and_return(list_issues_service)
end
context 'service result is successful' do
before do
expect(list_issues_service).to receive(:execute)
.and_return(status: :success, issues: [error])
expect(list_issues_service).to receive(:external_url)
.and_return(external_url)
end
let(:error) { build(:error_tracking_error) }
it 'returns a list of errors' do
get :index, params: project_params(format: :json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('error_tracking/index')
expect(json_response['external_url']).to eq(external_url)
expect(json_response['errors']).to eq([error].as_json)
end
end
context 'service result is erroneous' do
let(:error_message) { 'error message' }
context 'without http_status' do
before do
expect(list_issues_service).to receive(:execute)
.and_return(status: :error, message: error_message)
end
it 'returns 400 with message' do
get :index, params: project_params(format: :json)
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).to eq(error_message)
end
end
context 'with explicit http_status' do
let(:http_status) { :no_content }
before do
expect(list_issues_service).to receive(:execute)
.and_return(status: :error, message: error_message, http_status: http_status)
end
it 'returns http_status with message' do
get :index, params: project_params(format: :json)
expect(response).to have_gitlab_http_status(http_status)
expect(json_response['message']).to eq(error_message)
end
end
end
end
end
private
def project_params(opts = {})
opts.reverse_merge(namespace_id: project.namespace, project_id: project)
end
end
# frozen_string_literal: true
FactoryBot.define do
factory :error_tracking_error, class: Gitlab::ErrorTracking::Error do
id 'id'
title 'title'
type 'error'
user_count 1
count 2
first_seen { Time.now }
last_seen { Time.now }
message 'message'
culprit 'culprit'
external_url 'http://example.com/id'
project_id 'project1'
project_name 'project name'
project_slug 'project_name'
short_id 'ID'
status 'unresolved'
frequency []
skip_create
end
end
{
"type": "object",
"required" : [
"external_url",
"last_seen",
"message",
"type"
],
"properties" : {
"id": { "type": "string"},
"first_seen": { "type": "string", "format": "date-time" },
"last_seen": { "type": "string", "format": "date-time" },
"type": { "type": "string" },
"message": { "type": "string" },
"culprit": { "type": "string" },
"count": { "type": "integer"},
"external_url": { "type": "string" },
"user_count": { "type": "integer"}
},
"additionalProperties": true
}
{
"type": "object",
"required": [
"external_url",
"errors"
],
"properties": {
"external_url": { "type": ["string", "null"] },
"errors": {
"type": "array",
"items": { "$ref": "error.json" }
}
},
"additionalProperties": false
}
...@@ -55,7 +55,7 @@ describe Sentry::Client do ...@@ -55,7 +55,7 @@ describe Sentry::Client do
context 'external_url' do context 'external_url' do
it 'is constructed correctly' do it 'is constructed correctly' do
expect(subject[0].external_url).to eq('https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/issues/11/') expect(subject[0].external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project/issues/11')
end end
end end
end end
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
describe ErrorTracking::ProjectErrorTrackingSetting do describe ErrorTracking::ProjectErrorTrackingSetting do
include ReactiveCachingHelpers
set(:project) { create(:project) } set(:project) { create(:project) }
subject { create(:project_error_tracking_setting, project: project) } subject { create(:project_error_tracking_setting, project: project) }
...@@ -48,10 +50,16 @@ describe ErrorTracking::ProjectErrorTrackingSetting do ...@@ -48,10 +50,16 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
describe '#sentry_external_url' do describe '#sentry_external_url' do
let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
it 'returns the correct url' do before do
subject.api_url = sentry_url subject.api_url = sentry_url
end
it 'returns the correct url' do
expect(subject.class).to receive(:extract_sentry_external_url).with(sentry_url).and_call_original
result = subject.sentry_external_url
expect(subject.sentry_external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project') expect(result).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project')
end end
end end
...@@ -60,4 +68,42 @@ describe ErrorTracking::ProjectErrorTrackingSetting do ...@@ -60,4 +68,42 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
expect(subject.sentry_client).to be_a(Sentry::Client) expect(subject.sentry_client).to be_a(Sentry::Client)
end end
end end
describe '#list_sentry_issues' do
let(:issues) { [:list, :of, :issues] }
let(:opts) do
{ issue_status: 'unresolved', limit: 10 }
end
let(:result) do
subject.list_sentry_issues(**opts)
end
context 'when cached' do
let(:sentry_client) { spy(:sentry_client) }
before do
stub_reactive_cache(subject, issues, opts)
synchronous_reactive_cache(subject)
expect(subject).to receive(:sentry_client).and_return(sentry_client)
end
it 'returns cached issues' do
expect(sentry_client).to receive(:list_issues).with(opts)
.and_return(issues)
expect(result).to eq(issues: issues)
end
end
context 'when not cached' do
it 'returns nil' do
expect(subject).not_to receive(:sentry_client)
expect(result).to be_nil
end
end
end
end end
...@@ -8,7 +8,7 @@ describe ErrorTracking::ListIssuesService do ...@@ -8,7 +8,7 @@ describe ErrorTracking::ListIssuesService do
let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
let(:token) { 'test-token' } let(:token) { 'test-token' }
let(:client) { Sentry::Client.new(sentry_url, token) } let(:result) { subject.execute }
let(:error_tracking_setting) do let(:error_tracking_setting) do
create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project)
...@@ -24,16 +24,29 @@ describe ErrorTracking::ListIssuesService do ...@@ -24,16 +24,29 @@ describe ErrorTracking::ListIssuesService do
describe '#execute' do describe '#execute' do
context 'with authorized user' do context 'with authorized user' do
context 'when list_sentry_issues returns issues' do
let(:issues) { [:list, :of, :issues] }
before do before do
expect(error_tracking_setting).to receive(:sentry_client).and_return(client) expect(error_tracking_setting)
.to receive(:list_sentry_issues).and_return(issues: issues)
end end
it 'calls sentry client' do it 'returns the issues' do
expect(client).to receive(:list_issues).and_return([]) expect(result).to eq(status: :success, issues: issues)
end
end
result = subject.execute context 'when list_sentry_issues returns nil' do
before do
expect(error_tracking_setting)
.to receive(:list_sentry_issues).and_return(nil)
end
expect(result).to include(status: :success) it 'result is not ready' do
expect(result).to eq(
status: :error, http_status: :no_content, message: 'not ready')
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