Skip to content
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
10 changes: 2 additions & 8 deletions sentry-rails/lib/sentry/rails/active_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,7 @@ def log_producer_span_error(e)
end

def record(job, trace_headers: nil, user: nil, &block)
# Always give this thread a fresh hub cloned from the main hub so
# the job's events are fully isolated. Save and restore whatever
# hub was on the thread before (e.g. the Rack request hub set by
# CaptureExceptions, or a stale hub left by a recycled thread-pool
# thread) so the outer context continues working correctly after
# the job finishes.
original_hub = Thread.current.thread_variable_get(Sentry::THREAD_LOCAL)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Was this comment inaccurate and that's why it got removed?

@sl0thentr0py sl0thentr0py Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we should generally not have this clanker generated comment bloat, so I will remove it wherever I see it unless it really adds any value.

original_hub = Sentry.get_current_hub_internal
Sentry.clone_hub_to_current_thread

Sentry.with_scope do |scope|
Expand Down Expand Up @@ -171,7 +165,7 @@ def record(job, trace_headers: nil, user: nil, &block)
end
end
ensure
Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, original_hub)
Sentry.set_current_hub_internal(original_hub)
end

def set_messaging_data(transaction, job)
Expand Down
42 changes: 37 additions & 5 deletions sentry-ruby/lib/sentry-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ def init(&block)
client = Client.new(config)
scope = Scope.new(max_breadcrumbs: config.max_breadcrumbs)
hub = Hub.new(client, scope)
Thread.current.thread_variable_set(THREAD_LOCAL, hub)
@hub_isolation_level = config.hub_isolation_level
set_current_hub_internal(hub)
@main_hub = hub
@background_worker = Sentry::BackgroundWorker.new(config)
@session_flusher = config.session_tracking? ? Sentry::SessionFlusher.new(config, client) : nil
Expand Down Expand Up @@ -309,7 +310,7 @@ def close

MUTEX.synchronize do
@main_hub = nil
Thread.current.thread_variable_set(THREAD_LOCAL, nil)
set_current_hub_internal(nil)
end
end

Expand Down Expand Up @@ -362,7 +363,7 @@ def get_current_hub
# ideally, we should do this proactively whenever a new thread is created
# but it's impossible for the SDK to keep track every new thread
# so we need to use this rather passive way to make sure the app doesn't crash
Thread.current.thread_variable_get(THREAD_LOCAL) || clone_hub_to_current_thread
get_current_hub_internal || clone_hub_to_current_thread
end

# Returns the current active client.
Expand All @@ -380,12 +381,14 @@ def get_current_scope
get_current_hub.current_scope
end

# Clones the main thread's active hub and stores it to the current thread.
# Clones the main hub and stores it for the current execution context
# (the current thread, or the current fiber when +config.hub_isolation_level+
# is +:fiber+).
#
# @return [void]
def clone_hub_to_current_thread
return unless initialized?
Thread.current.thread_variable_set(THREAD_LOCAL, get_main_hub.clone)
set_current_hub_internal(get_main_hub.clone)
end

# Takes a block and yields the current active scope.
Expand Down Expand Up @@ -737,6 +740,35 @@ def utc_now
def dependency_installed?(name)
Object.const_defined?(name)
end

# Reads the hub stored for the current execution context. The active
# isolation level (cached from +config.hub_isolation_level+ at init) decides
# whether that context is the current thread or the current fiber. Reads the
# cached level rather than the configuration to avoid recursing back through
# hub resolution.
#
# @!visibility private
# @return [Hub, nil]
def get_current_hub_internal
if @hub_isolation_level == :fiber
::Fiber[THREAD_LOCAL]
else
::Thread.current.thread_variable_get(THREAD_LOCAL)
end
end

# Stores +hub+ for the current execution context (thread or fiber).
#
# @!visibility private
# @param hub [Hub, nil]
# @return [Hub, nil]
def set_current_hub_internal(hub)
if @hub_isolation_level == :fiber
::Fiber[THREAD_LOCAL] = hub
else
::Thread.current.thread_variable_set(THREAD_LOCAL, hub)
end
end
end
end

