cleaned up and addressed most todos

lib/datadog/tracing/contrib/grape/endpoint.rb

@@ -94,22 +94,12 @@ 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]
+                integration_route = endpoint.env['grape.routing_args'][:route_info].pattern.path
+                Thread.current[:datadog_http_routing] << [:grape, endpoint.env['SCRIPT_NAME'], integration_route]
 
                 # override the current span with this notification values
                 span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) unless api_view.nil?
-                # 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_PATH, path)
                 span.set_tag(Ext::TAG_ROUTE_METHOD, request_method)
 
                 span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_METHOD, request_method)

lib/datadog/tracing/contrib/rack/middlewares.rb

@@ -107,21 +107,14 @@ def call(env)
             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[3]
-              # TODO: probably should get HTTP method from route resolvers as well
+              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
-                # 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
               end
             end
 

lib/datadog/tracing/contrib/rails/patcher.rb

@@ -38,20 +38,10 @@ 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
+              integration_route = result.first[2].path.spec.to_s if result.any?
 
-              # 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)
+              Thread.current[:datadog_http_routing] << [:rails, args.first.env['SCRIPT_NAME'], integration_route]
+              span.resource = "#{args.first.env['REQUEST_METHOD']} #{integration_route}"
             end
 
             result

lib/datadog/tracing/contrib/sinatra/tracer.rb

@@ -47,20 +47,17 @@ def route_eval
               configuration = Datadog.configuration.tracing[:sinatra]
               return super unless Tracing.enabled?
 
-              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]
+              integration_route = Sinatra::Env.route_path(env)
+              Thread.current[:datadog_http_routing] << [:sinatra, env['SCRIPT_NAME'], integration_route]
 
               Tracing.trace(
                 Ext::SPAN_ROUTE,
                 service: configuration[:service_name],
                 span_type: Tracing::Metadata::Ext::HTTP::TYPE_INBOUND,
-                resource: "#{request.request_method} #{datadog_route}",
+                resource: "#{request.request_method} #{integration_route}",
               ) do |span, trace|
                 span.set_tag(Ext::TAG_APP_NAME, settings.name || settings.superclass.name)
-                span.set_tag(Ext::TAG_ROUTE_PATH, datadog_route)
+                span.set_tag(Ext::TAG_ROUTE_PATH, integration_route)
 
                 if request.script_name && !request.script_name.empty?
                   span.set_tag(Ext::TAG_SCRIPT_NAME, request.script_name)
@@ -69,11 +66,6 @@ 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)