undefined method `traces' for #<Datadog::Transport::HTTP::Client ... when using SyncWriter

[Yurokle]

Hello folks. I'm seeing the following error when trying to use SyncWriter in my specs:

#<Thread:0x00007ffb036a0c60@/Users/ynaidyon/.rvm/gems/ruby-2.6.5/gems/ddtrace-0.30.1/lib/ddtrace/sync_writer.rb:38 run> terminated with exception (report_on_exception is true):
/Users/ynaidyon/.rvm/gems/ruby-2.6.5/gems/ddtrace-0.30.1/lib/ddtrace/sync_writer.rb:53:in `flush_trace': 
undefined method `traces' for #<Datadog::Transport::HTTP::Client:0x00007ffb02b02c50> (NoMethodError)

I think it was broken around here: 46b685b#diff-34a63124ba72d0f43c54ea1cb77f78e5

[delner]

Thanks for the report! I think #904 should fix this; feel free to try it out.

[Yurokle]

Hello! I've been testing this, and I'm not sure if it actually fixes the problem, or maybe it just introduces another issue.
So what I'm doing in my code in test environment is something like this:

::Datadog.configure do |config|
  config.tracer(
    # ...
    transport_options: proc { |t| t.adapter(:test, TestBuffer) },
    writer: Datadog::SyncWriter.new,
  )

Unfortunately, it looks like when I'm running my specs it still tries to make calls via the Net::HTTP adapter and not the Test one. I think following line is the reason for this:
https://github.com/DataDog/dd-trace-rb/blob/master/lib/ddtrace/tracer.rb#L401

I think the problem here is more in my code, rather than in yours - I'm using both custom writer and transport adapter, but maybe it would make sense to make those things mutually exclusive? Or like raise an error of you pass both, since transport_options: won't be applied to a custom writer since those are just arguments for a writer constructor.

[Yurokle]

Actually, it looks like transport_options: aren't just raw arguments to a writer constructor, but rather a hash with :priority_sampler, :transport_options arguments which is being constructed in the #configure_writer method of the Datadog::Tracer

[Yurokle]

I don't know if people actually want to be able to pass a custom writer object or they just want to be able to pick from sync and async writers, if the latter is the case, mb it would make sense to use a symbol / class name to distinguish between the two, and keep the initialization logic in one place (#configure_writer method)
Just a thought.

[delner]

Yeah I think these are good suggestions: we want to minimize configuration whenever possible, so we've been looking at ways to implement some designs to our components that will help us improve on this e.g. #879

In the mean time, there's a couple of ways to configure SyncWriter with a test adapter:

Datadog.configure do |config|
  config.tracer(
    writer: Datadog::SyncWriter.new(
      transport: Datadog::Transport::HTTP.default { |t| t.adapter :test, [] }
    )
  )
end

Or

Datadog.configure do |config|
  config.tracer(
    writer: Datadog::SyncWriter.new(
      transport_options: { on_build: proc { |t| t.adapter(:test, []) } }
    )
  )
end

Both should work, but I'm working on plans that will hopefully make this less complicated in the near future.

[delner]

Merged to master to be released with 0.31.0.

[Yurokle]

Dope, thanks! 🚂🎺👍

…

On Tue, Jan 7, 2020 at 2:23 PM David Elner ***@***.***> wrote: Merged to master to be released with 0.31.0. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#903?email_source=notifications&email_token=AABZGPODRVZVKDCU5J5246LQ4TJERA5CNFSM4KBRBMBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIJ7HDA#issuecomment-571732876>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABZGPMXTVUAT4HMLRFTXFDQ4TJERANCNFSM4KBRBMBA> .

-- Yuriy Naidyon