Expand Down
39 changes: 39 additions & 0 deletions sentry-ruby/lib/sentry/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,23 @@ class Configuration
# @return [Boolean]
attr_accessor :strict_trace_continuation

# Which execution primitive owns the SDK's current hub.
#
# [+:thread+ (default)] Store the hub in thread-local storage. Correct for
# thread-based servers (Puma, Unicorn) and background processors (Sidekiq,
# Resque). Every fiber on a thread shares one hub.
# [+:fiber+] Store the hub in Fiber Storage (Ruby 3.2+). Each fiber gets its
# own hub and child fibers inherit it, so concurrent requests on a
# fiber-based server (Falcon/async) are isolated instead of sharing and
# corrupting one another's scope. Requested on a Ruby without Fiber
# Storage (< 3.2), the SDK logs a warning and falls back to +:thread+.
#
# @return [Symbol]
attr_reader :hub_isolation_level

# Isolation levels the SDK understands for hub storage.
ISOLATION_LEVELS = %i[thread fiber].freeze

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit and could be a follow-up mini-refactor, but: we have a validate macro - we could extend it to support included_in: [...] and then overriding the writer would only be needed for the situation where the Fiber storage API is not available.


# these are not config options
# @!visibility private
attr_reader :errors, :gem_specs
Expand Down Expand Up @@ -541,6 +558,7 @@ def initialize
self.capture_queue_time = true
self.org_id = nil
self.strict_trace_continuation = false
self.hub_isolation_level = :thread

spotlight_env = ENV["SENTRY_SPOTLIGHT"]
spotlight_bool = Sentry::Utils::EnvHelper.env_to_bool(spotlight_env, strict: true)
Expand Down Expand Up @@ -610,6 +628,23 @@ def release=(value)
@release = value
end

def hub_isolation_level=(level)
level = level.to_sym if level.respond_to?(:to_sym)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What's the reason for allowing non-symbols over here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just syntax sugar


unless ISOLATION_LEVELS.include?(level)
raise ArgumentError, "hub_isolation_level must be one of #{ISOLATION_LEVELS.inspect}, got #{level.inspect}"
end

# :fiber relies on Fiber Storage (Ruby 3.2+); downgrade so the hub is never
# asked to call Fiber[] on a Ruby that lacks it.
if level == :fiber && !fiber_storage_available?
log_warn("hub_isolation_level :fiber requires Ruby 3.2+ Fiber Storage; falling back to :thread on Ruby #{RUBY_VERSION}.")
level = :thread
end

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We could completely avoid that and define writer method dynamically depending on the availability, which would simplify runtime production code path.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we still want to have the warning message if they try to set it, so this needs to stay


@hub_isolation_level = level
end
Comment thread
cursor[bot] marked this conversation as resolved.

def breadcrumbs_logger=(logger)
loggers =
if logger.is_a?(Array)
Expand Down Expand Up @@ -806,6 +841,10 @@ def run_after_close_callbacks

private

def fiber_storage_available?
::Fiber.respond_to?(:[]) && ::Fiber.respond_to?(:[]=)
end

def init_dsn(dsn_string)
return if dsn_string.nil? || dsn_string.empty?

Expand Down
6 changes: 3 additions & 3 deletions sentry-ruby/lib/sentry/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ def setup_sentry_test(&block)
test_client = Sentry::Client.new(dummy_config.dup)
main_hub.bind_client(test_client)

# Realign the current thread's hub with the main hub so direct
# Realign the current execution context's hub with the main hub so direct
# `sentry_events` reads and any hub the Rack middleware clones from the
# main hub all observe the same DummyTransport.
Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, main_hub)
Sentry.set_current_hub_internal(main_hub)
end

# Clears all stored events and envelopes.
Expand Down Expand Up @@ -165,7 +165,7 @@ def reset_sentry_globals!
Sentry.instance_variable_set(:"@#{var}", nil)
end

Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, nil)
Sentry.set_current_hub_internal(nil)
end
end
end
Expand Down
36 changes: 36 additions & 0 deletions sentry-ruby/spec/sentry/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,42 @@ class SentryConfigurationSample < Sentry::Configuration
end
end

