Invert dependency between Jaeger and Jaeger.Thrift NuGet packages

Falco20019 commented 5 years ago

Which problem is this PR solving?

Fixes #116
Related to #76

Short description of the changes

This is a breaking change in a couple of aspects.

Without setting-up Configuration.SenderConfiguration.DefaultSenderResolver, RemoteReporter.Builder.Build() will result in NoopSender instead of UdpSender
Package structure changes, although the Jaeger NuGET package will resolve correctly
This will result in the usage being compile-compatible but not assembly-compatible.

This solution is similar to how it was solved in Java.

Things taken into account

I tried to avoid having to Configuration.SenderConfiguration.DefaultSenderResolver by using reflection. This works for .NET Framework 4.7 and .NET Core 2.1, but did not work with ASP.NET Core 2.1. The main problem is, that ASP.NET Core did not include the assembly in the AppDomain since it was not called directly from code.

Another option would have been to use something like a Jaeger.Initialize() method that could be offered by the regular Jaeger package, but focusing on just a central default seemed like the most straight-forward and cleaner solution.

Falco20019 commented 4 years ago

@yurishkuro As #118 is solved now, I would like to finally trigger this change.

yurishkuro commented 4 years ago

you may need to squash all the commits to fix the DCO

Falco20019 commented 4 years ago

I squash and merge them using admin rights, so I don’t care about DCO upfront. I just make sure that I care about the warning and have it in the squashed :)

I will integrate your suggestions and merge it later today. Thanks again for the great input!

jaegertracing / jaeger-client-csharp