jaegertracing/jaeger-client-csharp

Thrift library has dependencies on Microsoft.AspNetCore.* and Microsoft.Extensions.* libraries

Closed this issue · 5 comments

cwe1ss commented

It seems like Thrift support for .NET is a huge mess. There's the original csharp code base and there's also a newer netcore code base. The latest csharp release is very old and doesn't target .NET Standard and the netcore project doesn't have any official NuGet releases yet.

This project currently has a local copy of the netcore project and it IL-merges it but even this project should not be used IMO because it has a a lot of weird non-standard dependencies - especially Microsoft.AspNetCore. This means that anyone who wants to use LetsTrace/Jaeger in a regular console app would get the whole ASP.NET Core stack DLLs as well - and this is not acceptable IMO.

We therefore need a different solution IMO. I can think of the following:

  • As LetsTrace/Jaeger is currently targeting netstandard2.0 we might be able to just reference the official Thrift package. They introduced this compatibility feature recently AFAIK. However, I'm not sure if the very old version of the official Thrift package still works for us.
  • Vendor-in the csharp project instead of netcore and have it target netstandard2.0. Ideally, this should work with minimal effort as .NET Standard 2.0 is very compatible to the full .NET framework. We would still have to IL-merge it - or we could also just release it under a different name and save us the pain of IL-merging. (e.g. Jaeger.Thrift.ThriftLib).
  • Try one of the existing forks of Thrift on NuGet - e.g. https://www.nuget.org/packages/Apache.Thrift/

Your thoughts?

cwe1ss commented

I've been looking at another alternative: We can just remove the parts from the netcore code that depend on non-standard libraries. As far as I've seen that's just a handful of "Server" files. That will probably be the easiest solution.

grounded042 commented

Thrift has been a pain from the beginning for all of the reasons you outlined above. I don't like IL-merging it, but it seemed like the best option at the time. It would be nice if they responded to the ticket that @Falco20019 put in (https://issues.apache.org/jira/browse/THRIFT-4512).

If we go the route of removing parts from the netcore code would that mean that we are now maintaining our own Thrift code and would have to be a little more active in when they do new releases? I don't know a lot about Thrift - do we know if they have any sort of SLA on compatibility? Would we get into a situation where if we don't keep the Thrift code we modified up to date we might loose compatibility with Jaeger?

cwe1ss commented

I tested this and I only had to remove two files and change one, so merging thrift changes shouldn't be much harder. But of course, this is a horrible solution. You/we shouldn't have to maintain our own version of Thrift.

I created two new issues in their issue tracker:

Seems like not many .NET people are active there though :sad:

  • 👍2
Falco20019 commented
  • I looked into using the official Thrift package, but it is 0.9.3 and that has no support for netcore.
  • I also looked into using the csharp code with 0.9.3. Since jaeger-idl targets 0.9.2 this should be possible. Depending on whether we get csharp compiled using netstandard2.0. Still, I don't like maintaining a version of Thrift, and if users already use Thrift in their project, they should be able to just use one NuGET package.
  • Using another NuGET package seems like our best option.

I don't really like that Jaeger uses Thrift. Would prefer it to use better and actively supported IDLs like gRPC or MsgPack.

I also upvoted your JIRA-Tickets and linked them in the later ticket for easier navigation and tracking.

  • 👍1
yurishkuro commented

Fwiw you don't need to be limited to 0.9.2. the IDL is compatible with many versions.