Skip to content

Commit

Permalink
adds tracing to rack requests and basic testing
Browse files Browse the repository at this point in the history
  • Loading branch information
zarirhamza committed Jan 11, 2024
1 parent 9353f02 commit 118bc82
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 14 deletions.
2 changes: 1 addition & 1 deletion lib/datadog/tracing/contrib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def endpoint_run(name, start, finish, id, payload)

span.set_error(payload[:exception_object]) if exception_is_error?(payload[:exception_object])

integration_route = endpoint.env['grape.routing_args'][:route_info].pattern.path
integration_route = endpoint.env['grape.routing_args'][:route_info].pattern.origin
Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route]

# override the current span with this notification values
Expand Down
20 changes: 8 additions & 12 deletions lib/datadog/tracing/contrib/rack/middlewares.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ def compute_queue_time(env)
end
end

# rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/PerceivedComplexity
# rubocop:disable Metrics/MethodLength
def call(env)
# Find out if this is rack within rack
previous_request_span = env[Ext::RACK_ENV_REQUEST_SPAN]
Expand Down Expand Up @@ -102,20 +105,16 @@ def call(env)
# call the rest of the stack
status, headers, response = @app.call(env)

# TODO: might be wrong if say rails routed to sinatra but sinatra
# fails to find a route, so I dumbly checked for 404?
if status != 404 && (routed = Thread.current[:datadog_http_routing].last)
# TODO: array == bad, this should be a Struct
last_script_name = routed[1]
last_route = routed[2]

# TODO: I seem to have gathered that in some (all?) cases this
# should be nil when no route is found, which woudl cater for the
# note above
if last_route
composite_route = last_script_name + last_route
request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, composite_route)
if last_script_name == '' && env['SCRIPT_NAME'] != ''
last_route = last_script_name
last_script_name = env['PATH_INFO']
end

request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, last_script_name + last_route) if last_route
end

[status, headers, response]
Expand Down Expand Up @@ -154,9 +153,6 @@ def call(env)
# rubocop:enable Lint/RescueException

# rubocop:disable Metrics/AbcSize
# rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/PerceivedComplexity
# rubocop:disable Metrics/MethodLength
def set_request_tags!(trace, request_span, env, status, headers, response, original_env)
request_header_collection = Header::RequestHeaderCollection.new(env)

Expand Down
3 changes: 2 additions & 1 deletion lib/datadog/tracing/contrib/rails/patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module Rails
module JourneyRouterPatch
def find_routes(*args, **kwargs)
result = super
integration_route = result.first[2].path.spec.to_s if result.any?
integration_route = result.first[2].path.spec.to_s.gsub(/\(\.:format\)/, '') if result.any?
Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route]
result
end
Expand Down Expand Up @@ -52,6 +52,7 @@ def before_initialize(app)
# Sometimes we don't want to activate middleware e.g. OpenTracing, etc.
add_middleware(app) if Datadog.configuration.tracing[:rails][:middleware]

# Check rails version if this works
ActionDispatch::Journey::Router.prepend(JourneyRouterPatch)

Rails::LogInjection.configure_log_tags(app.config)
Expand Down
4 changes: 4 additions & 0 deletions spec/datadog/tracing/contrib/rails/rack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ def internal_server_error
expect(request_span.service).to eq(tracer.default_service)
expect(request_span.resource).to eq('TestController#full')
expect(request_span.get_tag('http.url')).to eq('/full')
expect(request_span.get_tag('http.route')).to eq('/full')
expect(request_span.get_tag('http.method')).to eq('GET')
expect(request_span.get_tag('http.status_code')).to eq('200')
expect(request_span).to be_measured
Expand Down Expand Up @@ -379,6 +380,7 @@ def internal_server_error
expect(request_span.span_type).to eq('web')
expect(request_span.resource).to eq('TestController#error')
expect(request_span.get_tag('http.url')).to eq('/error')
expect(request_span.get_tag('http.route')).to eq('/error')
expect(request_span.get_tag('http.method')).to eq('GET')
expect(request_span.get_tag('http.status_code')).to eq('500')
expect(request_span).to have_error
Expand Down Expand Up @@ -414,6 +416,7 @@ def internal_server_error
expect(request_span.span_type).to eq('web')
expect(request_span.resource).to eq('TestController#soft_error')
expect(request_span.get_tag('http.url')).to eq('/soft_error')
expect(request_span.get_tag('http.route')).to eq('/soft_error')
expect(request_span.get_tag('http.method')).to eq('GET')
expect(request_span.get_tag('http.status_code')).to eq('520')
expect(request_span).to have_error
Expand Down Expand Up @@ -449,6 +452,7 @@ def internal_server_error
expect(request_span.span_type).to eq('web')
expect(request_span.resource).to eq('TestController#sub_error')
expect(request_span.get_tag('http.url')).to eq('/sub_error')
expect(request_span.get_tag('http.route')).to eq('/sub_error')
expect(request_span.get_tag('http.method')).to eq('GET')
expect(request_span.get_tag('http.status_code')).to eq('500')
expect(request_span).to have_error
Expand Down

0 comments on commit 118bc82

Please sign in to comment.