Skip to content

Commit 6fdc56e

Browse files
authored
Merge pull request #124 from Zoncovsky/feature/separate-local-and-prod-errors
Refactor deploys display and enhance GitHub integration settings, separate local errors and prod errors
2 parents 145aea8 + 6bc098e commit 6fdc56e

File tree

18 files changed

+308
-371
lines changed

18 files changed

+308
-371
lines changed

Gemfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ GEM
100100
bindex (0.8.1)
101101
bootsnap (1.18.6)
102102
msgpack (~> 1.2)
103-
brakeman (8.0.2)
103+
brakeman (8.0.4)
104104
racc
105105
builder (3.3.0)
106106
bullet (7.2.0)

app/controllers/api/v1/cli/incidents_controller.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,9 @@ def format_endpoint(issue)
150150

151151
def extract_backtrace(issue)
152152
recent_event = issue.events.order(occurred_at: :desc).first
153-
return [issue.top_frame] unless recent_event&.backtrace
153+
return [issue.top_frame].compact unless recent_event&.backtrace
154154

155-
# Return first 10 frames
156-
recent_event.backtrace.first(10)
155+
recent_event.formatted_backtrace.first(10)
157156
end
158157

159158
def extract_tags(issue)

app/controllers/api/v1/cli/traces_controller.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,17 +119,17 @@ def build_spans(project, endpoint, rollups)
119119
}
120120
end
121121

122-
# Check for N+1 issues
123122
n_plus_ones = project.sql_fingerprints
124123
.where("created_at > ?", 24.hours.ago)
125-
.where(severity: %w[high critical])
124+
.where("total_count > ?", 10)
125+
.order(total_count: :desc)
126126
.limit(3)
127127

128128
n_plus_ones.each do |fp|
129129
n1_time = fp.avg_duration_ms || 50
130130
n1_percent = [(n1_time.to_f / [avg_total, 1].max * 100).round, 50].min
131131
spans << {
132-
name: "N+1: #{fp.table_name || 'query'}",
132+
name: "N+1: #{fp.query_type || 'query'}",
133133
duration_ms: n1_time.round,
134134
percent: n1_percent
135135
}
@@ -143,20 +143,20 @@ def estimate_sql_time(project, endpoint)
143143
fingerprints = project.sql_fingerprints.where("created_at > ?", 24.hours.ago)
144144
return 0 if fingerprints.empty?
145145

146-
fingerprints.sum(:total_duration_ms).to_f / [fingerprints.sum(:call_count), 1].max
146+
fingerprints.sum(:total_duration_ms).to_f / [fingerprints.sum(:total_count), 1].max
147147
end
148148

149149
def identify_bottlenecks(project, endpoint, spans)
150150
bottlenecks = []
151151

152-
# Check for N+1 queries
153152
n_plus_ones = project.sql_fingerprints
154153
.where("created_at > ?", 24.hours.ago)
155-
.where(severity: %w[high critical])
154+
.where("total_count > ?", 10)
155+
.order(total_count: :desc)
156156

157157
if n_plus_ones.any?
158158
n_plus_ones.limit(2).each do |fp|
159-
bottlenecks << "N+1 query on #{fp.table_name || 'table'} (#{fp.call_count} calls)"
159+
bottlenecks << "N+1 query on #{fp.query_type || 'table'} (#{fp.total_count} calls)"
160160
end
161161
end
162162

app/controllers/deploys_controller.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ def index
88
@project_scope = @current_project || @project
99

1010
if @project_scope
11-
@deploys = @project_scope.deploys
12-
.includes(:release, :user)
13-
.recent
14-
.to_a
11+
@deploys = Deploy.includes(:project, :release, :user)
12+
.where(project_id: @project_scope.id, account_id: current_account&.id)
13+
.recent
14+
.limit(10)
15+
.to_a
1516

1617
@max_live_seconds =
1718
@deploys.each_with_index.map do |deploy, i|

