diff --git a/.gitignore b/.gitignore index 83704eaa9..f5a971c3b 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ Gemfile.lock .idea *.rdb .rgignore +.claude node_modules .vite diff --git a/CHANGELOG.md b/CHANGELOG.md index 82e618efc..6bb6a6d38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,21 @@ - Remove stacktrace trimming ([#2714](https://github.com/getsentry/sentry-ruby/pull/2714)) - `config.enabled_environments` now defaults to `nil` instead of `[]` for sending to all environments ([#2716](https://github.com/getsentry/sentry-ruby/pull/2716)) - Remove `:monotonic_active_support_logger` from `config.breadcrumbs_logger` ([#2717](https://github.com/getsentry/sentry-ruby/pull/2717)) +- Requests which have response status codes in the inclusive ranges `[(301..303), (305..399), (401..404)]` will no longer create transactions by default. See `config.trace_ignore_status_codes` below to control what gets traced. + +### Features + +- Add `config.trace_ignore_status_codes` to control which response codes to ignore for tracing ([#2725](https://github.com/getsentry/sentry-ruby/pull/2725)) + + You can pass in an Array of individual status codes or ranges of status codes. + + ```ruby + Sentry.init do |config| + # ... + # will ignore 404, 501, 502, 503 + config.trace_ignore_status_codes = [404, (501..503)] + end + ``` ### Internal diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 4bdfad6c9..9b4aae768 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -328,6 +328,18 @@ def logger # @return [Array] attr_accessor :trace_propagation_targets + # Collection of HTTP status codes or ranges of codes to ignore when tracing incoming requests. + # If a transaction's http.response.status_code matches one of these values, + # the transaction will be dropped and marked as not sampled. + # Defaults to TRACE_IGNORE_STATUS_CODES_DEFAULT. + # + # @example + # # ignore 404 and 502 <= status_code <= 511 + # config.trace_ignore_status_codes = [404, (502..511)] + # + # @return [Array, Array] + attr_reader :trace_ignore_status_codes + # The instrumenter to use, :sentry or :otel # @return [Symbol] attr_reader :instrumenter @@ -379,6 +391,8 @@ def logger SERVER_PORT ].freeze + TRACE_IGNORE_STATUS_CODES_DEFAULT = [(301..303), (305..399), (401..404)] + HEROKU_DYNO_METADATA_MESSAGE = "You are running on Heroku but haven't enabled Dyno Metadata. For Sentry's "\ "release detection to work correctly, please run `heroku labs:enable runtime-dyno-metadata`" @@ -483,6 +497,7 @@ def initialize self.server_name = server_name_from_env self.instrumenter = :sentry self.trace_propagation_targets = [PROPAGATION_TARGETS_MATCH_ALL] + self.trace_ignore_status_codes = TRACE_IGNORE_STATUS_CODES_DEFAULT self.enabled_patches = DEFAULT_PATCHES.dup self.before_send = nil @@ -582,6 +597,14 @@ def instrumenter=(instrumenter) @instrumenter = INSTRUMENTERS.include?(instrumenter) ? instrumenter : :sentry end + def trace_ignore_status_codes=(codes) + unless codes.is_a?(Array) && codes.all? { |code| valid_status_code_entry?(code) } + raise ArgumentError, "trace_ignore_status_codes must be an Array of integers or ranges between (100-599) where begin <= end" + end + + @trace_ignore_status_codes = codes + end + def enable_tracing=(enable_tracing) unless enable_tracing.nil? log_warn <<~MSG @@ -788,6 +811,23 @@ def processor_count available_processor_count = Concurrent.available_processor_count if Concurrent.respond_to?(:available_processor_count) available_processor_count || Concurrent.processor_count end + + def valid_http_status_code?(code) + code.is_a?(Integer) && code >= 100 && code <= 599 + end + + def valid_status_code_entry?(entry) + case entry + when Integer + valid_http_status_code?(entry) + when Range + valid_http_status_code?(entry.begin) && + valid_http_status_code?(entry.end) && + entry.begin <= entry.end + else + false + end + end end class StructuredLoggingConfiguration diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index 9274b6497..31a84cfd2 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/sentry-ruby/lib/sentry/transaction.rb @@ -81,6 +81,7 @@ def initialize( @tracing_enabled = hub.configuration.tracing_enabled? @traces_sampler = hub.configuration.traces_sampler @traces_sample_rate = hub.configuration.traces_sample_rate + @trace_ignore_status_codes = hub.configuration.trace_ignore_status_codes @sdk_logger = hub.configuration.sdk_logger @release = hub.configuration.release @environment = hub.configuration.environment @@ -287,7 +288,15 @@ def finish(hub: nil, end_timestamp: nil) @hub.stop_profiler!(self) - if @sampled + if @sampled && ignore_status_code? + @sampled = false + + status_code = get_http_status_code + log_debug("#{MESSAGE_PREFIX} Discarding #{generate_transaction_description} due to ignored HTTP status code: #{status_code}") + + @hub.current_client.transport.record_lost_event(:event_processor, "transaction") + @hub.current_client.transport.record_lost_event(:event_processor, "span") + elsif @sampled event = hub.current_client.event_from_transaction(self) hub.capture_event(event) else @@ -371,6 +380,21 @@ def populate_head_baggage @baggage = Baggage.new(items, mutable: false) end + def ignore_status_code? + return false unless @trace_ignore_status_codes + + status_code = get_http_status_code + return false unless status_code + + @trace_ignore_status_codes.any? do |ignored| + ignored.is_a?(Range) ? ignored.include?(status_code) : status_code == ignored + end + end + + def get_http_status_code + @data && @data[Span::DataConventions::HTTP_STATUS_CODE] + end + class SpanRecorder attr_reader :max_length, :spans diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index 5bc0818c2..9c62d8e3c 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -772,4 +772,44 @@ class SentryConfigurationSample < Sentry::Configuration }.to output(/`config.logger=` is deprecated/).to_stderr end end + + describe "#trace_ignore_status_codes" do + it "has default values" do + expect(subject.trace_ignore_status_codes).to eq([(301..303), (305..399), (401..404)]) + end + + it "can be configured with individual status codes" do + subject.trace_ignore_status_codes = [404, 500] + expect(subject.trace_ignore_status_codes).to eq([404, 500]) + end + + it "can be configured with ranges" do + subject.trace_ignore_status_codes = [(300..399), (500..599)] + expect(subject.trace_ignore_status_codes).to eq([(300..399), (500..599)]) + end + + it "can be configured with mixed individual codes and ranges" do + subject.trace_ignore_status_codes = [404, (500..599)] + expect(subject.trace_ignore_status_codes).to eq([404, (500..599)]) + end + + it "raises ArgumentError when not an Array" do + expect { subject.trace_ignore_status_codes = 404 }.to raise_error(ArgumentError, /must be an Array/) + expect { subject.trace_ignore_status_codes = "404" }.to raise_error(ArgumentError, /must be an Array/) + end + + it "raises ArgumentError for invalid status codes" do + expect { subject.trace_ignore_status_codes = [99] }.to raise_error(ArgumentError, /must be.* between \(100-599\)/) + expect { subject.trace_ignore_status_codes = [600] }.to raise_error(ArgumentError, /must be.* between \(100-599\)/) + expect { subject.trace_ignore_status_codes = ["404"] }.to raise_error(ArgumentError, /must be an Array of integers/) + end + + it "raises ArgumentError for invalid ranges" do + expect { subject.trace_ignore_status_codes = [[400]] }.to raise_error(ArgumentError, /must be.* ranges/) + expect { subject.trace_ignore_status_codes = [[400, 500, 600]] }.to raise_error(ArgumentError, /must be.* ranges/) + expect { subject.trace_ignore_status_codes = [[500, 400]] }.to raise_error(ArgumentError, /must be.* begin <= end/) + expect { subject.trace_ignore_status_codes = [[99, 200]] }.to raise_error(ArgumentError, /must be.* between \(100-599\)/) + expect { subject.trace_ignore_status_codes = [[400, 600]] }.to raise_error(ArgumentError, /must be.* between \(100-599\)/) + end + end end diff --git a/sentry-ruby/spec/sentry/transaction_spec.rb b/sentry-ruby/spec/sentry/transaction_spec.rb index 60b9ed14b..a70eb3aed 100644 --- a/sentry-ruby/spec/sentry/transaction_spec.rb +++ b/sentry-ruby/spec/sentry/transaction_spec.rb @@ -18,6 +18,9 @@ ) end + let(:string_io) { StringIO.new } + let(:sdk_logger) { Logger.new(string_io) } + describe ".from_sentry_trace" do let(:sentry_trace) { subject.to_sentry_trace } @@ -197,14 +200,9 @@ end describe "#set_initial_sample_decision" do - let(:string_io) { StringIO.new } - let(:logger) do - ::Logger.new(string_io) - end - before do perform_basic_setup do |config| - config.sdk_logger = logger + config.sdk_logger = sdk_logger end end @@ -556,6 +554,53 @@ ) end end + + describe "with trace_ignore_status_codes" do + before do + perform_basic_setup do |config| + config.traces_sample_rate = 1.0 + config.sdk_logger = sdk_logger + config.trace_ignore_status_codes = [404, (500..503)] + end + end + + it "drops transaction with individual status code match" do + subject.set_http_status(404) + subject.finish + + expect(sentry_events).to be_empty + expect(Sentry.get_current_client.transport).to have_recorded_lost_event(:event_processor, 'transaction') + expect(Sentry.get_current_client.transport).to have_recorded_lost_event(:event_processor, 'span') + expect(string_io.string).to include("Discarding") + expect(string_io.string).to include("due to ignored HTTP status code: 404") + end + + it "drops transaction with range status code match" do + subject.set_http_status(502) + subject.finish + + expect(sentry_events).to be_empty + expect(Sentry.get_current_client.transport).to have_recorded_lost_event(:event_processor, 'transaction') + expect(Sentry.get_current_client.transport).to have_recorded_lost_event(:event_processor, 'span') + expect(string_io.string).to include("Discarding") + expect(string_io.string).to include("due to ignored HTTP status code: 502") + end + + it "does not drop transaction with status code outside ignore list" do + subject.set_http_status(200) + subject.finish + + expect(sentry_events).not_to be_empty + expect(sentry_events.last).to be_a(Sentry::TransactionEvent) + end + + it "does not drop transaction without a status code set" do + subject.finish + + expect(sentry_events).not_to be_empty + expect(sentry_events.first).to be_a(Sentry::TransactionEvent) + end + end end describe "#get_baggage" do