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

Migrate from sentry-raven to sentry-ruby #8818

Merged
merged 9 commits into from
Jan 23, 2024
2 changes: 1 addition & 1 deletion common/lib/dependabot/shared_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def initialize(message:, error_context:, error_class: nil, trace: nil)
end

sig { returns(T::Hash[Symbol, T.untyped]) }
def raven_context
def sentry_context
{ fingerprint: [@fingerprint], extra: @error_context.except(:stderr_output, :fingerprint) }
end
end
Expand Down
2 changes: 1 addition & 1 deletion npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def initialize(message:, error_context:)
@error_context = error_context
end

def raven_context
def sentry_context
{ extra: @error_context }
end
end
Expand Down
23 changes: 0 additions & 23 deletions sorbet/rbi/shims/sentry-raven.rbi

This file was deleted.

55 changes: 55 additions & 0 deletions sorbet/rbi/shims/sentry-ruby.rbi
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# typed: strong
# frozen_string_literal: true

module Sentry
Copy link
Contributor Author

@JamieMagee JamieMagee Jan 18, 2024

Choose a reason for hiding this comment

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

tapioca gem sentry-ruby generates an empty file, so this is minimal handwritten types for our uses.

class << self
sig { params(_blk: T.proc.params(arg0: Sentry::Configuration).void).void }
def init(&_blk); end

sig { params(exception: Exception, options: T.untyped).void }
def capture_exception(exception, **options); end
end

class Configuration
sig { returns(T.nilable(::Logger)) }
attr_accessor :logger

sig { returns(T.nilable(String)) }
attr_accessor :project_root

sig { returns(T.nilable(::Regexp)) }
attr_accessor :app_dirs_pattern

sig { returns(T::Boolean) }
attr_accessor :propagate_traces

sig do
params(
value: T.proc
.params(
event: ::Sentry::Event,
hint: T::Hash[Symbol, T.untyped]
)
.returns(::Sentry::Event)
).void
end
def before_send=(value); end
end

class Event; end

class ErrorEvent < ::Sentry::Event
sig { returns(::Sentry::ExceptionInterface) }
attr_reader :exception
end

class ExceptionInterface
sig { returns(T::Array[::Sentry::SingleExceptionInterface]) }
attr_reader :values
end

class SingleExceptionInterface
sig { returns(String) }
attr_accessor :value
end
end
2 changes: 0 additions & 2 deletions sorbet/rbi/todo.rbi
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
# typed: false

module ::Azure::Error::NotFound; end
module ::Raven; end
class ::Raven::Processor; end
module Bundler::CompactIndexClient::Updater; end
module Bundler::SolveFailure; end
module Dependabot::NpmAndYarn::FileFetcher::Pysch::SyntaxError; end
Expand Down
2 changes: 1 addition & 1 deletion updater/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ gem "opentelemetry-instrumentation-faraday", "~> 0.23"
gem "opentelemetry-instrumentation-http", "~> 0.23"
gem "opentelemetry-instrumentation-net_http", "~> 0.22"
gem "opentelemetry-sdk", "~> 1.3"
gem "sentry-raven", "~> 3.1"
gem "sentry-ruby", "~> 5.1"
gem "terminal-table", "~> 3.0.2"

group :test do
Expand Down
7 changes: 4 additions & 3 deletions updater/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ GEM
base64 (0.1.1)
citrus (3.0.2)
commonmarker (0.23.10)
concurrent-ruby (1.2.3)
crack (0.4.5)
rexml
debug (1.8.0)
Expand Down Expand Up @@ -321,8 +322,8 @@ GEM
sawyer (0.9.2)
addressable (>= 2.3.5)
faraday (>= 0.17.3, < 3)
sentry-raven (3.1.2)
faraday (>= 1.0)
sentry-ruby (5.16.1)
concurrent-ruby (~> 1.0, >= 1.0.2)
sorbet-runtime (0.5.11193)
stackprof (0.2.25)
stringio (3.0.8)
Expand Down Expand Up @@ -384,7 +385,7 @@ DEPENDENCIES
rubocop (~> 1.58.0)
rubocop-performance (~> 1.19.0)
rubocop-sorbet (~> 0.7.3)
sentry-raven (~> 3.1)
sentry-ruby (~> 5.1)
stackprof (~> 0.2.16)
terminal-table (~> 3.0.2)
turbo_tests (~> 2.2.0)
Expand Down
10 changes: 6 additions & 4 deletions updater/bin/fetch_files.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
# typed: false
# typed: strict
# frozen_string_literal: true

$LOAD_PATH.unshift(__dir__ + "/../lib")
require "sorbet-runtime"

$LOAD_PATH.unshift(T.must(__dir__) + "/../lib")

$stdout.sync = true

require "raven"
require "sentry-ruby"
require "dependabot/setup"
require "dependabot/file_fetcher_command"
require "debug" if ENV["DEBUG"]
Expand All @@ -16,7 +18,7 @@ class UpdaterKilledError < StandardError; end
puts "Received SIGTERM"
error = UpdaterKilledError.new("Updater process killed with SIGTERM")
tags = { "gh.dependabot_api.update_job.id": ENV.fetch("DEPENDABOT_JOB_ID", nil) }
Raven.capture_exception(error, tags: tags)
Sentry.capture_exception(error, tags: tags)
exit
end

Expand Down
4 changes: 2 additions & 2 deletions updater/bin/update_files.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

$stdout.sync = true

require "raven"
require "sentry-ruby"
require "dependabot/setup"
require "dependabot/update_files_command"
require "debug" if ENV["DEBUG"]
Expand All @@ -16,7 +16,7 @@ class UpdaterKilledError < StandardError; end
puts "Received SIGTERM"
error = UpdaterKilledError.new("Updater process killed with SIGTERM")
tags = { "gh.dependabot_api.update_job.id": ENV.fetch("DEPENDABOT_JOB_ID", nil) }
Raven.capture_exception(error, tags: tags)
Sentry.capture_exception(error, tags: tags)
exit
end

Expand Down
1 change: 0 additions & 1 deletion updater/lib/dependabot/base_command.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# typed: true
# frozen_string_literal: true

require "raven"
require "dependabot/api_client"
require "dependabot/service"
require "dependabot/logger"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# typed: strong
# frozen_string_literal: true

require "raven"
require "sentry-ruby"
require "sorbet-runtime"
require "dependabot/sentry/processor"

# ExceptionSanitizer filters potential secrets/PII from exception payloads
class ExceptionSanitizer < Raven::Processor
class ExceptionSanitizer < ::Dependabot::Sentry::Processor
extend T::Sig

REPO = %r{[\w.\-]+/([\w.\-]+)}
Expand All @@ -18,21 +19,24 @@ class ExceptionSanitizer < Raven::Processor
)

sig do
params(data: T::Hash[Symbol, T.nilable(T::Hash[Symbol, T::Array[T::Hash[Symbol, String]]])])
.returns(T::Hash[Symbol, T.untyped])
override
.params(
event: ::Sentry::Event,
_hint: T::Hash[Symbol, T.untyped]
)
.returns(::Sentry::Event)
end
def process(data)
return data unless data.dig(:exception, :values)
def process(event, _hint)
return event unless event.is_a?(::Sentry::ErrorEvent)

