-
Notifications
You must be signed in to change notification settings - Fork 375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[rack] add request queuing option to the Rack middleware #272
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
635874d
[rack] add request queuing option to the Rack middleware; compatibili…
50d75d2
[rack] provide request queue documentation
275d5f9
[wip] add debug logs
9f57afe
Support alternative header `X-Queue-Start`
p-lambert a5d3dfd
Changed: 'enqueuing' --> 'queuing' in Rack.
delner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
module Datadog | ||
module Contrib | ||
module Rack | ||
# QueueTime simply... | ||
module QueueTime | ||
REQUEST_START = 'HTTP_X_REQUEST_START'.freeze | ||
QUEUE_START = 'HTTP_X_QUEUE_START'.freeze | ||
|
||
module_function | ||
|
||
def get_request_start(env, now = Time.now.utc) | ||
header = env[REQUEST_START] || env[QUEUE_START] | ||
return unless header | ||
|
||
# nginx header is in the format "t=1512379167.574" | ||
# TODO: this should be generic enough to work with any | ||
# frontend web server or load balancer | ||
time_string = header.split('t=')[1] | ||
return if time_string.nil? | ||
|
||
# return the request_start only if it's lesser than | ||
# current time, to avoid significant clock skew | ||
request_start = Time.at(time_string.to_f) | ||
request_start.utc > now ? nil : request_start | ||
rescue StandardError => e | ||
# in case of an Exception we don't create a | ||
# `request.queuing` span | ||
Datadog::Tracer.log.debug("[rack] unable to parse request queue headers: #{e}") | ||
nil | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
require 'ddtrace/contrib/rack/request_queue' | ||
|
||
class QueueTimeParserTest < Minitest::Test | ||
include Rack::Test::Methods | ||
|
||
def test_nginx_header | ||
# ensure nginx headers are properly parsed | ||
headers = {} | ||
headers['HTTP_X_REQUEST_START'] = 't=1512379167.574' | ||
request_start = Datadog::Contrib::Rack::QueueTime.get_request_start(headers) | ||
assert_equal(1512379167.574, request_start.to_f) | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
require 'contrib/rack/helpers' | ||
|
||
class RequestQueuingTest < RackBaseTest | ||
def setup | ||
super | ||
# enable request_queuing | ||
Datadog.configuration[:rack][:request_queuing] = true | ||
end | ||
|
||
def test_request_queuing_header | ||
# ensure a queuing Span is created if the header is available | ||
request_start = (Time.now.utc - 5).to_i | ||
header 'x-request-start', "t=#{request_start}" | ||
get '/success/' | ||
assert last_response.ok? | ||
|
||
spans = @tracer.writer.spans | ||
assert_equal(2, spans.length) | ||
|
||
rack_span = spans.find { |s| s.name == 'rack.request' } | ||
frontend_span = spans.find { |s| s.name == 'http_server.queue' } | ||
refute_nil(rack_span) | ||
refute_nil(frontend_span) | ||
|
||
assert_equal('http', rack_span.span_type) | ||
assert_equal('rack', rack_span.service) | ||
assert_equal('GET 200', rack_span.resource) | ||
assert_equal('GET', rack_span.get_tag('http.method')) | ||
assert_equal('200', rack_span.get_tag('http.status_code')) | ||
assert_equal('/success/', rack_span.get_tag('http.url')) | ||
assert_equal(0, rack_span.status) | ||
assert_equal(frontend_span.span_id, rack_span.parent_id) | ||
|
||
assert_equal('web-server', frontend_span.service) | ||
assert_equal(request_start, frontend_span.start_time.to_i) | ||
end | ||
|
||
def test_request_alternative_queuing_header | ||
# ensure a queuing Span is created if the header is available | ||
request_start = (Time.now.utc - 5).to_i | ||
header 'x-queue-start', "t=#{request_start}" | ||
get '/success/' | ||
assert last_response.ok? | ||
|
||
spans = @tracer.writer.spans | ||
assert_equal(2, spans.length) | ||
|
||
rack_span = spans.find { |s| s.name == 'rack.request' } | ||
frontend_span = spans.find { |s| s.name == 'http_server.queue' } | ||
refute_nil(rack_span) | ||
refute_nil(frontend_span) | ||
|
||
assert_equal('http', rack_span.span_type) | ||
assert_equal('rack', rack_span.service) | ||
assert_equal('GET 200', rack_span.resource) | ||
assert_equal('GET', rack_span.get_tag('http.method')) | ||
assert_equal('200', rack_span.get_tag('http.status_code')) | ||
assert_equal('/success/', rack_span.get_tag('http.url')) | ||
assert_equal(0, rack_span.status) | ||
assert_equal(frontend_span.span_id, rack_span.parent_id) | ||
|
||
assert_equal('web-server', frontend_span.service) | ||
assert_equal(request_start, frontend_span.start_time.to_i) | ||
end | ||
|
||
def test_request_queuing_service_name | ||
# ensure a queuing Span is created if the header is available | ||
Datadog.configuration[:rack][:web_service_name] = 'nginx' | ||
request_start = (Time.now.utc - 5).to_i | ||
header 'x-request-start', "t=#{request_start}" | ||
get '/success/' | ||
assert last_response.ok? | ||
|
||
spans = @tracer.writer.spans | ||
assert_equal(2, spans.length) | ||
|
||
rack_span = spans.find { |s| s.name == 'rack.request' } | ||
frontend_span = spans.find { |s| s.name == 'http_server.queue' } | ||
refute_nil(rack_span) | ||
refute_nil(frontend_span) | ||
|
||
assert_equal('nginx', frontend_span.service) | ||
end | ||
|
||
def test_clock_skew | ||
# ensure a queuing Span is NOT created if there is a clock skew | ||
# where the starting time is greater than current host Time.now | ||
request_start = (Time.now.utc + 5).to_i | ||
header 'x-request-start', "t=#{request_start}" | ||
get '/success/' | ||
assert last_response.ok? | ||
|
||
spans = @tracer.writer.spans | ||
assert_equal(1, spans.length) | ||
|
||
rack_span = spans[0] | ||
assert_equal('rack.request', rack_span.name) | ||
end | ||
|
||
def test_wrong_header | ||
# ensure a queuing Span is NOT created if the header is wrong | ||
header 'x-request-start', 'something_weird' | ||
get '/success/' | ||
assert last_response.ok? | ||
|
||
spans = @tracer.writer.spans | ||
assert_equal(1, spans.length) | ||
|
||
rack_span = spans[0] | ||
assert_equal('rack.request', rack_span.name) | ||
end | ||
|
||
def test_enabled_missing_header | ||
# ensure a queuing Span is NOT created if the header is missing | ||
get '/success/' | ||
assert last_response.ok? | ||
|
||
spans = @tracer.writer.spans | ||
assert_equal(1, spans.length) | ||
|
||
rack_span = spans[0] | ||
assert_equal('rack.request', rack_span.name) | ||
end | ||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider moving this to the core library in a future pull request. The behavior here is generic enough, and useful for other integrations.