app/controllers/errors_controller.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ def index
6060
@q = base_scope.ransack(params[:q])
6161
scoped_issues = if params[:q]&.dig(:s).present?
6262
@q.result.includes(:project)
63-
else
63+
else
6464
@q.result.includes(:project).severity_ordered
65-
end
65+
end
6666

6767
# Use pagy_countless to skip the expensive SELECT COUNT(*) on millions of rows.
6868
# Trade-off: we don't show "Page X of Y" or total count in pagination.
@@ -125,6 +125,7 @@ def index
125125
.where("controller_action NOT LIKE '%Controller#%'")
126126
.pluck(:id)
127127
.to_set
128+
128129
else
129130
@total_events_24h = 0
130131
@events_24h_by_issue_id = {}

app/helpers/application_helper.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ def show_sidebar?
1313
controller_path.include?("users")
1414
end
1515

16+
# Returns the URL only if it uses a safe protocol (http/https), nil otherwise.
17+
# Used in views to prevent Brakeman LinkToHref warnings when rendering
18+
# user-supplied URLs as link hrefs.
19+
def safe_url(url)
20+
return unless url.present? && url.match?(%r{\Ahttps?://}i)
21+
url
22+
end
23+
1624
# Unified helper for errors index path that respects project scoping (global, project_id, or slug)
1725
def errors_index_path(options = {})
1826
if defined?(@current_project) && @current_project

app/jobs/data_retention_job.rb

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ class DataRetentionJob < ApplicationJob
77
DEFAULT_RETENTION_DAYS = 31
88
FREE_PLAN_RETENTION_DAYS = 5
99
BATCH_SIZE = 50_000 # Larger batches since we have indexes on occurred_at
10+
PURGEABLE_TABLES = %w[events performance_events].freeze
1011

1112
# Delete events and performance events based on plan-specific retention.
1213
# Runs daily via Sidekiq Cron.
@@ -70,24 +71,20 @@ def delete_old_performance_events_for_accounts(cutoff_date, account_ids)
7071
delete_in_batches_for_accounts("performance_events", cutoff_date, account_ids)
7172
end
7273

73-
# Efficient batch deletion using raw SQL (global — all accounts)
7474
def delete_in_batches(table_name, cutoff_date)
75+
validate_table_name!(table_name)
7576
total_deleted = 0
7677
conn = ActiveRecord::Base.connection
77-
sanitized_table = conn.quote_table_name(table_name)
78-
sanitized_cutoff = conn.quote(cutoff_date.utc)
78+
quoted_table = conn.quote_table_name(table_name)
7979

8080
loop do
81-
sql = <<-SQL.squish
82-
DELETE FROM #{sanitized_table}
83-
WHERE ctid IN (
84-
SELECT ctid FROM #{sanitized_table}
85-
WHERE occurred_at < #{sanitized_cutoff}
86-
LIMIT #{BATCH_SIZE}
87-
)
88-
SQL
89-
90-
result = ActiveRecord::Base.connection.execute(sql)
81+
sql = ActiveRecord::Base.sanitize_sql_array([
82+
"DELETE FROM #{quoted_table} WHERE ctid IN (SELECT ctid FROM #{quoted_table} WHERE occurred_at < ? LIMIT ?)",
83+
cutoff_date.utc,
84+
BATCH_SIZE
85+
])
86+
87+
result = conn.execute(sql)
9188
deleted_count = result.cmd_tuples
9289
total_deleted += deleted_count
9390

@@ -100,27 +97,27 @@ def delete_in_batches(table_name, cutoff_date)
10097
total_deleted
10198
end
10299

103-
# Efficient batch deletion scoped to specific accounts (via project_id join)
104100
def delete_in_batches_for_accounts(table_name, cutoff_date, account_ids)
101+
validate_table_name!(table_name)
105102
total_deleted = 0
106103
conn = ActiveRecord::Base.connection
107-
sanitized_table = conn.quote_table_name(table_name)
108-
sanitized_cutoff = conn.quote(cutoff_date.utc)
109-
sanitized_ids = account_ids.map { |id| conn.quote(id) }.join(", ")
104+
quoted_table = conn.quote_table_name(table_name)
110105

111106
loop do
112-
sql = <<-SQL.squish
113-
DELETE FROM #{sanitized_table}
114-
WHERE ctid IN (
115-
SELECT #{sanitized_table}.ctid FROM #{sanitized_table}
116-
INNER JOIN projects ON projects.id = #{sanitized_table}.project_id
117-
WHERE #{sanitized_table}.occurred_at < #{sanitized_cutoff}
118-
AND projects.account_id IN (#{sanitized_ids})
119-
LIMIT #{BATCH_SIZE}
120-
)
121-
SQL
122-
123-
result = ActiveRecord::Base.connection.execute(sql)
107+
sql = ActiveRecord::Base.sanitize_sql_array([
108+
"DELETE FROM #{quoted_table} WHERE ctid IN (" \
109+
"SELECT #{quoted_table}.ctid FROM #{quoted_table} " \
110+
"INNER JOIN projects ON projects.id = #{quoted_table}.project_id " \
111+
"WHERE #{quoted_table}.occurred_at < ? " \
112+
"AND projects.account_id IN (?) " \
113+
"LIMIT ?" \
114+
")",
115+
cutoff_date.utc,
116+
account_ids,
117+
BATCH_SIZE
118+
])
119+
120+
result = conn.execute(sql)
124121
deleted_count = result.cmd_tuples
125122
total_deleted += deleted_count
126123

@@ -132,4 +129,8 @@ def delete_in_batches_for_accounts(table_name, cutoff_date, account_ids)
132129

133130
total_deleted
134131
end
132+
133+
def validate_table_name!(table_name)
134+
raise ArgumentError, "Unknown table: #{table_name}" unless PURGEABLE_TABLES.include?(table_name)
135+
end
135136
end

app/models/event.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,20 @@ def self.ingest_error(project:, payload:)
6060
event_context[:culprit_frame] = payload[:culprit_frame]
6161
end
6262

63+
# Link to Release and Deploy records when release_version is provided
64+
release = nil
65+
deploy = nil
66+
if payload[:release_version].present?
67+
release = project.releases.find_by(version: payload[:release_version])
68+
deploy = release ? project.deploys.where(release: release).order(started_at: :desc).first : nil
69+
end
70+
6371
# Create event (individual occurrence)
6472
event = create!(
6573
project: project,
6674
issue: issue,
75+
release: release,
76+
deploy_id: deploy&.id,
6777
exception_class: exception_class,
6878
message: message,
6979
backtrace: backtrace,

app/views/deploys/index.html.erb

Lines changed: 18 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,63 +24,36 @@
2424
</thead>
2525
<tbody class="bg-white divide-y divide-gray-200">
2626
<% if @deploys.any? %>
27-
<% @deploys.each_with_index do |deploy, index| %>
28-
<% next_deploy = index.positive? ? @deploys[index - 1] : nil %>
29-
<tr>
30-
<td class="px-6 py-4 whitespace-nowrap text-sm font-medium text-gray-900">
31-
<%= deploy.release.version&.slice(0, 7) %>
27+
<% @deploys.each_with_index do |deploy, i| %>
28+
<% next_deploy = i.positive? ? @deploys[i - 1] : nil %>
29+
<tr class="hover:bg-gray-50">
30+
<td class="px-6 py-4 font-mono text-sm text-indigo-600">
31+
<%= deploy.release&.version || "unknown" %>
3232
</td>
33-
<td class="px-6 py-4 whitespace-nowrap">
34-
<span class="inline-flex items-center px-2.5 py-0.5 rounded-full text-xs font-medium <%= deploy[:status] == 'success' ? 'bg-green-100 text-green-800' : 'bg-red-100 text-red-800' %>">
35-
<%= deploy.status.capitalize %>
33+
<td class="px-6 py-4">
34+
<% status = deploy.status || "completed" %>
35+
<span class="inline-flex items-center px-2.5 py-0.5 rounded-full text-xs font-medium
36+
<%= status == 'completed' ? 'bg-green-100 text-green-800' : status == 'failed' ? 'bg-red-100 text-red-800' : 'bg-yellow-100 text-yellow-800' %>">
37+
<%= status %>
3638
</span>
3739
</td>
38-
<td class="px-6 py-4 whitespace-nowrap text-sm text-gray-500">
39-
<div class="text-gray-900 font-medium">
40-
<%= live_for_human(deploy, next_deploy) %>
41-
</div>
42-
43-
<div class="h-1 bg-gray-200 rounded">
44-
<div
45-
class="h-1 bg-gray-400 rounded"
46-
style="width: <%= progress_percent(
47-
deploy.live_for_seconds(next_deploy),
48-
@max_live_seconds
49-
) %>%">
50-
</div>
51-
</div>
40+
<td class="px-6 py-4 text-sm text-gray-500 whitespace-nowrap">
41+
<%= distance_of_time_in_words(deploy.live_for_seconds(next_deploy)) %>
5242
</td>
53-
<td class="px-6 py-4 whitespace-nowrap text-sm text-gray-500">
54-
<%= deploy.started_at.strftime("%b %eth %H:%M") %>
43+
<td class="px-6 py-4 text-sm text-gray-500 whitespace-nowrap">
44+
<%= time_ago_in_words(deploy.started_at) %> ago
5545
</td>
56-
<td class="px-6 py-4 whitespace-nowrap text-sm font-medium">
57-
<div class="space-y-1">
58-
<div class="text-gray-900">
59-
<%= deploy.errors_count %>
60-
</div>
61-
<div class="h-1 bg-gray-200 rounded">
62-
<div
63-
class="h-1 bg-gray-400 rounded"
64-
style="width: <%= progress_percent(deploy.errors_count, @max_errors) %>%">
65-
</div>
66-
</div>
67-
</div>
46+
<td class="px-6 py-4 text-sm text-gray-500">
47+
<%= deploy.errors_count %>
6848
</td>
69-
<td class="px-6 py-4 whitespace-nowrap text-sm font-medium text-gray-900">
49+
<td class="px-6 py-4 text-sm text-gray-500">
7050
<%= deploy.errors_per_hour %>
71-
72-
<div class="h-1 bg-gray-200 rounded">
73-
<div
74-
class="h-1 bg-gray-400 rounded"
75-
style="width: <%= progress_percent(deploy.errors_per_hour, @max_errors_per_hour) %>%">
76-
</div>
77-
</div>
7851
</td>
7952
</tr>
8053
<% end %>
8154
<% else %>
8255
<tr>
83-
<td colspan="7" class="px-6 py-12 text-center">
56+
<td colspan="6" class="px-6 py-12 text-center">
8457
<div class="mx-auto">
8558
<div class="w-12 h-12 mx-auto mb-4 text-gray-400">
8659
🚢

app/views/errors/_status_dropdown_list.html.erb

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,7 @@
44
<button type="button"
55
data-action="click->dropdown#toggle"
66
onclick="event.stopPropagation();"
7-
class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-full text-xs font-medium transition-all duration-200 hover:shadow-md hover:scale-105 focus:outline-none focus:ring-2 focus:ring-offset-1
8-
<%= case issue.status
9-
when 'open' then 'bg-red-50 text-red-700 border border-red-200 focus:ring-red-400'
10-
when 'wip' then 'bg-amber-50 text-amber-700 border border-amber-200 focus:ring-amber-400'
11-
else 'bg-emerald-50 text-emerald-700 border border-emerald-200 focus:ring-emerald-400'
12-
end %>">
7+
class="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-full text-xs font-medium transition-all duration-200 hover:shadow-md hover:scale-105 focus:outline-none focus:ring-2 focus:ring-offset-1 bg-gray-50 text-gray-700 border border-gray-200 focus:ring-gray-400">
138
<span class="w-1.5 h-1.5 rounded-full <%= case issue.status
149
when 'open' then 'bg-red-500'
1510
when 'wip' then 'bg-amber-500'

0 commit comments

Comments
 (0)