From 3026b9b22a49c20c14726be027eaa747655f3887 Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Wed, 22 Nov 2023 19:06:23 -0800 Subject: [PATCH 1/7] Sidecar support in Sentry-Ruby --- sentry-ruby/lib/sentry-ruby.rb | 1 + sentry-ruby/lib/sentry/configuration.rb | 7 ++ sentry-ruby/lib/sentry/spotlight.rb | 2 + .../lib/sentry/spotlight/configuration.rb | 50 ++++++++++++ sentry-ruby/lib/sentry/spotlight/transport.rb | 77 +++++++++++++++++++ sentry-ruby/lib/sentry/transport.rb | 1 + .../lib/sentry/transport/http_transport.rb | 6 ++ sentry-ruby/spec/sentry/configuration_spec.rb | 8 ++ .../sentry/transport/http_transport_spec.rb | 29 +++++++ 9 files changed, 181 insertions(+) create mode 100644 sentry-ruby/lib/sentry/spotlight.rb create mode 100644 sentry-ruby/lib/sentry/spotlight/configuration.rb create mode 100644 sentry-ruby/lib/sentry/spotlight/transport.rb diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index c3577d0f0..c1d48db69 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -22,6 +22,7 @@ require "sentry/background_worker" require "sentry/session_flusher" require "sentry/cron/monitor_check_ins" +require "sentry/spotlight" [ "sentry/rake", diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 1f89f31a3..865d3bc05 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -7,6 +7,7 @@ require "sentry/dsn" require "sentry/release_detector" require "sentry/transport/configuration" +require "sentry/spotlight/configuration" require "sentry/linecache" require "sentry/interfaces/stacktrace_builder" @@ -234,6 +235,10 @@ def capture_exception_frame_locals=(value) # @return [Boolean, nil] attr_reader :enable_tracing + # Returns the Spotlight::Configuration object + # @return [Spotlight::Configuration] + attr_reader :spotlight + # Send diagnostic client reports about dropped events, true by default # tries to attach to an existing envelope max once every 30s # @return [Boolean] @@ -358,6 +363,8 @@ def initialize @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 diff --git a/sentry-ruby/lib/sentry/spotlight.rb b/sentry-ruby/lib/sentry/spotlight.rb new file mode 100644 index 000000000..fef97affd --- /dev/null +++ b/sentry-ruby/lib/sentry/spotlight.rb @@ -0,0 +1,2 @@ +require "sentry/spotlight/configuration" +require "sentry/spotlight/transport" diff --git a/sentry-ruby/lib/sentry/spotlight/configuration.rb b/sentry-ruby/lib/sentry/spotlight/configuration.rb new file mode 100644 index 000000000..294cc14d9 --- /dev/null +++ b/sentry-ruby/lib/sentry/spotlight/configuration.rb @@ -0,0 +1,50 @@ +module Sentry + module Spotlight + # Sentry Spotlight configuration. + class Configuration + + # When enabled, Sentry will send all events and traces to the provided + # Spotlight Sidecar URL. + # Defaults to false. + # @return [Boolean] + attr_reader :enabled + + # Spotlight Sidecar URL as a String. + # Defaults to "http://localhost:8969/stream" + # @return [String] + attr_accessor :sidecar_url + + def initialize + @enabled = false + @sidecar_url = "http://localhost:8969/stream" + end + + def enabled? + enabled + end + + # Enables or disables Spotlight. + def enabled=(value) + unless [true, false].include?(value) + raise ArgumentError, "Spotlight config.enabled must be a boolean" + end + + if value == true + unless ['development', 'test'].include?(environment_from_env) + # Using the default logger here for a one-off warning. + ::Sentry::Logger.new(STDOUT).warn("[Spotlight] Spotlight is enabled in a non-development environment!") + end + end + end + + private + + # TODO: Sentry::Configuration already reads the env the same way as below, but it also has a way to _set_ environment + # in it's config. So this introduces a bug where env could be different, depending on whether the user set the environment + # manually. + def environment_from_env + ENV['SENTRY_CURRENT_ENV'] || ENV['SENTRY_ENVIRONMENT'] || ENV['RAILS_ENV'] || ENV['RACK_ENV'] || 'development' + end + end + end +end diff --git a/sentry-ruby/lib/sentry/spotlight/transport.rb b/sentry-ruby/lib/sentry/spotlight/transport.rb new file mode 100644 index 000000000..8aba4970d --- /dev/null +++ b/sentry-ruby/lib/sentry/spotlight/transport.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require "net/http" +require "zlib" + +module Sentry + module Spotlight + + + # 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 Transport + + GZIP_ENCODING = "gzip" + GZIP_THRESHOLD = 1024 * 30 + CONTENT_TYPE = 'application/x-sentry-envelope' + USER_AGENT = "sentry-ruby/#{Sentry::VERSION}" + + # Initialize a new Spotlight transport + # with the provided Spotlight configuration. + def initialize(spotlight_configuration) + @configuration = spotlight_configuration + end + + def send_data(data) + encoding = "" + + if should_compress?(data) + data = Zlib.gzip(data) + encoding = GZIP_ENCODING + end + + headers = { + 'Content-Type' => CONTENT_TYPE, + 'Content-Encoding' => encoding, + 'X-Sentry-Auth' => generate_auth_header, + 'User-Agent' => USER_AGENT + } + + response = conn.start do |http| + request = ::Net::HTTP::Post.new(@configuration.sidecar_url, headers) + request.body = data + http.request(request) + 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'] + + raise Sentry::ExternalError, error_info + end + rescue SocketError => e + raise Sentry::ExternalError.new(e.message) + end + + private + + def should_compress?(data) + @transport_configuration.encoding == GZIP_ENCODING && data.bytesize >= GZIP_THRESHOLD + end + + # Similar to HTTPTransport connection, but does not support Proxy and SSL + def conn + sidecar = URL(@configuration.sidecar_url) + connection = ::Net::HTTP.new(sidecar.hostname, sidecar.port, nil) + connection.use_ssl = false + connection + end + + end + end +end diff --git a/sentry-ruby/lib/sentry/transport.rb b/sentry-ruby/lib/sentry/transport.rb index 2b786af78..686ba2dc2 100644 --- a/sentry-ruby/lib/sentry/transport.rb +++ b/sentry-ruby/lib/sentry/transport.rb @@ -32,6 +32,7 @@ class Transport def initialize(configuration) @logger = configuration.logger @transport_configuration = configuration.transport + @spotlight_configuration = configuration.spotlight @dsn = configuration.dsn @rate_limits = {} @send_client_reports = configuration.send_client_reports diff --git a/sentry-ruby/lib/sentry/transport/http_transport.rb b/sentry-ruby/lib/sentry/transport/http_transport.rb index 4b1fa1771..937dd1162 100644 --- a/sentry-ruby/lib/sentry/transport/http_transport.rb +++ b/sentry-ruby/lib/sentry/transport/http_transport.rb @@ -29,9 +29,15 @@ def initialize(*args) @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) + end end def send_data(data) + @spotlight_transport.send_data(data) unless @spotlight_transport.nil? + encoding = "" if should_compress?(data) diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index 5e4beb7ee..742532263 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -256,6 +256,14 @@ end end + 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") + end + end + context 'configuring for async' do it 'should be configurable to send events async' do subject.async = ->(_e) { :ok } diff --git a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb index bf342e356..27b7c85c6 100644 --- a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb @@ -321,4 +321,33 @@ end end end + + 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) } + + it "calls @spotlight_transport.send_data(data)" do + configuration.spotlight.enabled = true + + stub_request(fake_response) + + subject.instance_variable_set(:@spotlight_transport, spotlight_transport) + expect( 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) + subject.send_data(data) + end + end + end end From 3d29a21422bd46d58f4b295a15100ae5905c5fd5 Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Mon, 27 Nov 2023 12:58:59 -0800 Subject: [PATCH 2/7] Simplify Spotlight --- sentry-ruby/lib/sentry/configuration.rb | 13 +++- sentry-ruby/lib/sentry/spotlight.rb | 65 +++++++++++++++- .../lib/sentry/spotlight/configuration.rb | 50 ------------ sentry-ruby/lib/sentry/spotlight/transport.rb | 77 ------------------- sentry-ruby/lib/sentry/transport.rb | 2 +- .../lib/sentry/transport/http_transport.rb | 13 +++- sentry-ruby/spec/sentry/configuration_spec.rb | 4 +- .../sentry/transport/http_transport_spec.rb | 12 +-- 8 files changed, 88 insertions(+), 148 deletions(-) delete mode 100644 sentry-ruby/lib/sentry/spotlight/configuration.rb delete mode 100644 sentry-ruby/lib/sentry/spotlight/transport.rb diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 865d3bc05..c57e1e24b 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -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" @@ -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 @@ -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 diff --git a/sentry-ruby/lib/sentry/spotlight.rb b/sentry-ruby/lib/sentry/spotlight.rb index fef97affd..58278aafa 100644 --- a/sentry-ruby/lib/sentry/spotlight.rb +++ b/sentry-ruby/lib/sentry/spotlight.rb @@ -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, + '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) + 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'] + + raise Sentry::ExternalError, error_info + 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) + 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 + end + end +end diff --git a/sentry-ruby/lib/sentry/spotlight/configuration.rb b/sentry-ruby/lib/sentry/spotlight/configuration.rb deleted file mode 100644 index 294cc14d9..000000000 --- a/sentry-ruby/lib/sentry/spotlight/configuration.rb +++ /dev/null @@ -1,50 +0,0 @@ -module Sentry - module Spotlight - # Sentry Spotlight configuration. - class Configuration - - # When enabled, Sentry will send all events and traces to the provided - # Spotlight Sidecar URL. - # Defaults to false. - # @return [Boolean] - attr_reader :enabled - - # Spotlight Sidecar URL as a String. - # Defaults to "http://localhost:8969/stream" - # @return [String] - attr_accessor :sidecar_url - - def initialize - @enabled = false - @sidecar_url = "http://localhost:8969/stream" - end - - def enabled? - enabled - end - - # Enables or disables Spotlight. - def enabled=(value) - unless [true, false].include?(value) - raise ArgumentError, "Spotlight config.enabled must be a boolean" - end - - if value == true - unless ['development', 'test'].include?(environment_from_env) - # Using the default logger here for a one-off warning. - ::Sentry::Logger.new(STDOUT).warn("[Spotlight] Spotlight is enabled in a non-development environment!") - end - end - end - - private - - # TODO: Sentry::Configuration already reads the env the same way as below, but it also has a way to _set_ environment - # in it's config. So this introduces a bug where env could be different, depending on whether the user set the environment - # manually. - def environment_from_env - ENV['SENTRY_CURRENT_ENV'] || ENV['SENTRY_ENVIRONMENT'] || ENV['RAILS_ENV'] || ENV['RACK_ENV'] || 'development' - end - end - end -end diff --git a/sentry-ruby/lib/sentry/spotlight/transport.rb b/sentry-ruby/lib/sentry/spotlight/transport.rb deleted file mode 100644 index 8aba4970d..000000000 --- a/sentry-ruby/lib/sentry/spotlight/transport.rb +++ /dev/null @@ -1,77 +0,0 @@ -# frozen_string_literal: true - -require "net/http" -require "zlib" - -module Sentry - module Spotlight - - - # 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 Transport - - GZIP_ENCODING = "gzip" - GZIP_THRESHOLD = 1024 * 30 - CONTENT_TYPE = 'application/x-sentry-envelope' - USER_AGENT = "sentry-ruby/#{Sentry::VERSION}" - - # Initialize a new Spotlight transport - # with the provided Spotlight configuration. - def initialize(spotlight_configuration) - @configuration = spotlight_configuration - end - - def send_data(data) - encoding = "" - - if should_compress?(data) - data = Zlib.gzip(data) - encoding = GZIP_ENCODING - end - - headers = { - 'Content-Type' => CONTENT_TYPE, - 'Content-Encoding' => encoding, - 'X-Sentry-Auth' => generate_auth_header, - 'User-Agent' => USER_AGENT - } - - response = conn.start do |http| - request = ::Net::HTTP::Post.new(@configuration.sidecar_url, headers) - request.body = data - http.request(request) - 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'] - - raise Sentry::ExternalError, error_info - end - rescue SocketError => e - raise Sentry::ExternalError.new(e.message) - end - - private - - def should_compress?(data) - @transport_configuration.encoding == GZIP_ENCODING && data.bytesize >= GZIP_THRESHOLD - end - - # Similar to HTTPTransport connection, but does not support Proxy and SSL - def conn - sidecar = URL(@configuration.sidecar_url) - connection = ::Net::HTTP.new(sidecar.hostname, sidecar.port, nil) - connection.use_ssl = false - connection - end - - end - end -end diff --git a/sentry-ruby/lib/sentry/transport.rb b/sentry-ruby/lib/sentry/transport.rb index 686ba2dc2..7f11e3161 100644 --- a/sentry-ruby/lib/sentry/transport.rb +++ b/sentry-ruby/lib/sentry/transport.rb @@ -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 diff --git a/sentry-ruby/lib/sentry/transport/http_transport.rb b/sentry-ruby/lib/sentry/transport/http_transport.rb index 937dd1162..e29193713 100644 --- a/sentry-ruby/lib/sentry/transport/http_transport.rb +++ b/sentry-ruby/lib/sentry/transport/http_transport.rb @@ -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 = "" @@ -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) diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index 742532263..9713a00dc 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -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 diff --git a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb index 27b7c85c6..0273e0a20 100644 --- a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb @@ -324,16 +324,14 @@ 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 @@ -341,11 +339,9 @@ 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 From fcc9fb597740f59ce97ad69cc3dc5311a196f4a1 Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Mon, 27 Nov 2023 13:55:33 -0800 Subject: [PATCH 3/7] Fixed the wrong endpoint URL --- sentry-ruby/lib/sentry/spotlight.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sentry-ruby/lib/sentry/spotlight.rb b/sentry-ruby/lib/sentry/spotlight.rb index 58278aafa..77777eef3 100644 --- a/sentry-ruby/lib/sentry/spotlight.rb +++ b/sentry-ruby/lib/sentry/spotlight.rb @@ -21,7 +21,7 @@ class Spotlight # 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" + @sidecar_url = sidecar_url.is_a?(String) ? sidecar_url : "http://localhost:8969/stream" end def send_data(data) @@ -32,7 +32,7 @@ def send_data(data) } response = conn.start do |http| - request = ::Net::HTTP::Post.new(@sidecar_url, headers) + request = ::Net::HTTP::Post.new("/stream", headers) request.body = data http.request(request) end @@ -45,7 +45,6 @@ def send_data(data) raise Sentry::ExternalError, error_info 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) end From b6ea84d2598668b7e6a7c038bfad97d8e7b1630d Mon Sep 17 00:00:00 2001 From: Natik Gadzhi Date: Mon, 27 Nov 2023 21:17:51 -0800 Subject: [PATCH 4/7] Spotlight changelog item --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccc65c59f..31ffe1569 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ### Features +- You can now use [Spotlight](https://spotlightjs.com) with your apps that use sentry-ruby! [#2175](https://github.com/getsentry/sentry-ruby/pulls/2175) + ### Bug Fixes - Network errors raised in `Sentry::HTTPTransport` will no longer be reported to Sentry [#2178](https://github.com/getsentry/sentry-ruby/pull/2178) From bf5119adc8c3bb70605839d86ef78b3f2ca4a900 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 4 Dec 2023 16:23:18 +0100 Subject: [PATCH 5/7] Rework some stuff --- CHANGELOG.md | 2 - sentry-ruby/lib/sentry-ruby.rb | 1 - sentry-ruby/lib/sentry/client.rb | 7 ++ sentry-ruby/lib/sentry/configuration.rb | 11 +-- sentry-ruby/lib/sentry/spotlight.rb | 62 ------------- sentry-ruby/lib/sentry/transport.rb | 14 +-- .../lib/sentry/transport/http_transport.rb | 91 ++++++++++--------- .../sentry/transport/spotlight_transport.rb | 28 ++++++ .../sentry/transport/http_transport_spec.rb | 32 +++---- sentry-ruby/spec/sentry/transport_spec.rb | 19 ---- 10 files changed, 101 insertions(+), 166 deletions(-) delete mode 100644 sentry-ruby/lib/sentry/spotlight.rb create mode 100644 sentry-ruby/lib/sentry/transport/spotlight_transport.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 233913ddc..85ff53d84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,9 @@ ### Features - - You can now use [Spotlight](https://spotlightjs.com) with your apps that use sentry-ruby! [#2175](https://github.com/getsentry/sentry-ruby/pulls/2175) - Improve default slug generation for `sidekiq-scheduler` [#2184](https://github.com/getsentry/sentry-ruby/pull/2184) - ### Bug Fixes - Network errors raised in `Sentry::HTTPTransport` will no longer be reported to Sentry [#2178](https://github.com/getsentry/sentry-ruby/pull/2178) diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index c1d48db69..c3577d0f0 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -22,7 +22,6 @@ require "sentry/background_worker" require "sentry/session_flusher" require "sentry/cron/monitor_check_ins" -require "sentry/spotlight" [ "sentry/rake", diff --git a/sentry-ruby/lib/sentry/client.rb b/sentry-ruby/lib/sentry/client.rb index d9e0f5b07..d2e886395 100644 --- a/sentry-ruby/lib/sentry/client.rb +++ b/sentry-ruby/lib/sentry/client.rb @@ -10,6 +10,10 @@ class Client # @return [Transport] attr_reader :transport + # The Transport object that'll send events for the client. + # @return [SpotlightTransport, nil] + attr_reader :spotlight_transport + # @!macro configuration attr_reader :configuration @@ -32,6 +36,8 @@ def initialize(configuration) DummyTransport.new(configuration) end end + + @spotlight_transport = SpotlightTransport.new(configuration) if configuration.spotlight end # Applies the given scope's data to the event and sends it to Sentry. @@ -167,6 +173,7 @@ def send_event(event, hint = nil) end transport.send_event(event) + spotlight_transport&.send_event(event) event rescue => e diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index c57e1e24b..606147aae 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -141,7 +141,7 @@ 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. @@ -242,10 +242,6 @@ def capture_exception_frame_locals=(value) # @return [Boolean, nil] attr_reader :enable_tracing - # Returns the Spotlight::Configuration object - # @return [Spotlight::Configuration] - attr_reader :spotlight - # Send diagnostic client reports about dropped events, true by default # tries to attach to an existing envelope max once every 30s # @return [Boolean] @@ -356,6 +352,7 @@ def initialize self.auto_session_tracking = true self.trusted_proxies = [] self.dsn = ENV['SENTRY_DSN'] + self.spotlight = false self.server_name = server_name_from_env self.instrumenter = :sentry self.trace_propagation_targets = [PROPAGATION_TARGETS_MATCH_ALL] @@ -367,8 +364,6 @@ 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) @@ -465,7 +460,7 @@ def profiles_sample_rate=(profiles_sample_rate) def sending_allowed? @errors = [] - valid? && capture_in_environment? + spotlight || (valid? && capture_in_environment?) end def sample_allowed? diff --git a/sentry-ruby/lib/sentry/spotlight.rb b/sentry-ruby/lib/sentry/spotlight.rb deleted file mode 100644 index 77777eef3..000000000 --- a/sentry-ruby/lib/sentry/spotlight.rb +++ /dev/null @@ -1,62 +0,0 @@ -# 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:8969/stream" - end - - def send_data(data) - headers = { - 'Content-Type' => CONTENT_TYPE, - 'Content-Encoding' => "", - 'User-Agent' => USER_AGENT - } - - response = conn.start do |http| - request = ::Net::HTTP::Post.new("/stream", headers) - request.body = data - http.request(request) - 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'] - - raise Sentry::ExternalError, error_info - end - - rescue SocketError, * Sentry::HTTPTransport::HTTP_ERRORS => e - raise Sentry::ExternalError.new(e.message) - 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 - end - end -end diff --git a/sentry-ruby/lib/sentry/transport.rb b/sentry-ruby/lib/sentry/transport.rb index 7f11e3161..393e36de6 100644 --- a/sentry-ruby/lib/sentry/transport.rb +++ b/sentry-ruby/lib/sentry/transport.rb @@ -32,7 +32,6 @@ class Transport def initialize(configuration) @logger = configuration.logger @transport_configuration = configuration.transport - @spotlight = configuration.spotlight @dsn = configuration.dsn @rate_limits = {} @send_client_reports = configuration.send_client_reports @@ -120,18 +119,6 @@ def is_rate_limited?(item_type) !!delay && delay > Time.now end - def generate_auth_header - now = Sentry.utc_now.to_i - fields = { - 'sentry_version' => PROTOCOL_VERSION, - 'sentry_client' => USER_AGENT, - 'sentry_timestamp' => now, - 'sentry_key' => @dsn.public_key - } - fields['sentry_secret'] = @dsn.secret_key if @dsn.secret_key - 'Sentry ' + fields.map { |key, value| "#{key}=#{value}" }.join(', ') - end - def envelope_from_event(event) # Convert to hash event_payload = event.to_hash @@ -221,3 +208,4 @@ def reject_rate_limited_items(envelope) require "sentry/transport/dummy_transport" require "sentry/transport/http_transport" +require "sentry/transport/spotlight_transport" diff --git a/sentry-ruby/lib/sentry/transport/http_transport.rb b/sentry-ruby/lib/sentry/transport/http_transport.rb index e29193713..200108098 100644 --- a/sentry-ruby/lib/sentry/transport/http_transport.rb +++ b/sentry-ruby/lib/sentry/transport/http_transport.rb @@ -23,22 +23,13 @@ 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 - @spotlight_transport = Sentry::Spotlight.new(@spotlight) - end + log_debug("Sentry HTTP Transport will connect to #{@dsn.server}") if @dsn end def send_data(data) - if should_send_to_spotlight? - @spotlight_transport.send_data(data) - end - encoding = "" if should_compress?(data) @@ -49,12 +40,14 @@ def send_data(data) headers = { 'Content-Type' => CONTENT_TYPE, 'Content-Encoding' => encoding, - 'X-Sentry-Auth' => generate_auth_header, 'User-Agent' => USER_AGENT } + auth_header = generate_auth_header + headers['X-Sentry-Auth'] = auth_header if auth_header + response = conn.start do |http| - request = ::Net::HTTP::Post.new(@endpoint, headers) + request = ::Net::HTTP::Post.new(endpoint, headers) request.body = data http.request(request) end @@ -79,6 +72,49 @@ def send_data(data) raise Sentry::ExternalError.new(e&.message) end + def endpoint + @dsn.envelope_endpoint + end + + def generate_auth_header + return nil unless @dsn + + now = Sentry.utc_now.to_i + fields = { + 'sentry_version' => PROTOCOL_VERSION, + 'sentry_client' => USER_AGENT, + 'sentry_timestamp' => now, + 'sentry_key' => @dsn.public_key + } + fields['sentry_secret'] = @dsn.secret_key if @dsn.secret_key + 'Sentry ' + fields.map { |key, value| "#{key}=#{value}" }.join(', ') + end + + def conn + server = URI(@dsn.server) + + # connection respects proxy setting from @transport_configuration, or environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY) + # Net::HTTP will automatically read the env vars. + # See https://ruby-doc.org/3.2.2/stdlibs/net/Net/HTTP.html#class-Net::HTTP-label-Proxies + connection = + if proxy = normalize_proxy(@transport_configuration.proxy) + ::Net::HTTP.new(server.hostname, server.port, proxy[:uri].hostname, proxy[:uri].port, proxy[:user], proxy[:password]) + else + ::Net::HTTP.new(server.hostname, server.port) + end + + connection.use_ssl = server.scheme == "https" + connection.read_timeout = @transport_configuration.timeout + connection.write_timeout = @transport_configuration.timeout if connection.respond_to?(:write_timeout) + connection.open_timeout = @transport_configuration.open_timeout + + ssl_configuration.each do |key, value| + connection.send("#{key}=", value) + end + + connection + end + private def has_rate_limited_header?(headers) @@ -143,35 +179,6 @@ 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) - - # connection respects proxy setting from @transport_configuration, or environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY) - # Net::HTTP will automatically read the env vars. - # See https://ruby-doc.org/3.2.2/stdlibs/net/Net/HTTP.html#class-Net::HTTP-label-Proxies - connection = - if proxy = normalize_proxy(@transport_configuration.proxy) - ::Net::HTTP.new(server.hostname, server.port, proxy[:uri].hostname, proxy[:uri].port, proxy[:user], proxy[:password]) - else - ::Net::HTTP.new(server.hostname, server.port) - end - - connection.use_ssl = server.scheme == "https" - connection.read_timeout = @transport_configuration.timeout - connection.write_timeout = @transport_configuration.timeout if connection.respond_to?(:write_timeout) - connection.open_timeout = @transport_configuration.open_timeout - - ssl_configuration.each do |key, value| - connection.send("#{key}=", value) - end - - connection - end - # @param proxy [String, URI, Hash] Proxy config value passed into `config.transport`. # Accepts either a URI formatted string, URI, or a hash with the `uri`, `user`, and `password` keys. # @return [Hash] Normalized proxy config that will be passed into `Net::HTTP` diff --git a/sentry-ruby/lib/sentry/transport/spotlight_transport.rb b/sentry-ruby/lib/sentry/transport/spotlight_transport.rb new file mode 100644 index 000000000..2290149ca --- /dev/null +++ b/sentry-ruby/lib/sentry/transport/spotlight_transport.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require "net/http" +require "zlib" + +module Sentry + # Designed to just report events to Spotlight in development. + class SpotlightTransport < HTTPTransport + DEFAULT_SIDECAR_URL = "http://localhost:8969/stream" + + def initialize(configuration) + super + @sidecar_url = configuration.spotlight.is_a?(String) ? configuration.spotlight : DEFAULT_SIDECAR_URL + end + + def endpoint + "/stream" + end + + # 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 + end + end +end diff --git a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb index 0273e0a20..d46173fe6 100644 --- a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb @@ -12,6 +12,7 @@ end let(:client) { Sentry::Client.new(configuration) } let(:event) { client.event_from_message("foobarbaz") } + let(:fake_time) { Time.now } let(:data) do subject.serialize_envelope(subject.envelope_from_event(event.to_hash)).first end @@ -322,28 +323,21 @@ end end - describe "Spotlight integration" do - let(:fake_response) { build_fake_response("200") } - - context "when spotlight is enabled" do - it "calls @spotlight_transport.send_data(data)" do - configuration.spotlight = true - - stub_request(fake_response) - - expect( subject.instance_variable_get(:@spotlight_transport) ).to receive(:send_data).with(data) - subject.send_data(data) - end + describe "#generate_auth_header" do + it "generates an auth header" do + expect(subject.send(:generate_auth_header)).to eq( + "Sentry sentry_version=7, sentry_client=sentry-ruby/#{Sentry::VERSION}, sentry_timestamp=#{fake_time.to_i}, " \ + "sentry_key=12345, sentry_secret=67890" + ) end - context "when spotlight integration is disabled" do - let(:spotlight_transport) { nil } - it "does not call @spotlight_transport.send_data(data)" do - stub_request(fake_response) + it "generates an auth header without a secret (Sentry 9)" do + configuration.server = "https://66260460f09b5940498e24bb7ce093a0@sentry.io/42" - expect( subject.instance_variable_get(:@spotlight_transport) ).not_to receive(:send_data).with(data) - subject.send_data(data) - end + expect(subject.send(:generate_auth_header)).to eq( + "Sentry sentry_version=7, sentry_client=sentry-ruby/#{Sentry::VERSION}, sentry_timestamp=#{fake_time.to_i}, " \ + "sentry_key=66260460f09b5940498e24bb7ce093a0" + ) end end end diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb index 7a44cbc89..fbad63039 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport_spec.rb @@ -9,7 +9,6 @@ config.logger = logger end end - let(:fake_time) { Time.now } let(:client) { Sentry::Client.new(configuration) } let(:hub) do @@ -475,22 +474,4 @@ end end end - - describe "#generate_auth_header" do - it "generates an auth header" do - expect(subject.send(:generate_auth_header)).to eq( - "Sentry sentry_version=7, sentry_client=sentry-ruby/#{Sentry::VERSION}, sentry_timestamp=#{fake_time.to_i}, " \ - "sentry_key=12345, sentry_secret=67890" - ) - end - - it "generates an auth header without a secret (Sentry 9)" do - configuration.server = "https://66260460f09b5940498e24bb7ce093a0@sentry.io/42" - - expect(subject.send(:generate_auth_header)).to eq( - "Sentry sentry_version=7, sentry_client=sentry-ruby/#{Sentry::VERSION}, sentry_timestamp=#{fake_time.to_i}, " \ - "sentry_key=66260460f09b5940498e24bb7ce093a0" - ) - end - end end From a50288b05c35bf9260436bbe642d55f4410d102a Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 4 Dec 2023 17:38:34 +0100 Subject: [PATCH 6/7] specs --- sentry-ruby/spec/sentry/client_spec.rb | 28 +++++++++++++ sentry-ruby/spec/sentry/configuration_spec.rb | 9 +++- .../sentry/transport/http_transport_spec.rb | 20 +++++++-- .../transport/spotlight_transport_spec.rb | 42 +++++++++++++++++++ sentry-ruby/spec/sentry_spec.rb | 18 ++++++++ 5 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 sentry-ruby/spec/sentry/transport/spotlight_transport_spec.rb diff --git a/sentry-ruby/spec/sentry/client_spec.rb b/sentry-ruby/spec/sentry/client_spec.rb index a3517dc1d..91eeeceee 100644 --- a/sentry-ruby/spec/sentry/client_spec.rb +++ b/sentry-ruby/spec/sentry/client_spec.rb @@ -67,6 +67,34 @@ def sentry_context end end + describe "#spotlight_transport" do + it "nil by default" do + expect(subject.spotlight_transport).to eq(nil) + end + + it "nil when false" do + configuration.spotlight = false + expect(subject.spotlight_transport).to eq(nil) + end + + it "has a transport when true" do + configuration.spotlight = true + expect(described_class.new(configuration).spotlight_transport).to be_a(Sentry::SpotlightTransport) + end + end + + describe "#send_event" do + context "with spotlight enabled" do + before { configuration.spotlight = true } + + it "calls spotlight transport" do + event = subject.event_from_message('test') + expect(subject.spotlight_transport).to receive(:send_event).with(event) + subject.send_event(event) + end + end + end + describe '#event_from_message' do let(:message) { 'This is a message' } diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index 9713a00dc..367c1450e 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -257,11 +257,18 @@ end describe "#spotlight" do - it "returns initialized Spotlight config by default" do + it "false by default" do expect(subject.spotlight).to eq(false) end end + describe "#sending_allowed?" do + it "true when spotlight" do + subject.spotlight = true + expect(subject.sending_allowed?).to eq(true) + end + end + context 'configuring for async' do it 'should be configurable to send events async' do subject.async = ->(_e) { :ok } diff --git a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb index d46173fe6..3c7b1b051 100644 --- a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb @@ -133,7 +133,7 @@ it "accepts a proxy from ENV[HTTP_PROXY]" do begin ENV["http_proxy"] = "https://stan:foobar@example.com:8080" - + stub_request(fake_response) do |_, http_obj| expect(http_obj.proxy_address).to eq("example.com") expect(http_obj.proxy_port).to eq(8080) @@ -143,7 +143,7 @@ expect(http_obj.proxy_pass).to eq("foobar") end end - + subject.send_data(data) ensure ENV["http_proxy"] = nil @@ -278,7 +278,7 @@ allow(::Net::HTTP).to receive(:new).and_raise(SocketError.new("socket error")) expect do subject.send_data(data) - end.to raise_error(Sentry::ExternalError) + end.to raise_error(Sentry::ExternalError) end it "reports other errors to Sentry if they are not recognized" do @@ -340,4 +340,18 @@ ) end end + + describe "#endpoint" do + it "returns correct endpoint" do + expect(subject.endpoint).to eq("/sentry/api/42/envelope/") + end + end + + describe "#conn" do + it "returns a connection" do + expect(subject.conn).to be_a(Net::HTTP) + expect(subject.conn.address).to eq("sentry.localdomain") + expect(subject.conn.use_ssl?).to eq(false) + end + end end diff --git a/sentry-ruby/spec/sentry/transport/spotlight_transport_spec.rb b/sentry-ruby/spec/sentry/transport/spotlight_transport_spec.rb new file mode 100644 index 000000000..f5bfebc3b --- /dev/null +++ b/sentry-ruby/spec/sentry/transport/spotlight_transport_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +RSpec.describe Sentry::SpotlightTransport do + let(:configuration) do + Sentry::Configuration.new.tap do |config| + config.spotlight = true + config.logger = Logger.new(nil) + end + end + + let(:custom_configuration) do + Sentry::Configuration.new.tap do |config| + config.spotlight = 'http://foobar@test.com' + config.logger = Logger.new(nil) + end + end + + subject { described_class.new(configuration) } + + describe '#endpoint' do + it 'returs correct endpoint' do + expect(subject.endpoint).to eq('/stream') + end + end + + describe '#conn' do + it 'returns connection with default host' do + expect(subject.conn).to be_a(Net::HTTP) + expect(subject.conn.address).to eq('localhost') + expect(subject.conn.port).to eq(8969) + expect(subject.conn.use_ssl?).to eq(false) + end + + it 'returns connection with overriden host' do + subject = described_class.new(custom_configuration) + expect(subject.conn).to be_a(Net::HTTP) + expect(subject.conn.address).to eq('test.com') + expect(subject.conn.port).to eq(80) + expect(subject.conn.use_ssl?).to eq(false) + end + end +end diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index b2fd37164..0396734ad 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -1,6 +1,9 @@ require "spec_helper" +require 'contexts/with_request_mock' RSpec.describe Sentry do + include_context "with request mock" + before do perform_basic_setup end @@ -145,6 +148,21 @@ event = last_sentry_event expect(event.tags[:hint][:foo]).to eq("bar") end + + context "with spotlight" do + before { perform_basic_setup { |c| c.spotlight = true } } + + it "sends the event to spotlight too" do + stub_request(build_fake_response("200")) do |request, http_obj| + expect(request["Content-Type"]).to eq("application/x-sentry-envelope") + expect(request["Content-Encoding"]).to eq("gzip") + expect(http_obj.address).to eq("localhost") + expect(http_obj.port).to eq(8969) + end + + described_class.send_event(event) + end + end end describe ".capture_event" do From 5e8462dc3a39b9f73e15f5eb87b20910a6b51791 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 5 Dec 2023 15:03:16 +0100 Subject: [PATCH 7/7] Add max fail 3 times --- .../lib/sentry/transport/http_transport.rb | 1 + .../sentry/transport/spotlight_transport.rb | 22 +++++++++++ .../transport/spotlight_transport_spec.rb | 39 +++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/sentry-ruby/lib/sentry/transport/http_transport.rb b/sentry-ruby/lib/sentry/transport/http_transport.rb index 200108098..270f05d4f 100644 --- a/sentry-ruby/lib/sentry/transport/http_transport.rb +++ b/sentry-ruby/lib/sentry/transport/http_transport.rb @@ -69,6 +69,7 @@ def send_data(data) raise Sentry::ExternalError, error_info end rescue SocketError, *HTTP_ERRORS => e + on_error if respond_to?(:on_error) raise Sentry::ExternalError.new(e&.message) end diff --git a/sentry-ruby/lib/sentry/transport/spotlight_transport.rb b/sentry-ruby/lib/sentry/transport/spotlight_transport.rb index 2290149ca..804f1d23e 100644 --- a/sentry-ruby/lib/sentry/transport/spotlight_transport.rb +++ b/sentry-ruby/lib/sentry/transport/spotlight_transport.rb @@ -7,16 +7,38 @@ module Sentry # Designed to just report events to Spotlight in development. class SpotlightTransport < HTTPTransport DEFAULT_SIDECAR_URL = "http://localhost:8969/stream" + MAX_FAILED_REQUESTS = 3 def initialize(configuration) super @sidecar_url = configuration.spotlight.is_a?(String) ? configuration.spotlight : DEFAULT_SIDECAR_URL + @failed = 0 + @logged = false + + log_debug("[Spotlight] initialized for url #{@sidecar_url}") end def endpoint "/stream" end + def send_data(data) + if @failed >= MAX_FAILED_REQUESTS + unless @logged + log_debug("[Spotlight] disabling because of too many request failures") + @logged = true + end + + return + end + + super + end + + def on_error + @failed += 1 + end + # Similar to HTTPTransport connection, but does not support Proxy and SSL def conn sidecar = URI(@sidecar_url) diff --git a/sentry-ruby/spec/sentry/transport/spotlight_transport_spec.rb b/sentry-ruby/spec/sentry/transport/spotlight_transport_spec.rb index f5bfebc3b..04de57af6 100644 --- a/sentry-ruby/spec/sentry/transport/spotlight_transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport/spotlight_transport_spec.rb @@ -15,8 +15,23 @@ end end + let(:client) { Sentry::Client.new(configuration) } + let(:event) { client.event_from_message("foobarbaz") } + let(:data) do + subject.serialize_envelope(subject.envelope_from_event(event.to_hash)).first + end + subject { described_class.new(configuration) } + it 'logs a debug message during initialization' do + string_io = StringIO.new + configuration.logger = Logger.new(string_io) + + subject + + expect(string_io.string).to include('sentry: [Spotlight] initialized for url http://localhost:8969/stream') + end + describe '#endpoint' do it 'returs correct endpoint' do expect(subject.endpoint).to eq('/stream') @@ -39,4 +54,28 @@ expect(subject.conn.use_ssl?).to eq(false) end end + + describe '#send_data' do + it 'fails a maximum of three times and logs disable once' do + string_io = StringIO.new + configuration.logger = Logger.new(string_io) + configuration.logger.level = :debug + + allow(::Net::HTTP).to receive(:new).and_raise(Errno::ECONNREFUSED) + + 3.times do + expect do + subject.send_data(data) + end.to raise_error(Sentry::ExternalError) + end + + 3.times do + expect do + subject.send_data(data) + end.not_to raise_error + end + + expect(string_io.string.scan('sentry: [Spotlight] disabling because of too many request failures').size).to eq(1) + end + end end