Invert dependency between Jaeger and Jaeger.Thrift NuGet packages

[avkom]

Requirement

We do not want that Jaeger NuGet package depends on Jaeger.Thrift package.

Problem

Jaeger package depends on Jaeger.Thrift package. Jaeger package may be a 'reference implementation' of OpenTracing standard on .NET platform that can be used with custom implementation of Jaeger.Senders.ISender interface. In case our custom implementation of ISender does not use Thrift protocol, we do not want that Jaeger NuGet package depends on Jaeger.Thrift and further Jaeger.Thrift.VendoredThrift packages.

Proposal

1. Invert dependency between Jaeger and Jaeger.Thrift packages. After this change, Jaeger.Thrift package will depend on Jaeger.
2. Move implementations of Jaeger.Senders.ISender interface from Jaeger project to Jaeger.Thrift project. (Move classes HTTPSender, ThriftSender, UdpSender, JaegerThriftSpanConverter, Configuration etc.).
3. After this change, developers have to reference Jaeger.Thrift NuGet package in their applications.

Challenges:

1. RemoteReporter.Builder.Build() depends on UdpSender. We need to break this dependency. For example, raise an exception when sender is not provided.

[Falco20019]

I would like to get rid of Thrift once gRPC support is fully added. I also would like to get rid of linking directly to the package as discussed in #76 but currently have little time to work on the library. I hope to find some time in January.

[yurishkuro]

Fwiw a similar refactoring happened in the Java client.

[avkom]

I am working on a proof of concept for this change, but it requires some polish. I am going to submit a pull request. Do you accept PRs in this repository?

[yurishkuro]

Do you accept PRs in this repository?

certainly

[Falco20019]

I think this will somewhat revive the discussion around #34. Maybe we should shift Jaeger back into Jaeger.Core and strip the transport layer out again. Jaeger could than be the alias to to Jaeger.Thrift as discussed in #34 so that people can just update without breaking.

@cwe1ss This would basically revert #59 but I think, since we now want to have the abstraction layer, this would make the most sense. This would also make #115 cleaner since we can then only have a dependency on Jaeger.Core instead of the whole implementation.

[avkom]

@Falco20019 Even if we keep HTTPSender, ThriftSender, UdpSender, JaegerThriftSpanConverter, and Configuration classes in Jaeger project and move all other types from Jaeger to Jaeger.Core project, this still be a breaking change for developers who use any of the moved types (for example, IMetricsFactory or IReporter) because these types are moved to a different assembly and to a different namespace, so the developer might have to add a reference to Jaeger.Core package or add "using Jaeger.Core...;`" directive.

[Falco20019]

I prepared something to try out. I wanted to use reflection to check what sender implementations are loaded with the application. Sadly, this does not work with ASP.NET Core. It seems that DLLs that are not referenced in code get not loaded into the AppDomain in ASP.NET Core.

The idea was to make it close to how Java implemented it with jaeger-thrift. It looks like I need to create a registry where the SenderFactory has to be registered, similar to the idea of how Microsoft implemented the LoggerFactory with LoggerProvider.

Has someone another idea how we could realize the plugin concept for senders? If not, I think this would even be the cleanest solution without any magic in the background. Only downside, the default RemoteReporter would then fallback to a NoopSender if no sender was manually added or if not the Configuration class was used.