Skip to content

Commit 6525d0b

Browse files
dingsdaxsl0thentr0pysolnicclaude
authored
feat: queue time capture for Rack (#2838)
* add configuration for capturing queue time default to true * Extract, normalize & add queue time to transaction * Test app for queue time in Sentry * Fix redos issue * move test app * Update CHANGELOG.md * Update CHANGELOG.md * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Neel Shah <neel.shah@sentry.io> * Delete sentry_queue_test.rb * ⏰👮🏻 * change name * remove upper bounds * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Peter Solnica <peter@solnica.online> * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Peter Solnica <peter@solnica.online> * fix(rack): Validate t= prefixed values in parse_request_start_header Non-numeric t= values like "t=invalid" or "t=" were silently converted to 0.0 by Ruby's String#to_f, resulting in a Unix epoch timestamp and a queue time of ~56 years instead of nil. Apply the same numeric regex guard already used for raw timestamps to the t= branch before calling to_f. Co-Authored-By: Claude <noreply@anthropic.com> * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Peter Solnica <peter@solnica.online> * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Peter Solnica <peter@solnica.online> * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Peter Solnica <peter@solnica.online> * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Peter Solnica <peter@solnica.online> * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Peter Solnica <peter@solnica.online> * Update sentry-ruby/lib/sentry/rack/capture_exceptions.rb Co-authored-by: Peter Solnica <peter@solnica.online> * fix(rack): Handle whitespace and comma-separated X-Request-Start values parse_request_start_header returned nil for headers that arrived with leading/trailing whitespace or as comma-separated values (multiple header occurrences collapsed by a proxy). Strip whitespace from the token and split on commas, taking the first (earliest) timestamp. Co-Authored-By: Claude <noreply@anthropic.com> * fix(rack): Coerce puma.request_body_wait to Float before comparison When env["puma.request_body_wait"] is a String (e.g. serialized by middleware), the `> 0` comparison raised ArgumentError, which was silently swallowed by the broad rescue, causing queue time to be dropped even with a valid X-Request-Start header. Call to_f on String values before the comparison. Numeric strings ("40") convert correctly; non-numeric strings ("N/A") become 0.0 and are treated as absent, so total_time_ms is returned unchanged. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Neel Shah <neel.shah@sentry.io> Co-authored-by: Peter Solnica <peter@solnica.online> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8edaa20 commit 6525d0b

File tree

6 files changed

+255
-1
lines changed

6 files changed

+255
-1
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
## Unreleased
22

3+
- Queue time capture for Rack ([#2838](https://github.com/getsentry/sentry-ruby/pull/2838))
4+
35
### Features
46

57
- Add support for OTLP ingestion in `sentry-opentelemetry` ([#2853](https://github.com/getsentry/sentry-ruby/pull/2853))

sentry-ruby/lib/sentry/configuration.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,12 @@ class Configuration
235235
# @return [Boolean]
236236
attr_accessor :send_default_pii
237237

238+
# Capture queue time from X-Request-Start header set by reverse proxies.
239+
# Works with any Rack app behind Nginx, HAProxy, Heroku router, etc.
240+
# Defaults to true.
241+
# @return [Boolean]
242+
attr_accessor :capture_queue_time
243+
238244
# Allow to skip Sentry emails within rake tasks
239245
# @return [Boolean]
240246
attr_accessor :skip_rake_integration
@@ -513,6 +519,7 @@ def initialize
513519
self.enable_backpressure_handling = false
514520
self.trusted_proxies = []
515521
self.dsn = ENV["SENTRY_DSN"]
522+
self.capture_queue_time = true
516523

517524
spotlight_env = ENV["SENTRY_SPOTLIGHT"]
518525
spotlight_bool = Sentry::Utils::EnvHelper.env_to_bool(spotlight_env, strict: true)

sentry-ruby/lib/sentry/rack/capture_exceptions.rb

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,14 @@ def start_transaction(env, scope)
7272
}
7373

7474
transaction = Sentry.continue_trace(env, **options)
75-
Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options)
75+
transaction = Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options)
76+
77+
# attach queue time if available
78+
if transaction && (queue_time = extract_queue_time(env))
79+
transaction.set_data(Span::DataConventions::HTTP_QUEUE_TIME_MS, queue_time)
80+
end
81+
82+
transaction
7683
end
7784

7885

@@ -87,6 +94,81 @@ def mechanism
8794
Sentry::Mechanism.new(type: MECHANISM_TYPE, handled: false)
8895
end
8996

97+
# Extracts queue time from the request environment.
98+
# Calculates the time (in milliseconds) the request spent waiting in the
99+
# web server queue before processing began.
100+
#
101+
# Subtracts puma.request_body_wait to account for time spent waiting for
102+
# slow clients to send the request body, isolating actual queue time.
103+
# See: https://github.com/puma/puma/blob/master/docs/architecture.md
104+
#
105+
# @param env [Hash] Rack env
106+
# @return [Float, nil] queue time in milliseconds or nil
107+
def extract_queue_time(env)
108+
return unless Sentry.configuration&.capture_queue_time
109+
110+
header_value = env["HTTP_X_REQUEST_START"]
111+
return unless header_value
112+
113+
request_start = parse_request_start_header(header_value)
114+
return unless request_start
115+
116+
total_time_ms = ((Time.now.to_f - request_start) * 1000).round(2)
117+
118+
# reject negative (clock skew between proxy & app server)
119+
return unless total_time_ms >= 0
120+
121+
puma_wait_ms = env["puma.request_body_wait"]
122+
puma_wait_ms = puma_wait_ms.to_f if puma_wait_ms.is_a?(String)
123+
124+
if puma_wait_ms && puma_wait_ms > 0
125+
queue_time_ms = total_time_ms - puma_wait_ms
126+
queue_time_ms >= 0 ? queue_time_ms : 0.0 # more sanity check
127+
else
128+
total_time_ms
129+
end
130+
rescue StandardError
131+
end
132+
133+
# Parses X-Request-Start header value to extract a timestamp.
134+
# Supports multiple formats:
135+
# - Nginx: "t=1234567890.123" (seconds with decimal)
136+
# - Heroku, HAProxy 1.9+: "t=1234567890123456" (microseconds)
137+
# - HAProxy < 1.9: "t=1234567890" (seconds)
138+
# - Generic: "1234567890.123" (raw timestamp)
139+
#
140+
# @param header_value [String] The X-Request-Start header value
141+
# @return [Float, nil] Timestamp in seconds since epoch or nil
142+
def parse_request_start_header(header_value)
143+
return unless header_value
144+
145+
# Take the first value if comma-separated (multiple headers collapsed by a proxy)
146+
# and strip surrounding whitespace from each token
147+
raw = header_value.split(",").first.to_s.strip
148+
149+
timestamp = if raw.start_with?("t=")
150+
value = raw[2..-1].strip
151+
return nil unless value.match?(/\A\d+(?:\.\d+)?\z/)
152+
value.to_f
153+
elsif raw.match?(/\A\d+(?:\.\d+)?\z/)
154+
raw.to_f
155+
else
156+
return
157+
end
158+
159+
# normalize: timestamps can be in seconds, milliseconds or microseconds
160+
# any timestamp > 10 trillion = microseconds
161+
if timestamp > 10_000_000_000_000
162+
timestamp / 1_000_000.0
163+
# timestamp > 10 billion & < 10 trillion = milliseconds
164+
elsif timestamp > 10_000_000_000
165+
timestamp / 1_000.0
166+
else
167+
timestamp # assume seconds
168+
end
169+
rescue StandardError
170+
end
171+
90172
def status_code_for_exception(exception)
91173
500
92174
end

sentry-ruby/lib/sentry/span.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ module DataConventions
4949
MESSAGING_DESTINATION_NAME = "messaging.destination.name"
5050
MESSAGING_MESSAGE_RECEIVE_LATENCY = "messaging.message.receive.latency"
5151
MESSAGING_MESSAGE_RETRY_COUNT = "messaging.message.retry.count"
52+
53+
# Time in ms the request spent in the server queue before processing began.
54+
HTTP_QUEUE_TIME_MS = "http.server.request.time_in_queue"
5255
end
5356

5457
STATUS_MAP = {

sentry-ruby/spec/sentry/configuration_spec.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,17 @@
329329
expect { subject.before_breadcrumb = true }.to raise_error(ArgumentError, "before_breadcrumb must be callable (or nil to disable)")
330330
end
331331

332+
describe "#capture_queue_time" do
333+
it "defaults to true" do
334+
expect(subject.capture_queue_time).to eq(true)
335+
end
336+
337+
it "can be set to false" do
338+
subject.capture_queue_time = false
339+
expect(subject.capture_queue_time).to eq(false)
340+
end
341+
end
342+
332343
context 'being initialized with a current environment' do
333344
before(:each) do
334345
subject.environment = 'test'

sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,155 @@ def will_be_sampled_by_sdk
660660
end
661661
end
662662

663+
describe "queue time capture" do
664+
let(:stack) do
665+
app = ->(_) { [200, {}, ['ok']] }
666+
Sentry::Rack::CaptureExceptions.new(app)
667+
end
668+
669+
before do
670+
perform_basic_setup do |config|
671+
config.traces_sample_rate = 1.0
672+
end
673+
end
674+
675+
let(:transaction) { last_sentry_event }
676+
677+
context "with X-Request-Start header" do
678+
it "attaches queue time to transaction" do
679+
timestamp = Time.now.to_f - 0.05 # 50ms ago
680+
env["HTTP_X_REQUEST_START"] = "t=#{timestamp}"
681+
682+
stack.call(env)
683+
684+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
685+
expect(queue_time).to be_within(10).of(50)
686+
end
687+
688+
it "subtracts puma.request_body_wait" do
689+
Timecop.freeze do
690+
timestamp = Time.now.to_f - 0.1 # 100ms ago
691+
env["HTTP_X_REQUEST_START"] = "t=#{timestamp}"
692+
env["puma.request_body_wait"] = 40 # 40ms waiting for client
693+
694+
stack.call(env)
695+
696+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
697+
expect(queue_time).to be_within(10).of(60) # 100 - 40
698+
end
699+
end
700+
701+
it "subtracts puma.request_body_wait when given as a string" do
702+
Timecop.freeze do
703+
timestamp = Time.now.to_f - 0.1 # 100ms ago
704+
env["HTTP_X_REQUEST_START"] = "t=#{timestamp}"
705+
env["puma.request_body_wait"] = "40"
706+
707+
stack.call(env)
708+
709+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
710+
expect(queue_time).to be_within(10).of(60) # 100 - 40
711+
end
712+
end
713+
714+
it "still returns queue time when puma.request_body_wait is a non-numeric string" do
715+
Timecop.freeze do
716+
timestamp = Time.now.to_f - 0.05 # 50ms ago
717+
env["HTTP_X_REQUEST_START"] = "t=#{timestamp}"
718+
env["puma.request_body_wait"] = "N/A"
719+
720+
stack.call(env)
721+
722+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
723+
expect(queue_time).to be_within(10).of(50)
724+
end
725+
end
726+
727+
it "handles different timestamp formats" do
728+
# Heroku/HAProxy microseconds format
729+
timestamp_us = ((Time.now.to_f - 0.03) * 1_000_000).to_i
730+
env["HTTP_X_REQUEST_START"] = "t=#{timestamp_us}"
731+
732+
stack.call(env)
733+
734+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
735+
expect(queue_time).to be_within(10).of(30)
736+
end
737+
end
738+
739+
context "without X-Request-Start header" do
740+
it "doesn't add queue time data" do
741+
stack.call(env)
742+
743+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
744+
expect(queue_time).to be_nil
745+
end
746+
end
747+
748+
context "with invalid header" do
749+
it "doesn't add queue time data" do
750+
env["HTTP_X_REQUEST_START"] = "invalid"
751+
752+
stack.call(env)
753+
754+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
755+
expect(queue_time).to be_nil
756+
end
757+
758+
it "doesn't add queue time data for malformed t= values" do
759+
["t=invalid", "t=abc", "t="].each do |bad_value|
760+
env["HTTP_X_REQUEST_START"] = bad_value
761+
762+
stack.call(env)
763+
764+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
765+
expect(queue_time).to be_nil, "expected nil for header #{bad_value.inspect}"
766+
end
767+
end
768+
769+
it "handles leading and trailing whitespace" do
770+
Timecop.freeze do
771+
timestamp = Time.now.to_f - 0.05
772+
env["HTTP_X_REQUEST_START"] = " t=#{timestamp} "
773+
774+
stack.call(env)
775+
776+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
777+
expect(queue_time).to be_within(10).of(50)
778+
end
779+
end
780+
781+
it "uses the first value when header is comma-separated" do
782+
Timecop.freeze do
783+
earlier = Time.now.to_f - 0.08
784+
later = Time.now.to_f - 0.02
785+
env["HTTP_X_REQUEST_START"] = "t=#{earlier}, t=#{later}"
786+
787+
stack.call(env)
788+
789+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
790+
expect(queue_time).to be_within(10).of(80)
791+
end
792+
end
793+
end
794+
795+
context "when capture_queue_time is disabled" do
796+
before do
797+
Sentry.configuration.capture_queue_time = false
798+
end
799+
800+
it "doesn't capture queue time" do
801+
timestamp = Time.now.to_f - 0.05
802+
env["HTTP_X_REQUEST_START"] = "t=#{timestamp}"
803+
804+
stack.call(env)
805+
806+
queue_time = transaction.contexts.dig(:trace, :data, 'http.server.request.time_in_queue')
807+
expect(queue_time).to be_nil
808+
end
809+
end
810+
end
811+
663812
describe "tracing without performance" do
664813
let(:incoming_prop_context) { Sentry::PropagationContext.new(Sentry::Scope.new) }
665814
let(:env) do

0 commit comments

Comments
 (0)