Respect child_of: option in Tracer#trace

[DocX]

It is documented behaviour in Getting Started guide, however the implementation did not respect it and always used the current context instead - even if the option was passed.

This change makes passing child_of: option to Tracer.trace effective.

[delner]

This change makes a lot of sense. Nice work with the tests, too.

We'll need to rebase and rerun tests after Excon is fixed, to make sure this doesn't cause any problems elsewhere in CI. Otherwise though, I'm happy with this, and it should be good to merge.

[marcotc]

@DocX could you please rebase with master? The excon CI failure should be fixed now.

[DocX]

@marcotc merged :)

[DocX]

@marcotc Hi, can you please help me with the failing tests? I can't figure out why is it failing :(

[delner]

From a glance, it looks like in Rails the traces are not being constructed as expected... I see some differences in an array of Span, so your change may somehow affect Rails instrumentation. I think you'd have to dig into the Rails test in question, compare the generated trace to the expected output, and reason whether the test is wrong, or Rails instrumentation is doing something it shouldn't.

@marcotc should be able to follow up with you soon on this, if you still need more help.

[DocX]

@delner thanks for update

@marcotc I would appreciate help on this, if you have context about the rails integration. thanks

[DocX]

I tried to test the failed examples, but when run alone they don't fail. So it seems there is some inter-example dependency or leak of some state:

bundle exec appraisal rails5-mysql2 ruby -I test test/contrib/rails/controller_test.rb 'template partial rendering is properly traced'

...

..............

Finished in 0.494967s, 30.3051 runs/s, 236.3794 assertions/s.

15 runs, 117 assertions, 0 failures, 0 errors, 0 skips

[marcotc]

Hey @DocX, I figured out what the issue is.
You changes have revealed that we were reusing the options Hash provided to Tracer#trace here:

dd-trace-rb/lib/ddtrace/contrib/active_support/notifications/subscription.rb

Line 78 in 7a643d1

tracer.trace(@span_name, @options).tap do |span|

Because we modify the provided options Hash, e.g. options[:child_of] = call_context, we were passing in polluted data to the #trace.

This wasn't an issue before because we were always overriding :child_of, which your PR fixes :)

I wrote a fix for the affected call site (it's only one affected in our code base thankfully). I thought about pushing a separate PR for it, but I believe it makes the most sense to be shipped with your PR.

Could you please include this commit in this PR? 8f33959
All current tests failures will succeed with this change.

[DocX]

@marcotc thanks for the help! i've added your commit to this PR :)

[marcotc]

Thank you so much for this work @DocX! 🚀