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

Faraday Integration Usage #786

Closed
mdross95 opened this issue Aug 2, 2019 · 4 comments · Fixed by #843
Closed

Faraday Integration Usage #786

mdross95 opened this issue Aug 2, 2019 · 4 comments · Fixed by #843
Assignees
Labels
bug Involves a bug community Was opened by a community member integrations Involves tracing integrations
Milestone

Comments

@mdross95
Copy link

mdross95 commented Aug 2, 2019

Hi,

I am attempting to use the faraday integration in a rails app. I am unclear on the usage of the integration. Looking at the API spec:

require 'faraday'
require 'ddtrace'

# Configure default Faraday tracing behavior
Datadog.configure do |c|
  c.use :faraday, options
end

# Configure Faraday tracing behavior for single connection
connection = Faraday.new('https://example.com') do |builder|
  builder.use(:ddtrace, options)
  builder.adapter Faraday.default_adapter
end

connection.get('/foo')

Are both of these additions necessary for faraday tracing? I ask because I originally attempted to add just the Datadog.configure block, but was unable to see any tracing in datadog. Once I added the builder.use line to a single connection for a sanity check, I was able to see the tracing show up. I was under the impression that was only necessary to customize a single connection if desired. Was I incorrect (and this is needed for every connection), or is something funky occurring with my default behavior (if the latter I can provide more information)? I am using faraday version 0.9.2 and ddtrace version 0.25.1.

Thanks,
Mike

@delner
Copy link
Contributor

delner commented Aug 5, 2019

My understanding was you should only need Datadog.configure, but can use builder.use(:ddtrace, options) to customize individual connections. However, looking at the unit tests, I can't see that we test with only Datadog.configure, so there's a reasonable chance this doesn't work as intended.

If so, we'll look into adjusting this so you only have to do Datadog.configure, if at all possible. Thanks for the report!

@delner delner self-assigned this Aug 5, 2019
@delner delner added bug Involves a bug community Was opened by a community member integrations Involves tracing integrations labels Aug 5, 2019
@mdross95
Copy link
Author

mdross95 commented Aug 5, 2019

Thanks!

@martinisoft
Copy link

This has also affected our integration as well despite having the latest Faraday and Faraday Middleware gems. Had to manually inject in .use :ddtrace in more than a few places where I thought it was working. I too was under the impression from the docs and the code that this injected itself automatically when enabled, but that doesn't appear to be the case.

marcotc added a commit that referenced this issue Oct 25, 2019
Fixes #786

This PR adds a default [Faraday](https://github.com/lostisland/faraday) middleware handler to allow for `ddtrace` users to be able to instrument it out-of-the-box, by simply enabling it:

```ruby
Datadog.configure do |c|
  c.use :faraday
end
```

Currently we also require users to explicitly configure our middleware on every Faraday connection:

```ruby
connection = Faraday.new('https://example.com') do |builder|
  builder.use(:ddtrace)
  # ...
end
```
This per-connection configuration won't be necessary anymore, unless custom configuration settings are desired for that connection.
@delner delner added this to the 0.29.0 milestone Nov 20, 2019
@delner
Copy link
Contributor

delner commented Nov 20, 2019

@mdross95 @martinisoft The change for this should roll with 0.29.0. Thanks for reporting this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug community Was opened by a community member integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants