Skip to content

Commit

Permalink
Simplify Spotlight
Browse files Browse the repository at this point in the history
  • Loading branch information
natikgadzhi committed Nov 27, 2023
1 parent 3026b9b commit 3d29a21
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 148 deletions.
13 changes: 10 additions & 3 deletions sentry-ruby/lib/sentry/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
require "sentry/dsn"
require "sentry/release_detector"
require "sentry/transport/configuration"
require "sentry/spotlight/configuration"
require "sentry/linecache"
require "sentry/interfaces/stacktrace_builder"

Expand Down Expand Up @@ -142,6 +141,14 @@ class Configuration
# Whether to capture local variables from the raised exception's frame. Default is false.
# @return [Boolean]
attr_accessor :include_local_variables

# Whether to capture events and traces into Spotlight. Default is false.
# If you set this to true, Sentry will send events and traces to the local
# Sidecar proxy at http://localhost:8969/stream.
# If you want to use a different Sidecar proxy address, set this to String
# with the proxy URL.
# @return [Boolean, String]
attr_accessor :spotlight

# @deprecated Use {#include_local_variables} instead.
alias_method :capture_exception_frame_locals, :include_local_variables
Expand Down Expand Up @@ -360,11 +367,11 @@ def initialize
self.traces_sampler = nil
self.enable_tracing = nil

self.spotlight = false

@transport = Transport::Configuration.new
@gem_specs = Hash[Gem::Specification.map { |spec| [spec.name, spec.version.to_s] }] if Gem::Specification.respond_to?(:map)

@spotlight = Spotlight::Configuration.new

run_post_initialization_callbacks
end

Expand Down
65 changes: 63 additions & 2 deletions sentry-ruby/lib/sentry/spotlight.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,63 @@
require "sentry/spotlight/configuration"
require "sentry/spotlight/transport"
# frozen_string_literal: true

require "net/http"
require "zlib"

module Sentry

# Spotlight Transport class is like HTTPTransport,
# but it's experimental, with limited featureset.
# - It does not care about rate limits, assuming working with local Sidecar proxy
# - Designed to just report events to Spotlight in development.
#
# TODO: This needs a cleanup, we could extract most of common code into a module.
class Spotlight
GZIP_ENCODING = "gzip"
GZIP_THRESHOLD = 1024 * 30
CONTENT_TYPE = 'application/x-sentry-envelope'
USER_AGENT = "sentry-ruby/#{Sentry::VERSION}"

# Takes the sidecar URL in and initializes the new Spotlight transport.
# HTTPTransport will call this if config.spotlight is truthy, and pass it here.
# so sidecar_url arg can either be true, or a string with the sidecar URL.
def initialize(sidecar_url)
@sidecar_url = sidecar_url.is_a?(String) ? sidecar_url : "http://localhost:8769/stream"
end

def send_data(data)
headers = {
'Content-Type' => CONTENT_TYPE,

Check warning on line 29 in sentry-ruby/lib/sentry/spotlight.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/spotlight.rb#L29

Added line #L29 was not covered by tests
'Content-Encoding' => "",
'User-Agent' => USER_AGENT
}

response = conn.start do |http|
request = ::Net::HTTP::Post.new(@sidecar_url, headers)
request.body = data
http.request(request)

Check warning on line 37 in sentry-ruby/lib/sentry/spotlight.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/spotlight.rb#L34-L37

Added lines #L34 - L37 were not covered by tests
end

unless response.code.match?(/\A2\d{2}/)
error_info = "the server responded with status #{response.code}"
error_info += "\nbody: #{response.body}"
error_info += " Error in headers is: #{response['x-sentry-error']}" if response['x-sentry-error']

Check warning on line 43 in sentry-ruby/lib/sentry/spotlight.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/spotlight.rb#L40-L43

Added lines #L40 - L43 were not covered by tests

raise Sentry::ExternalError, error_info

Check warning on line 45 in sentry-ruby/lib/sentry/spotlight.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/spotlight.rb#L45

Added line #L45 was not covered by tests
end

# TODO: We might want to rescue the other HTTP_ERRORS like in HTTPTransport
rescue SocketError, * Sentry::HTTPTransport::HTTP_ERRORS => e
raise Sentry::ExternalError.new(e.message)

Check warning on line 50 in sentry-ruby/lib/sentry/spotlight.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/spotlight.rb#L50

Added line #L50 was not covered by tests
end

private

# Similar to HTTPTransport connection, but does not support Proxy and SSL
def conn
sidecar = URI(@sidecar_url)
connection = ::Net::HTTP.new(sidecar.hostname, sidecar.port, nil)
connection.use_ssl = false
connection

Check warning on line 60 in sentry-ruby/lib/sentry/spotlight.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/spotlight.rb#L57-L60

Added lines #L57 - L60 were not covered by tests
end
end
end
50 changes: 0 additions & 50 deletions sentry-ruby/lib/sentry/spotlight/configuration.rb

This file was deleted.

77 changes: 0 additions & 77 deletions sentry-ruby/lib/sentry/spotlight/transport.rb

This file was deleted.

2 changes: 1 addition & 1 deletion sentry-ruby/lib/sentry/transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Transport
def initialize(configuration)
@logger = configuration.logger
@transport_configuration = configuration.transport
@spotlight_configuration = configuration.spotlight
@spotlight = configuration.spotlight
@dsn = configuration.dsn
@rate_limits = {}
@send_client_reports = configuration.send_client_reports
Expand Down
13 changes: 9 additions & 4 deletions sentry-ruby/lib/sentry/transport/http_transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,21 @@ class HTTPTransport < Transport
Zlib::BufError, Errno::EHOSTUNREACH, Errno::ECONNREFUSED
].freeze


def initialize(*args)
super
@endpoint = @dsn.envelope_endpoint

log_debug("Sentry HTTP Transport will connect to #{@dsn.server}")

if @spotlight_configuration.enabled?
@spotlight_transport = Sentry::Spotlight::Transport.new(@transport_configuration)
if @spotlight
@spotlight_transport = Sentry::Spotlight.new(@spotlight)
end
end

def send_data(data)
@spotlight_transport.send_data(data) unless @spotlight_transport.nil?
if should_send_to_spotlight?
@spotlight_transport.send_data(data)
end

encoding = ""

Expand Down Expand Up @@ -142,6 +143,10 @@ def should_compress?(data)
@transport_configuration.encoding == GZIP_ENCODING && data.bytesize >= GZIP_THRESHOLD
end

def should_send_to_spotlight?
!@spotlight_transport.nil?
end

def conn
server = URI(@dsn.server)

Expand Down
4 changes: 1 addition & 3 deletions sentry-ruby/spec/sentry/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,7 @@

describe "#spotlight" do
it "returns initialized Spotlight config by default" do
spotlight_config = subject.spotlight
expect(spotlight_config.enabled).to eq(false)
expect(spotlight_config.sidecar_url).to eq("http://localhost:8969/stream")
expect(subject.spotlight).to eq(false)
end
end

Expand Down
12 changes: 4 additions & 8 deletions sentry-ruby/spec/sentry/transport/http_transport_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -324,28 +324,24 @@

describe "Spotlight integration" do
let(:fake_response) { build_fake_response("200") }
context "when spotlight is enabled" do
let(:spotlight_transport) { Sentry::Spotlight::Transport.new(configuration.spotlight) }

context "when spotlight is enabled" do
it "calls @spotlight_transport.send_data(data)" do
configuration.spotlight.enabled = true
configuration.spotlight = true

stub_request(fake_response)

subject.instance_variable_set(:@spotlight_transport, spotlight_transport)
expect( spotlight_transport ).to receive(:send_data).with(data)
expect( subject.instance_variable_get(:@spotlight_transport) ).to receive(:send_data).with(data)
subject.send_data(data)
end
end

context "when spotlight integration is disabled" do
let(:spotlight_transport) { nil }
it "does not call @spotlight_transport.send_data(data)" do
configuration.spotlight.enabled = false

stub_request(fake_response)

expect( spotlight_transport ).not_to receive(:send_data).with(data)
expect( subject.instance_variable_get(:@spotlight_transport) ).not_to receive(:send_data).with(data)
subject.send_data(data)
end
end
Expand Down

0 comments on commit 3d29a21

Please sign in to comment.