T.must(data[:exception])[:values] = T.must(data.dig(:exception, :values)).map do |e|
event.exception.values.each do |e|
PATTERNS.each do |key, regex|
e[:value] = T.must(e[:value]).gsub(regex) do |match|
e.value = e.value.gsub(regex) do |match|
match.sub(/#{T.must(Regexp.last_match).captures.compact.first}\z/, "[FILTERED_#{key.to_s.upcase}]")
end
end
e
end

data
event
end
end
35 changes: 35 additions & 0 deletions updater/lib/dependabot/sentry/processor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# typed: strong
# frozen_string_literal: true

require "sorbet-runtime"

module Dependabot
module Sentry
class Processor
extend T::Sig
extend T::Helpers

abstract!

# Process an event before it is sent to Sentry
sig do
abstract
.params(
event: ::Sentry::Event,
hint: T::Hash[Symbol, T.untyped]
)
.returns(::Sentry::Event)
end
def process(event, hint); end

# The default processor chain.
# This chain is applied in the order of the array.
sig { params(event: ::Sentry::Event, hint: T::Hash[Symbol, T.untyped]).returns(::Sentry::Event) }
def self.process_chain(event, hint)
[ExceptionSanitizer, SentryContext].reduce(event) do |acc, processor|
processor.new.process(acc, hint)
end
end
end
end
end
25 changes: 25 additions & 0 deletions updater/lib/dependabot/sentry/sentry_context_processor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# typed: strict
# frozen_string_literal: true

require "sorbet-runtime"

require "dependabot/sentry/processor"

class SentryContext < ::Dependabot::Sentry::Processor
sig do
override
.params(
event: Sentry::Event,
hint: T::Hash[Symbol, T.untyped]
)
.returns(Sentry::Event)
end
def process(event, hint)
if (exception = hint[:exception])
exception.raven_context.each do |key, value|
event.send("#{key}=", value)
end
end
Comment on lines +18 to +22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

event
end
end
33 changes: 16 additions & 17 deletions updater/lib/dependabot/service.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# typed: strict
# frozen_string_literal: true

require "raven"
require "sentry-ruby"
require "sorbet-runtime"
require "terminal-table"

require "dependabot/api_client"
require "dependabot/opentelemetry"
require "sorbet-runtime"

# This class provides an output adapter for the Dependabot Service which manages
# communication with the private API as well as consolidated error handling.
Expand Down Expand Up @@ -83,7 +84,7 @@ def update_dependency_list(dependency_snapshot:)
client.update_dependency_list(dependency_payload, dependency_file_paths)
end

# This method wraps the Raven client as the Application error tracker
# This method wraps the Sentry client as the Application error tracker
# the service uses to notice errors.
#
# This should be called as an alternative/in addition to record_update_job_error
Expand All @@ -100,21 +101,19 @@ def update_dependency_list(dependency_snapshot:)
end
def capture_exception(error:, job: nil, dependency: nil, dependency_group: nil, tags: {}, extra: {})
::Dependabot::OpenTelemetry.record_exception(error: error, job: job, tags: tags)
T.unsafe(Raven).capture_exception(
::Sentry.capture_exception(
error,
{
tags: tags.merge({
"gh.dependabot_api.update_job.id": job&.id,
"gh.dependabot_api.update_config.package_manager": job&.package_manager,
"gh.repo.is_private": job&.repo_private?
}.compact),
extra: extra.merge({
dependency_name: dependency&.name,
dependency_group: dependency_group&.name
}.compact),
user: {
id: job&.repo_owner
}
tags: tags.merge({
"gh.dependabot_api.update_job.id": job&.id,
"gh.dependabot_api.update_config.package_manager": job&.package_manager,
"gh.repo.is_private": job&.repo_private?
}.compact),
extra: extra.merge({
dependency_name: dependency&.name,
dependency_group: dependency_group&.name
}.compact),
user: {
id: job&.repo_owner
Comment on lines +106 to +116
Copy link
Contributor Author

Choose a reason for hiding this comment

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

}
)
end
Expand Down
11 changes: 7 additions & 4 deletions updater/lib/dependabot/setup.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
# typed: strict
# frozen_string_literal: true

require "sentry-ruby"

require "dependabot/environment"
require "dependabot/logger"
require "dependabot/logger/formats"
require "dependabot/environment"
require "dependabot/sentry/processor"

Dependabot.logger = Logger.new($stdout).tap do |logger|
logger.level = Dependabot::Environment.log_level
logger.formatter = Dependabot::Logger::BasicFormatter.new
end

require "dependabot/sentry"
Raven.configure do |config|
Sentry.init do |config|
config.logger = Dependabot.logger
config.project_root = File.expand_path("../../..", __dir__)

Expand Down Expand Up @@ -40,7 +42,8 @@
devcontainers
)}x

config.processors += [ExceptionSanitizer]
config.before_send = ->(event, hint) { Dependabot::Sentry::Processor.process_chain(event, hint) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

config.processors has been removed in favour of a lambda before_send. I've replicated the behaviour by using a reducer to call processors in order.

I had to migrate raven_context to a custom processor as well, because raven_context is no longer attached to events by default.

config.propagate_traces = false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This prevents Sentry adding Sentry-Trace and Baggage headers by default to all outgoing requests. It's designed to make it easier to do distributed tracing between microservices or frontend/backend. As we don't have that architecture, and it currently breaks our smoke test cache, I've chosen to disable it by default.

If we want to get this distributed trace information, we'll need to ignore these headers in our updater proxy.

References:

Copy link
Member

Choose a reason for hiding this comment

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

I think disabling is the right call as Dependabot makes most calls to external endpoints that don't care about our tracing.

end

require "dependabot/opentelemetry"
Expand Down
Loading
Loading