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

Sinatra integration not setting rack.request #486

Closed
jpaulgs opened this issue Jul 16, 2018 · 5 comments · Fixed by #1015
Closed

Sinatra integration not setting rack.request #486

jpaulgs opened this issue Jul 16, 2018 · 5 comments · Fixed by #1015
Assignees
Labels
community Was opened by a community member integrations Involves tracing integrations question General inquiry that may or may not involve changes
Milestone

Comments

@jpaulgs
Copy link

jpaulgs commented Jul 16, 2018

I'm setting up DD APM to trace a Rack -> Sinatra -> Postgres application

With the following configuration each of these layers are presented as a seperate service in the environment. Which seems to be the way DD is expecting to be setup.

  Datadog.configure do |c|
    c.tracer(
      hostname: ENV.fetch('DD_AGENT_SERVICE_HOST'),
      port: ENV.fetch('DD_AGENT_SERVICE_PORT'),
      env: ENV.fetch('DD_APM_ENV')
    )
    c.use :rack
    c.use :sinatra 
    c.use :sequel
  end

  use Datadog::Contrib::Rack::TraceMiddleware

image

However the rack service is grouping requests by http method and response code

image

Whilst the Sinatra service is working as expected:

image

From reading through the code it seems like the Sinatra integration should be setting the resource on the rack span or am I missing something?

@delner
Copy link
Contributor

delner commented Jul 16, 2018

I do not believe the Sinatra integration is currently setup to do this. Rails does by default, by looking at its parent span, which if its a 'rack.request', it then overrides the resource on it. Out of the box, Sinatra won't do this, but it could by retrieving the 'datadog.rack_request_span' key from the Rack ENV and overriding its resource. (We'd like to find a better way of doing this but that's the current option right now.)

If you don't want to patch the Sinatra integration, you could use the Processing Pipeline to modify the Rack parent span whenever a Sinatra span is processed. Not ideal, but it would probably get things working in the interim.

Let me know if this works for you!

@delner delner added question General inquiry that may or may not involve changes integrations Involves tracing integrations community Was opened by a community member labels Jul 16, 2018
@delner delner self-assigned this Jul 16, 2018
@jpaulgs
Copy link
Author

jpaulgs commented Jul 17, 2018

I just created a pull request that updates the sinatra middleware to set the rack.resource.

Have a look and see what you think

@delner
Copy link
Contributor

delner commented Jul 17, 2018

@jpaulgs I'll check it out. Thanks!

@marcotc
Copy link
Member

marcotc commented Apr 24, 2020

Thank you for the report again @jpaulgs!

#1015 was just merged, which addresses this issue.
We'll update this GH issue when the release is out with this fix.

@marcotc marcotc added this to the 0.35.0 milestone Apr 24, 2020
@marcotc
Copy link
Member

marcotc commented May 4, 2020

v0.35.0 has been released, with #1015 which address this issue.
But please let us know if there's any further feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Was opened by a community member integrations Involves tracing integrations question General inquiry that may or may not involve changes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants