Commit 81987dee authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch 'dz-error-tracking-integrated-client' into 'master'

Expose integrated error tracking to UI and services

See merge request gitlab-org/gitlab!66847
parents 17ca886f 08a65298
# frozen_string_literal: true
module ErrorTracking
class ErrorsFinder
def initialize(current_user, project, params)
@current_user = current_user
@project = project
@params = params
end
def execute
return ErrorTracking::Error.none unless authorized?
collection = project.error_tracking_errors
collection = by_status(collection)
# Limit collection until pagination implemented
collection.limit(20)
end
private
attr_reader :current_user, :project, :params
def by_status(collection)
if params[:status].present? && ErrorTracking::Error.statuses.key?(params[:status])
collection.for_status(params[:status])
else
collection
end
end
def authorized?
Ability.allowed?(current_user, :read_sentry_issue, project)
end
end
end
......@@ -5,10 +5,19 @@ class ErrorTracking::Error < ApplicationRecord
has_many :events, class_name: 'ErrorTracking::ErrorEvent'
scope :for_status, -> (status) { where(status: status) }
validates :project, presence: true
validates :name, presence: true
validates :description, presence: true
validates :actor, presence: true
validates :status, presence: true
enum status: {
unresolved: 0,
resolved: 1,
ignored: 2
}
def self.report_error(name:, description:, actor:, platform:, timestamp:)
safe_find_or_create_by(
......@@ -20,4 +29,64 @@ class ErrorTracking::Error < ApplicationRecord
error.update!(last_seen_at: timestamp)
end
end
def title
if description.present?
"#{name} #{description}"
else
name
end
end
def title_truncated
title.truncate(64)
end
# For compatibility with sentry integration
def to_sentry_error
Gitlab::ErrorTracking::Error.new(
id: id,
title: title_truncated,
message: description,
culprit: actor,
first_seen: first_seen_at,
last_seen: last_seen_at,
status: status,
count: events_count
)
end
# For compatibility with sentry integration
def to_sentry_detailed_error
Gitlab::ErrorTracking::DetailedError.new(
id: id,
title: title_truncated,
message: description,
culprit: actor,
first_seen: first_seen_at.to_s,
last_seen: last_seen_at.to_s,
count: events_count,
user_count: 0, # we don't support user count yet.
project_id: project.id,
status: status,
tags: { level: nil, logger: nil },
external_url: external_url,
external_base_url: external_base_url
)
end
private
# For compatibility with sentry integration
def external_url
Gitlab::Routing.url_helpers.details_namespace_project_error_tracking_index_url(
namespace_id: project.namespace,
project_id: project,
issue_id: id)
end
# For compatibility with sentry integration
def external_base_url
Gitlab::Routing.url_helpers.root_url
end
end
......@@ -8,4 +8,69 @@ class ErrorTracking::ErrorEvent < ApplicationRecord
validates :error, presence: true
validates :description, presence: true
validates :occurred_at, presence: true
def stacktrace
@stacktrace ||= build_stacktrace
end
# For compatibility with sentry integration
def to_sentry_error_event
Gitlab::ErrorTracking::ErrorEvent.new(
issue_id: error_id,
date_received: occurred_at,
stack_trace_entries: stacktrace
)
end
private
def build_stacktrace
raw_stacktrace = find_stacktrace_from_payload
return [] unless raw_stacktrace
raw_stacktrace.map do |entry|
{
'lineNo' => entry['lineno'],
'context' => build_stacktrace_context(entry),
'filename' => entry['filename'],
'function' => entry['function'],
'colNo' => 0 # we don't support colNo yet.
}
end
end
def find_stacktrace_from_payload
exception_entry = payload.dig('exception')
if exception_entry
exception_values = exception_entry.dig('values')
stack_trace_entry = exception_values&.detect { |h| h['stacktrace'].present? }
stack_trace_entry&.dig('stacktrace', 'frames')
end
end
def build_stacktrace_context(entry)
context = []
error_line = entry['context_line']
error_line_no = entry['lineno']
pre_context = entry['pre_context']
post_context = entry['post_context']
context += lines_with_position(pre_context, error_line_no - pre_context.size)
context += lines_with_position([error_line], error_line_no)
context += lines_with_position(post_context, error_line_no + 1)
context.reject(&:blank?)
end
def lines_with_position(lines, position)
return [] if lines.blank?
lines.map.with_index do |line, index|
next unless line
[position + index, line]
end
end
end
......@@ -31,12 +31,13 @@ module ErrorTracking
validates :api_url, length: { maximum: 255 }, public_url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true
validates :enabled, inclusion: { in: [true, false] }
validates :integrated, inclusion: { in: [true, false] }
validates :api_url, presence: { message: 'is a required field' }, if: :enabled
validate :validate_api_url_path, if: :enabled
validates :token, presence: { message: 'is a required field' }, if: :enabled
with_options if: :sentry_enabled do
validates :api_url, presence: { message: 'is a required field' }
validates :token, presence: { message: 'is a required field' }
validate :validate_api_url_path
end
attr_encrypted :token,
mode: :per_attribute_iv,
......@@ -45,6 +46,14 @@ module ErrorTracking
after_save :clear_reactive_cache!
def sentry_enabled
enabled && !integrated_client?
end
def integrated_client?
integrated && ::Feature.enabled?(:integrated_error_tracking, project)
end
def api_url=(value)
super
clear_memoization(:api_url_slugs)
......@@ -79,7 +88,7 @@ module ErrorTracking
def sentry_client
strong_memoize(:sentry_client) do
ErrorTracking::SentryClient.new(api_url, token)
::ErrorTracking::SentryClient.new(api_url, token)
end
end
......
......@@ -8,7 +8,7 @@ module ErrorTracking
private
def perform
response = project_error_tracking_setting.issue_details(issue_id: params[:issue_id])
response = find_issue_details(params[:issue_id])
compose_response(response) do
# The gitlab_issue attribute can contain an absolute GitLab url from the Sentry Client
......@@ -36,5 +36,29 @@ module ErrorTracking
def parse_response(response)
{ issue: response[:issue] }
end
def find_issue_details(issue_id)
# There are 2 types of the data source for the error tracking feature:
#
# * When integrated error tracking is enabled, we use the application database
# to read and save error tracking data.
#
# * When integrated error tracking is disabled we call
# project_error_tracking_setting method which works with Sentry API.
#
# Issue https://gitlab.com/gitlab-org/gitlab/-/issues/329596
#
if project_error_tracking_setting.integrated_client?
error = project.error_tracking_errors.find(issue_id)
# We use the same response format as project_error_tracking_setting
# method below for compatibility with existing code.
{
issue: error.to_sentry_detailed_error
}
else
project_error_tracking_setting.issue_details(issue_id: issue_id)
end
end
end
end
......@@ -5,7 +5,7 @@ module ErrorTracking
private
def perform
response = project_error_tracking_setting.issue_latest_event(issue_id: params[:issue_id])
response = find_issue_latest_event(params[:issue_id])
compose_response(response)
end
......@@ -13,5 +13,30 @@ module ErrorTracking
def parse_response(response)
{ latest_event: response[:latest_event] }
end
def find_issue_latest_event(issue_id)
# There are 2 types of the data source for the error tracking feature:
#
# * When integrated error tracking is enabled, we use the application database
# to read and save error tracking data.
#
# * When integrated error tracking is disabled we call
# project_error_tracking_setting method which works with Sentry API.
#
# Issue https://gitlab.com/gitlab-org/gitlab/-/issues/329596
#
if project_error_tracking_setting.integrated_client?
error = project.error_tracking_errors.find(issue_id)
event = error.events.last
# We use the same response format as project_error_tracking_setting
# method below for compatibility with existing code.
{
latest_event: event.to_sentry_error_event
}
else
project_error_tracking_setting.issue_latest_event(issue_id: issue_id)
end
end
end
end
......@@ -5,10 +5,12 @@ module ErrorTracking
private
def perform
response = project_error_tracking_setting.update_issue(
update_opts = {
issue_id: params[:issue_id],
params: update_params
)
}
response = update_issue(update_opts)
compose_response(response) do
project_error_tracking_setting.expire_issues_cache
......@@ -69,5 +71,31 @@ module ErrorTracking
return error('Error Tracking is not enabled') unless enabled?
return error('Access denied', :unauthorized) unless can_update?
end
def update_issue(opts)
# There are 2 types of the data source for the error tracking feature:
#
# * When integrated error tracking is enabled, we use the application database
# to read and save error tracking data.
#
# * When integrated error tracking is disabled we call
# project_error_tracking_setting method which works with Sentry API.
#
# Issue https://gitlab.com/gitlab-org/gitlab/-/issues/329596
#
if project_error_tracking_setting.integrated_client?
error = project.error_tracking_errors.find(opts[:issue_id])
error.status = opts[:params][:status]
error.save!
# We use the same response format as project_error_tracking_setting
# method below for compatibility with existing code.
{
updated: true
}
else
project_error_tracking_setting.update_issue(**opts)
end
end
end
end
......@@ -22,13 +22,15 @@ module ErrorTracking
def perform
return invalid_status_error unless valid_status?
response = project_error_tracking_setting.list_sentry_issues(
sentry_opts = {
issue_status: issue_status,
limit: limit,
search_term: params[:search_term].presence,
sort: sort,
cursor: params[:cursor].presence
)
}
response = list_issues(sentry_opts)
compose_response(response)
end
......@@ -56,5 +58,36 @@ module ErrorTracking
def sort
params[:sort] || DEFAULT_SORT
end
def list_issues(opts)
# There are 2 types of the data source for the error tracking feature:
#
# * When integrated error tracking is enabled, we use the application database
# to read and save error tracking data.
#
# * When integrated error tracking is disabled we call
# project_error_tracking_setting method which works with Sentry API.
#
# Issue https://gitlab.com/gitlab-org/gitlab/-/issues/329596
#
if project_error_tracking_setting.integrated_client?
# We are going to support more options in the future.
# For now we implement the bare minimum for rendering the list in UI.
filter_opts = {
status: opts[:issue_status]
}
errors = ErrorTracking::ErrorsFinder.new(current_user, project, filter_opts).execute
# We use the same response format as project_error_tracking_setting
# method below for compatibility with existing code.
{
issues: errors.map(&:to_sentry_error),
pagination: {}
}
else
project_error_tracking_setting.list_sentry_issues(**opts)
end
end
end
end
# frozen_string_literal: true
class Gitlab::Seeder::ErrorTrackingSeeder
attr_reader :project
def initialize(project)
@project = project
end
def seed
parsed_event = Gitlab::Json.parse(read_fixture_file('parsed_event.json'))
ErrorTracking::CollectErrorService
.new(project, nil, event: parsed_event)
.execute
end
private
def read_fixture_file(file)
File.read(fixture_path(file))
end
def fixture_path(file)
Rails.root.join('spec', 'fixtures', 'error_tracking', file)
end
end
Gitlab::Seeder.quiet do
admin_user = User.admins.first
Project.not_mass_generated.visible_to_user(admin_user).sample(1).each do |project|
puts "\nActivating integrated error tracking for the '#{project.full_path}' project"
unless Feature.enabled?(:integrated_error_tracking, project)
puts '- enabling feature flag'
Feature.enable(:integrated_error_tracking, project)
end
puts '- enabling in settings'
project.error_tracking_setting || project.create_error_tracking_setting
project.error_tracking_setting.update!(enabled: true, integrated: true)
puts '- seeding an error'
seeder = Gitlab::Seeder::ErrorTrackingSeeder.new(project)
seeder.seed
end
end
# frozen_string_literal: true
class AddIntegratedToErrorTrackingSetting < ActiveRecord::Migration[6.1]
def up
add_column :project_error_tracking_settings, :integrated, :boolean, null: false, default: false
end
def down
remove_column :project_error_tracking_settings, :integrated
end
end
# frozen_string_literal: true
class AddStatusToErrorTrackingError < ActiveRecord::Migration[6.1]
def up
add_column :error_tracking_errors, :status, :integer, null: false, default: 0, limit: 2
end
def down
remove_column :error_tracking_errors, :status
end
end
d989534193566d90f1d4d61a0a588f3204670b67e049e875011a06b32ffd941a
\ No newline at end of file
8c317e202b9fb5fc3733325fd2447f65283c3752fcb314033f5d3b2b28484f71
\ No newline at end of file
......@@ -12922,6 +12922,7 @@ CREATE TABLE error_tracking_errors (
created_at timestamp with time zone NOT NULL,
updated_at timestamp with time zone NOT NULL,
events_count bigint DEFAULT 0 NOT NULL,
status smallint DEFAULT 0 NOT NULL,
CONSTRAINT check_18a758e537 CHECK ((char_length(name) <= 255)),
CONSTRAINT check_b5cb4d3888 CHECK ((char_length(actor) <= 255)),
CONSTRAINT check_c739788b12 CHECK ((char_length(description) <= 1024)),
......@@ -17005,7 +17006,8 @@ CREATE TABLE project_error_tracking_settings (
encrypted_token character varying,
encrypted_token_iv character varying,
project_name character varying,
organization_name character varying
organization_name character varying,
integrated boolean DEFAULT false NOT NULL
);
CREATE TABLE project_export_jobs (
......@@ -294,6 +294,7 @@ excluded_attributes:
- :encrypted_token
- :encrypted_token_iv
- :enabled
- :integrated
service_desk_setting:
- :outgoing_name
priorities:
......
......@@ -35,5 +35,10 @@ FactoryBot.define do
platform { 'ruby' }
first_seen_at { Time.now.iso8601 }
last_seen_at { Time.now.iso8601 }
status { 'unresolved' }
trait :resolved do
status { 'resolved' }
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ErrorTracking::ErrorsFinder do
let_it_be(:project) { create(:project) }
let_it_be(:user) { project.creator }
let_it_be(:error) { create(:error_tracking_error, project: project) }
let_it_be(:error_resolved) { create(:error_tracking_error, :resolved, project: project) }
before do
project.add_maintainer(user)
end
describe '#execute' do
let(:params) { {} }
subject { described_class.new(user, project, params).execute }
it { is_expected.to contain_exactly(error, error_resolved) }
context 'with status parameter' do
let(:params) { { status: 'resolved' } }
it { is_expected.to contain_exactly(error_resolved) }
end
end
end
......@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe ErrorTracking::ErrorEvent, type: :model do
let_it_be(:event) { create(:error_tracking_error_event) }
describe 'relationships' do
it { is_expected.to belong_to(:error) }
end
......@@ -11,4 +13,33 @@ RSpec.describe ErrorTracking::ErrorEvent, type: :model do
it { is_expected.to validate_presence_of(:description) }
it { is_expected.to validate_presence_of(:occurred_at) }
end
describe '#stacktrace' do
it 'generates a correct stacktrace in expected format' do
expected_context = [
[132, " end\n"],
[133, "\n"],
[134, " begin\n"],
[135, " block.call(work, *extra)\n"],
[136, " rescue Exception => e\n"],
[137, " STDERR.puts \"Error reached top of thread-pool: #\{e.message\} (#\{e.class\})\"\n"],
[138, " end\n"]
]
expected_entry = {
'lineNo' => 135,
'context' => expected_context,
'filename' => 'puma/thread_pool.rb',
'function' => 'block in spawn_thread',
'colNo' => 0
}
expect(event.stacktrace).to be_kind_of(Array)
expect(event.stacktrace.first).to eq(expected_entry)
end
end
describe '#to_sentry_error_event' do
it { expect(event.to_sentry_error_event).to be_kind_of(Gitlab::ErrorTracking::ErrorEvent) }
end
end
......@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe ErrorTracking::Error, type: :model do
let_it_be(:error) { create(:error_tracking_error) }
describe 'relationships' do
it { is_expected.to belong_to(:project) }
it { is_expected.to have_many(:events) }
......@@ -13,4 +15,16 @@ RSpec.describe ErrorTracking::Error, type: :model do
it { is_expected.to validate_presence_of(:description) }
it { is_expected.to validate_presence_of(:actor) }
end
describe '#title' do
it { expect(error.title).to eq('ActionView::MissingTemplate Missing template posts/edit') }
end
describe '#to_sentry_error' do
it { expect(error.to_sentry_error).to be_kind_of(Gitlab::ErrorTracking::Error) }
end
describe '#to_sentry_detailed_error' do
it { expect(error.to_sentry_detailed_error).to be_kind_of(Gitlab::ErrorTracking::DetailedError) }
end
end
......@@ -54,20 +54,22 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do
valid_api_url = 'http://example.com/api/0/projects/org-slug/proj-slug/'
valid_token = 'token'
where(:enabled, :token, :api_url, :valid?) do
true | nil | nil | false
true | nil | valid_api_url | false
true | valid_token | nil | false
true | valid_token | valid_api_url | true
false | nil | nil | true
false | nil | valid_api_url | true
false | valid_token | nil | true
false | valid_token | valid_api_url | true
where(:enabled, :integrated, :token, :api_url, :valid?) do
true | true | nil | nil | true
true | false | nil | nil | false
true | false | nil | valid_api_url | false
true | false | valid_token | nil | false
true | false | valid_token | valid_api_url | true
false | false | nil | nil | true
false | false | nil | valid_api_url | true
false | false | valid_token | nil | true
false | false | valid_token | valid_api_url | true
end
with_them do
before do
subject.enabled = enabled
subject.integrated = integrated
subject.token = token
subject.api_url = api_url
end
......@@ -472,4 +474,25 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do
expect(subject.list_sentry_issues(params)).to eq(nil)
end
end
describe '#sentry_enabled' do
using RSpec::Parameterized::TableSyntax
where(:enabled, :integrated, :feature_flag, :sentry_enabled) do
true | false | false | true
true | true | false | true
true | true | true | false
false | false | false | false
end
with_them do
before do
subject.enabled = enabled
subject.integrated = integrated
stub_feature_flags(integrated_error_tracking: feature_flag)
end
it { expect(subject.sentry_enabled).to eq(sentry_enabled) }
end
end
end
......@@ -39,6 +39,21 @@ RSpec.describe ErrorTracking::IssueDetailsService do
include_examples 'error tracking service data not ready', :issue_details
include_examples 'error tracking service sentry error handling', :issue_details
include_examples 'error tracking service http status handling', :issue_details
context 'integrated error tracking' do
let_it_be(:error) { create(:error_tracking_error, project: project) }
let(:params) { { issue_id: error.id } }
before do
error_tracking_setting.update!(integrated: true)
end
it 'returns the error in detailed format' do
expect(result[:status]).to eq(:success)
expect(result[:issue].to_json).to eq(error.to_sentry_detailed_error.to_json)
end
end
end
include_examples 'error tracking service unauthorized user'
......
......@@ -5,7 +5,9 @@ require 'spec_helper'
RSpec.describe ErrorTracking::IssueLatestEventService do
include_context 'sentry error tracking context'
subject { described_class.new(project, user) }
let(:params) { {} }
subject { described_class.new(project, user, params) }
describe '#execute' do
context 'with authorized user' do
......@@ -25,6 +27,22 @@ RSpec.describe ErrorTracking::IssueLatestEventService do
include_examples 'error tracking service data not ready', :issue_latest_event
include_examples 'error tracking service sentry error handling', :issue_latest_event
include_examples 'error tracking service http status handling', :issue_latest_event
context 'integrated error tracking' do
let_it_be(:error) { create(:error_tracking_error, project: project) }
let_it_be(:event) { create(:error_tracking_error_event, error: error) }
let(:params) { { issue_id: error.id } }
before do
error_tracking_setting.update!(integrated: true)
end
it 'returns the latest event in expected format' do
expect(result[:status]).to eq(:success)
expect(result[:latest_event].to_json).to eq(event.to_sentry_error_event.to_json)
end
end
end
include_examples 'error tracking service unauthorized user'
......
......@@ -114,6 +114,21 @@ RSpec.describe ErrorTracking::IssueUpdateService do
end
include_examples 'error tracking service sentry error handling', :update_issue
context 'integrated error tracking' do
let(:error) { create(:error_tracking_error, project: project) }
let(:arguments) { { issue_id: error.id, status: 'resolved' } }
let(:update_issue_response) { { updated: true, status: :success, closed_issue_iid: nil } }
before do
error_tracking_setting.update!(integrated: true)
end
it 'resolves the error and responds with expected format' do
expect(update_service.execute).to eq(update_issue_response)
expect(error.reload.status).to eq('resolved')
end
end
end
include_examples 'error tracking service unauthorized user'
......
......@@ -52,6 +52,20 @@ RSpec.describe ErrorTracking::ListIssuesService do
include_examples 'error tracking service unauthorized user'
include_examples 'error tracking service disabled'
context 'integrated error tracking' do
let_it_be(:error) { create(:error_tracking_error, project: project) }
before do
error_tracking_setting.update!(integrated: true)
end
it 'returns the error in expected format' do
expect(result[:status]).to eq(:success)
expect(result[:issues].size).to eq(1)
expect(result[:issues].first.to_json).to eq(error.to_sentry_error.to_json)
end
end
end
describe '#external_url' 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