Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Sentry integration #16

Merged
merged 2 commits into from
Aug 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/rspecq
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ OptionParser.new do |o|
BANNER

o.separator ""
o.separator "OPTIONS:"
o.separator "OPTIONS:"

o.on("-b", "--build ID", "A unique identifier for the build. Should be " \
"common among workers participating in the same build.") do |v|
Expand Down
1 change: 1 addition & 0 deletions lib/rspecq.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "rspec/core"
require "sentry-raven"

module RSpecQ
# If a worker haven't executed an example for more than WORKER_LIVENESS_SEC
Expand Down
61 changes: 45 additions & 16 deletions lib/rspecq/worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Worker

# Retry failed examples up to N times (with N being the supplied value)
# before considering them legit failures
#
#
# Defaults to 3
attr_accessor :max_requeues

Expand Down Expand Up @@ -102,10 +102,11 @@ def try_publish_queue!(queue)

timings = queue.timings
if timings.empty?
# TODO: should be a warning reported somewhere (Sentry?)
q_size = queue.publish(files_to_run.shuffle)
puts "WARNING: No timings found! Published queue in " \
"random order (size=#{q_size})"
log_event(
"No timings found! Published queue in random order (size=#{q_size})",
"warning"
)
return
end

Expand All @@ -114,7 +115,12 @@ def try_publish_queue!(queue)
end.map(&:first) & files_to_run

if slow_files.any?
puts "Slow files (threshold=#{file_split_threshold}): #{slow_files}"
log_event(
"Slow files detected (threshold=#{file_split_threshold}): #{slow_files}",
georgepsarakis marked this conversation as resolved.
Show resolved Hide resolved
"info",
slow_files: slow_files,
slow_files_count: slow_files.count,
)
end

# prepare jobs to run
Expand Down Expand Up @@ -163,20 +169,25 @@ def reset_rspec_state!
# Their errors will be reported in the normal flow, when they're picked up
# as jobs by a worker.
def files_to_example_ids(files)
# TODO: do this programatically
cmd = "DISABLE_SPRING=1 bundle exec rspec --dry-run --format json #{files.join(' ')}"
cmd = "DISABLE_SPRING=1 bundle exec rspec --dry-run --format json #{files.join(' ')} 2>&1"
out = `#{cmd}`
cmd_result = $?

if !$?.success?
# TODO: emit warning to Sentry
puts "WARNING: Error splitting slow files; falling back to regular scheduling:"
if !cmd_result.success?
rspec_output = begin
agis marked this conversation as resolved.
Show resolved Hide resolved
JSON.parse(out)
rescue JSON::ParserError
out
end

begin
pp JSON.parse(out)
rescue JSON::ParserError
puts out
end
puts
log_event(
"Failed to split slow files, falling back to regular scheduling",
"error",
rspec_output: rspec_output,
cmd_result: cmd_result.inspect,
)

pp rspec_output

return files
end
Expand All @@ -192,5 +203,23 @@ def relative_path(job)
def elapsed(since)
Process.clock_gettime(Process::CLOCK_MONOTONIC) - since
end

# Prints msg to standard output and emits an event to Sentry, if the
# SENTRY_DSN environment variable is set.
def log_event(msg, level, additional={})
puts msg
georgepsarakis marked this conversation as resolved.
Show resolved Hide resolved
georgepsarakis marked this conversation as resolved.
Show resolved Hide resolved

Raven.capture_message(msg, level: level, extra: {
build: @build_id,
worker: @worker_id,
queue: queue.inspect,
files_or_dirs_to_run: @files_or_dirs_to_run,
populate_timings: populate_timings,
file_split_threshold: file_split_threshold,
heartbeat_updated_at: @heartbeat_updated_at,
object: self.inspect,
pid: Process.pid,
}.merge(additional))
end
end
end
1 change: 1 addition & 0 deletions rspecq.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Gem::Specification.new do |s|
end

s.add_dependency "redis"
s.add_dependency "sentry-raven"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note that although raven-ruby has only one dependency at the moment as far as I can tell, this could create unnecessary issues with gem incompatibilities when the integration is not used. So perhaps in the future the whole integration along with dependencies can become opt-in?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could do this by not adding sentry-raven as a dependency and requiring the user to explicitly add it to their Gemfile. That said, I believe we should hold on until this pops up as an issue.


s.add_development_dependency "rake"
s.add_development_dependency "pry-byebug"
Expand Down