Skip to content
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

Add http.route tag to rack #3345

Merged
merged 39 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
f5320cc
Add crude span for Rails routing
lloeki Dec 21, 2023
1a1909c
Move span creation to Routing::RouteSet
lloeki Dec 21, 2023
047e41a
added routes to sinatra/grape and fixes up rails
zarirhamza Dec 27, 2023
52246bc
linter fixes
zarirhamza Dec 27, 2023
00cf5af
Handle all sorts of nesting
lloeki Jan 10, 2024
737b9ec
cleaned up and addressed most todos
zarirhamza Jan 10, 2024
2d9676a
removes unnecessary rails tracing
zarirhamza Jan 11, 2024
db033da
adds tracing to rack requests and basic testing
zarirhamza Jan 11, 2024
d3dfe32
Add crude span for Rails routing
lloeki Dec 21, 2023
2a26d9c
Move span creation to Routing::RouteSet
lloeki Dec 21, 2023
8e39733
added routes to sinatra/grape and fixes up rails
zarirhamza Dec 27, 2023
02761bb
linter fixes
zarirhamza Dec 27, 2023
016df11
Handle all sorts of nesting
lloeki Jan 10, 2024
898e959
cleaned up and addressed most todos
zarirhamza Jan 10, 2024
9353f02
removes unnecessary rails tracing
zarirhamza Jan 11, 2024
118bc82
adds tracing to rack requests and basic testing
zarirhamza Jan 11, 2024
604f07b
fixes grape flaky test
zarirhamza Jan 12, 2024
8159dac
Merge branch 'add-rails-routing-instrumentation' of ssh://github.com/…
zarirhamza Jan 12, 2024
48334db
Update gemfiles/*
zarirhamza Jan 12, 2024
b335dd5
remove kwargs to fix tests
zarirhamza Jan 12, 2024
2bc87c4
Merge branch 'add-rails-routing-instrumentation' of ssh://github.com/…
zarirhamza Jan 12, 2024
e4aa83a
Skip patching for Rails 3
marcotc Jan 13, 2024
78fe38d
Skip patching for Rails 3
marcotc Jan 13, 2024
2f901db
Skip for Rails < 4.2
marcotc Jan 16, 2024
16e3cb3
linter fixes
zarirhamza Jan 16, 2024
82aaa02
Try to fix Spring test failure
marcotc Jan 16, 2024
8419654
removes unnecessary constants
zarirhamza Jan 25, 2024
696a3d0
added comment and fixed nested rack routes
zarirhamza Jan 25, 2024
ce0db69
fixes leak and potential errors
zarirhamza Jan 30, 2024
4ad8a6d
fixed sinatra route when using script name
zarirhamza Feb 5, 2024
1f89257
adds tests
zarirhamza Feb 5, 2024
293e8c4
Update gemfiles/*
zarirhamza Feb 5, 2024
40fd7bb
update appraisal files
zarirhamza Feb 6, 2024
564c0bc
Merge branch 'master' into add-rails-routing-instrumentation
zarirhamza Feb 6, 2024
c7301d3
adds jruby tests and lint
zarirhamza Feb 6, 2024
b640089
Update gemfiles/*
zarirhamza Feb 6, 2024
128ce7d
fixed rakefile
zarirhamza Feb 6, 2024
6d429ea
Merge branch 'master' into add-rails-routing-instrumentation
zarirhamza Feb 7, 2024
6d0f118
rewrites as trace tag
zarirhamza Feb 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion lib/datadog/tracing/contrib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,22 @@ def endpoint_run(name, start, finish, id, payload)

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

# TODO: pick one, but Rails has only the first one, and I feel like it makes the most sense

# this one has (.json)
datadog_route = endpoint.env['grape.routing_args'][:route_info].pattern.path
# this one does not have (.json)
datadog_route = endpoint.env['grape.routing_args'][:route_info].pattern.origin
# TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144
# TODO: :grape should be a reference to the integration name
Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], endpoint.env['PATH_INFO'], datadog_route]

# override the current span with this notification values
span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil?
span.set_tag(Ext::TAG_ROUTE_PATH, path)
# TODO: should this rather be like this?
# span.set_tag(Ext::TAG_ROUTE_PATH, path)
# span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path)
span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route)
span.set_tag(Ext::TAG_ROUTE_METHOD, request_method)

span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method)
Expand Down
25 changes: 25 additions & 0 deletions lib/datadog/tracing/contrib/rack/middlewares.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,38 @@ def call(env)
request_span.resource = nil
request_trace = Tracing.active_trace
env[Ext::RACK_ENV_REQUEST_SPAN] = request_span
Thread.current[:datadog_http_routing] = []

# Copy the original env, before the rest of the stack executes.
# Values may change; we want values before that happens.
original_env = env.dup

# 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)
zarirhamza marked this conversation as resolved.
Show resolved Hide resolved
# TODO: array == bad, this should be a Struct
last_script_name = routed[1]
last_route = routed[3]
# TODO: probably should get HTTP method from route resolvers as well

# 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
# TODO: concatenate better? (according to spec I think it's fine /-wise)
composite_route = last_script_name + last_route
composite_path = last_script_name + last_route

request_span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_ROUTE, composite_route)

# TODO: should it be composite_route?
request_span.resource = env['REQUEST_METHOD'] + ' ' + composite_path
zarirhamza marked this conversation as resolved.
Show resolved Hide resolved
end
end

[status, headers, response]

# rubocop:disable Lint/RescueException
Expand Down
4 changes: 4 additions & 0 deletions lib/datadog/tracing/contrib/rails/ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ module Ext
ENV_ANALYTICS_ENABLED = 'DD_TRACE_RAILS_ANALYTICS_ENABLED'
ENV_ANALYTICS_SAMPLE_RATE = 'DD_TRACE_RAILS_ANALYTICS_SAMPLE_RATE'
ENV_DISABLE = 'DISABLE_DATADOG_RAILS'
SPAN_ROUTE = 'rails.route'
zarirhamza marked this conversation as resolved.
Show resolved Hide resolved
TAG_ROUTE_PATH = 'rails.route.path'
TAG_COMPONENT = 'rails'
TAG_OPERATION_ROUTING = 'routing'
end
end
end
Expand Down
51 changes: 51 additions & 0 deletions lib/datadog/tracing/contrib/rails/patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,59 @@
require_relative 'middlewares'
require_relative 'utils'
require_relative '../semantic_logger/patcher'
require_relative 'ext'

module Datadog
module Tracing
module Contrib
module Rails
# Patcher to begin span on Rails routing
module RoutingRouteSetPatch
def call(*args, **kwargs)
result = nil

configuration = Datadog.configuration.tracing[:rails]

Tracing.trace(
Ext::SPAN_ROUTE,
service: configuration[:service_name],
span_type: Tracing::Metadata::Ext::HTTP::TYPE_INBOUND,
) do |span|
span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT)
span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTING)
result = super
end

result
end
end

# Patcher to trace rails routing done by JourneyRouter
module JourneyRouterPatch
def find_routes(*args, **kwargs)
result = super

if Datadog::Tracing.enabled? && (span = Datadog::Tracing.active_span)
if result.any?
datadog_route = result.first[2].path.spec.to_s
end
zarirhamza marked this conversation as resolved.
Show resolved Hide resolved

# TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144
# TODO: :rails should be a reference to the integration name
Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], args.first.env['PATH_INFO'], datadog_route]

span.resource = datadog_route.to_s

# TODO: should this rather be like this?
# span.set_tag(Ext::TAG_ROUTE_PATH, path_info)
# span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path)
span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route)
end

result
end
end

# Patcher enables patching of 'rails' module.
module Patcher
include Contrib::Patcher
Expand Down Expand Up @@ -41,6 +89,9 @@ 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]

ActionDispatch::Routing::RouteSet.prepend(RoutingRouteSetPatch)
ActionDispatch::Journey::Router.prepend(JourneyRouterPatch)

Rails::LogInjection.configure_log_tags(app.config)
end
end
Expand Down
9 changes: 9 additions & 0 deletions lib/datadog/tracing/contrib/sinatra/tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ def route_eval

datadog_route = Sinatra::Env.route_path(env)

# TODO: instead of a thread local this may be put in env[something], but I'm not sure we can rely on it bubbling all the way up. see https://github.com/rack/rack/issues/2144
# TODO: :sinatra should be a reference to the integration name
Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], env['PATH_INFO'], datadog_route]

Tracing.trace(
Ext::SPAN_ROUTE,
service: configuration[:service_name],
Expand All @@ -65,6 +69,11 @@ def route_eval
span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT)
span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ROUTE)

# TODO: should this rather be like this?
# span.set_tag(Ext::TAG_ROUTE_PATH, path_info)
# span.set_tag(Ext::TAG_ROUTE_PATTERN, datadog_path)
span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route)

trace.resource = span.resource

sinatra_request_span = Sinatra::Env.datadog_span(env)
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 @@ -86,6 +86,7 @@ module HTTP
TYPE_TEMPLATE = 'template'
TAG_CLIENT_IP = 'http.client_ip'
HEADER_USER_AGENT = 'User-Agent'
TAG_ROUTE = 'http.route'

# 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 @@ -42,6 +42,7 @@ module Datadog
ERROR_RANGE: ::Range[::Integer]
TAG_BASE_URL: ::String
TAG_METHOD: ::String
TAG_ROUTE: String
TAG_STATUS_CODE: ::String
TAG_USER_AGENT: ::String
TAG_URL: ::String
Expand Down
Loading