Skip to content

Commit

Permalink
rewrites as trace tag
Browse files Browse the repository at this point in the history
  • Loading branch information
zarirhamza committed Feb 8, 2024
1 parent 6d429ea commit 6d0f118
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 21 deletions.
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ namespace :spec do

desc '' # "Explicitly hiding from `rake -T`"
RSpec::Core::RakeTask.new(:routetest) do |t, args|
t.pattern = 'spec/datadog/tracing/contrib/rack/http_route_spec.rb'
t.pattern = 'spec/datadog/tracing/contrib/http_route_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

Expand Down
7 changes: 2 additions & 5 deletions lib/datadog/tracing/contrib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ def endpoint_start_process(_name, _start, _finish, _id, payload)
Datadog.logger.error(e.message)
end

# rubocop:disable Metrics/AbcSize
def endpoint_run(name, start, finish, id, payload)
return unless Thread.current[KEY_RUN]

Expand Down Expand Up @@ -105,17 +104,15 @@ def endpoint_run(name, start, finish, id, payload)
span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method)
span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_URL, path)

if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array)
Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route]
end
trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, integration_route)
trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, endpoint.env['SCRIPT_NAME'])
ensure
span.start(start)
span.finish(finish)
end
rescue StandardError => e
Datadog.logger.error(e.message)
end
# rubocop:enable Metrics/AbcSize

def endpoint_start_render(*)
return if Thread.current[KEY_RENDER]
Expand Down
15 changes: 9 additions & 6 deletions lib/datadog/tracing/contrib/rack/middlewares.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ def call(env)
request_trace = Tracing.active_trace || TraceOperation.new

env[Ext::RACK_ENV_REQUEST_SPAN] = request_span
Thread.current[:datadog_http_routing] = []

Datadog::Core::Remote::Tie::Tracing.tag(boot, request_span)

Expand All @@ -113,9 +112,8 @@ def call(env)
# call the rest of the stack
status, headers, response = @app.call(env)

if status != 404 && (routed = Thread.current[:datadog_http_routing].last)
last_script_name = routed[1]
last_route = routed[2]
if status != 404 && (last_route = request_trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE))
last_script_name = request_trace.get_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH)

# 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 All @@ -128,8 +126,13 @@ def call(env)
last_route = env['PATH_INFO']
end

request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, last_script_name + last_route) if last_route
Thread.current[:datadog_http_routing] = []
# Clear the route and route path tags from the request trace to avoid possibility of misplacement
request_trace.clear_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE)
request_trace.clear_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH)

# Ensure tags are placed in rack.request span as desired
request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, last_script_name + last_route)
request_span.clear_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH)
end

[status, headers, response]
Expand Down
6 changes: 3 additions & 3 deletions lib/datadog/tracing/contrib/rails/patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ module JourneyRouterPatch
def find_routes(*args)
result = super
integration_route = result.first[2].path.spec.to_s.gsub(/\(\.:format\)/, '') if result.any?
if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array)
Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route]
end
request_trace = Tracing.active_trace || TraceOperation.new
request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, integration_route)
request_trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, args.first.env['SCRIPT_NAME'])
result
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/datadog/tracing/contrib/sinatra/tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ def route_eval
Contrib::Analytics.set_measured(span)

_, path = env['sinatra.route'].split(' ', 2)
if Thread.current.key?(:datadog_http_routing) && Thread.current[:datadog_http_routing].is_a?(Array)
Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], path]
end
trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, path)
trace.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE_PATH, env['SCRIPT_NAME'])
# binding.pry
super
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/tracing/metadata/ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ module HTTP
TAG_CLIENT_IP = 'http.client_ip'
HEADER_USER_AGENT = 'User-Agent'
TAG_ROUTE = 'http.route'
TAG_ROUTE_PATH = 'http.route.path'

# General header functionality
module Headers
Expand Down
1 change: 1 addition & 0 deletions sig/datadog/tracing/metadata/ext.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ module Datadog
TAG_BASE_URL: ::String
TAG_METHOD: ::String
TAG_ROUTE: String
TAG_ROUTE_PATH: String
TAG_STATUS_CODE: ::String
TAG_USER_AGENT: ::String
TAG_URL: ::String
Expand Down
6 changes: 3 additions & 3 deletions spec/datadog/tracing/contrib/http_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
apps_to_build = apps

Rack::Builder.new do
apps_to_build.each do |root, app|
map root do
apps_to_build.each do |route, app|
map route do
run app
end
end
Expand Down Expand Up @@ -155,7 +155,7 @@ def show

expect(span.get_tag('http.route')).to eq('/sinatra/hello/world')

next
break
end
end
end
Expand Down

0 comments on commit 6d0f118

Please sign in to comment.