describe "#hub_isolation_level" do
it "defaults to :thread" do
expect(subject.hub_isolation_level).to eq(:thread)
end

it "accepts :thread" do
subject.hub_isolation_level = :thread
expect(subject.hub_isolation_level).to eq(:thread)
end
Comment thread
cursor[bot] marked this conversation as resolved.

it "raises ArgumentError for an unknown level" do
expect { subject.hub_isolation_level = :process }
.to raise_error(ArgumentError, /hub_isolation_level must be one of/)
end

context "when Fiber storage is available", when: { fiber_storage?: [] } do
it "accepts :fiber" do
subject.hub_isolation_level = :fiber
expect(subject.hub_isolation_level).to eq(:fiber)
end

it "coerces string values" do
subject.hub_isolation_level = "fiber"
expect(subject.hub_isolation_level).to eq(:fiber)
end
end

context "when Fiber storage is unavailable", when: { no_fiber_storage?: [] } do
it "falls back to :thread with a warning" do
expect(subject).to receive(:log_warn).with(/requires Ruby 3\.2\+ Fiber Storage/)
subject.hub_isolation_level = :fiber
expect(subject.hub_isolation_level).to eq(:thread)
end
end
end

describe "#validate" do
it "logs a warning if StackProf is not installed" do
allow(Sentry).to receive(:dependency_installed?).with(:StackProf).and_return(false)
Expand Down
60 changes: 60 additions & 0 deletions sentry-ruby/spec/sentry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,66 @@
end
end

describe "fiber isolation", when: { fiber_storage?: [] } do
before do
perform_basic_setup { |config| config.hub_isolation_level = :fiber }
end

after do
described_class.set_current_hub_internal(nil)
described_class.instance_variable_set(:@hub_isolation_level, :thread)
end

# Regression for cross-request contamination on fiber-based servers (Falcon,
# async), where many concurrent requests run as sibling fibers on one thread.
# With thread-local storage they share a single hub, so a scope opened by one
# request and held across a reactor yield is visible to (and clobbered by) the
# others. Fiber storage gives each request fiber its own hub.
it "keeps each sibling fiber's scope isolated across a yield" do
transport = described_class.get_main_hub.current_client.transport
transport.events.clear

requests = 3.times.map do |i|
Fiber.new do
described_class.clone_hub_to_current_thread
described_class.configure_scope { |scope| scope.set_user(id: i) }
Fiber.yield # simulate yielding to the reactor mid-request
described_class.capture_message(i.to_s)
end
end

requests.each(&:resume) # every request sets its user, then yields
requests.each(&:resume) # every request now captures its event

attributed = transport.events.to_h { |e| [e.message.to_i, e.user[:id]] }
expect(attributed).to eq({ 0 => 0, 1 => 1, 2 => 2 })
end

it "lets a child fiber inherit the parent request's hub" do
described_class.clone_hub_to_current_thread
parent_hub = described_class.get_current_hub

inherited = Fiber.new { described_class.get_current_hub }.resume

expect(inherited).to eq(parent_hub)
end

it "stores the hub in a fiber variable (instead of a thread variable)" do
described_class.set_tags(outside_fiber: true)

fiber = Fiber.new do
described_class.clone_hub_to_current_thread
described_class.set_tags(inside_fiber: true)
described_class.get_current_scope.tags
end

inside_tags = fiber.resume

expect(inside_tags).to eq({ outside_fiber: true, inside_fiber: true })
expect(described_class.get_current_scope.tags).to eq({ outside_fiber: true })
end
end

shared_examples "capture_helper" do
context "with sending_allowed? condition" do
before do
Expand Down
8 changes: 8 additions & 0 deletions sentry-ruby/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ def self.ruby_version?(op, version)
RUBY_VERSION.public_send(op, version)
end

def self.fiber_storage?
Fiber.respond_to?(:[]) && Fiber.respond_to?(:[]=)
end

def self.no_fiber_storage?
!fiber_storage?
end

def self.ruby_engine?(engine)
RUBY_ENGINE == engine
end
Expand Down
Loading