Skip to content

Commit

Permalink
Improve rack instrumentation and specs
Browse files Browse the repository at this point in the history
  • Loading branch information
y9v committed Sep 13, 2024
1 parent df0bd9a commit 3207b16
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ def set_http_route_tags(route_spec, script_name)
if script_name && !script_name.empty?
request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, script_name)
end
rescue StandardError => e
Datadog.logger.error(e.message)
end

def dispatcher_route?(route)
Expand Down
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 @@ -66,7 +66,7 @@ def endpoint_start_process(_name, _start, _finish, _id, payload)
if (grape_route = env['grape.routing_args'] && env['grape.routing_args'][:route_info])
trace.set_tag(
Tracing::Metadata::Ext::HTTP::TAG_ROUTE,
grape_route.path&.gsub(/\(\.{1}:?\w+\)\z/, '')
grape_route.path&.gsub(/\(\.:?\w+\)\z/, '')
)

trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, env['SCRIPT_NAME'])
Expand Down
2 changes: 1 addition & 1 deletion lib/datadog/tracing/contrib/rack/middlewares.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def set_request_tags!(trace, request_span, env, status, headers, response, origi
request_span.set_tag(Tracing::Metadata::Ext::TAG_KIND, Tracing::Metadata::Ext::SpanKind::TAG_SERVER)

if status != 404 && (last_route = trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE))
last_script_name = trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH).to_s
last_script_name = trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH).to_s || ''

# If the last_script_name is empty but the env['SCRIPT_NAME'] is NOT empty
# then the current rack request was not routed and must be accounted for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,5 @@

expect(traces).to be_empty
end

it 'rescues exceptions' do
expect(Datadog::Tracing).to receive(:active_trace).and_raise('boom')

expect(Datadog.logger).to receive(:error).with('boom')

described_class.set_http_route_tags('/users/:id', '/auth')
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
describe '#find_routes' do
before do
engine.routes.append do
get '/sign-in/(:expires_in)' => 'tokens#create'
get '/sign-in' => 'tokens#create'
end

auth_engine = engine
Expand All @@ -41,6 +41,7 @@
get '/items/:id', to: 'items#by_id', id: /\d+/
get '/items/:slug', to: 'items#by_slug', id: /(\w-)+/

get 'books(/:category)', to: 'books#index'
get 'books/*section/:title', to: 'books#show'
end
end
Expand Down Expand Up @@ -88,6 +89,10 @@ def by_slug
stub_const(
'BooksController',
Class.new(ActionController::Base) do
def index
head :ok
end

def show
head :ok
end
Expand Down Expand Up @@ -180,14 +185,25 @@ def create
expect(request_span.tags).not_to have_key('http.route.path')
end

it 'sets http.route correctly for routes with optional parameter' do
get 'books/some-category'

request_span = spans.first

expect(last_response).to be_ok
expect(request_span.name).to eq('rack.request')
expect(request_span.tags.fetch('http.route')).to eq('/books(/:category)')
expect(request_span.tags).not_to have_key('http.route.path')
end

it 'sets http.route and http.route.path for rails engine routes' do
get '/api/auth/sign-in'

request_span = spans.first

expect(last_response).to be_ok
expect(request_span.name).to eq('rack.request')
expect(request_span.tags.fetch('http.route')).to eq('/api/auth/sign-in(/:expires_in)')
expect(request_span.tags.fetch('http.route')).to eq('/api/auth/sign-in')
expect(request_span.tags).not_to have_key('http.route.path')
end

Expand Down

0 comments on commit 3207b16

Please sign in to comment.