Falco20019
commented
4 years ago I what perspective is the public API different?
Closed Lercher closed 4 years ago
Falco20019
commented
4 years ago I what perspective is the public API different?
Lercher
commented
4 years ago This Thrift:
Adds this reference:
with path D:\Daten\github.com\jaegertracing\jaeger-client-csharp\packages\ApacheThrift.0.13.0.1\lib\net45\Thrift45.dll
Running my sample console app then bombs with
Object explorer then shows this. Note the [Thrift] and [Thrift45] markers that are displayed where differences are recognized by my VS2017:
This happens on a modern 64bit Win10 client, where I wanted to reproduce my problem. What puzzles me right now is that I have a different machine (32bit Win7 that I must support, sorry) where the two thrift.dll differences are more dramatic, e.g. there is no TProtocol at all, which hinders the thrift sender to compile. Nevermind though, I must investigate that issue in the first place.
N.B.: additional references coming with thrift:
AddingThrift13.txt
Falco20019
commented
4 years ago You should not add a dependency on ApacheThrift yourself. Use the transitive dependency through Jaeger. Only that way you will end up with the correct netstandard version of Apache. See https://github.com/dotnet/sdk/issues/1791#issuecomment-440895111
Lercher
commented
4 years ago On API differences compiling Jaeger.Thrift: Its a lot of using errors like
1>D:\Daten\lsservice\Jaeger.Thrift\Agent\AggregationValidator.cs(15,23,15,31): error CS0234: The type or namespace name 'Entities' does not exist in the namespace 'Thrift.Protocol' (are you missing an assembly reference?)
1>D:\Daten\lsservice\Jaeger.Thrift\Agent\AggregationValidator.cs(16,23,16,32): error CS0234: The type or namespace name 'Utilities' does not exist in the namespace 'Thrift.Protocol' (are you missing an assembly reference?)
1>D:\Daten\lsservice\Jaeger.Thrift\Agent\AggregationValidator.cs(18,14,18,23): error CS0234: The type or namespace name 'Processor' does not exist in the namespace 'Thrift' (are you missing an assembly reference?)and then a lot of doesn't implement like that:
1>D:\Daten\lsservice\Jaeger.Thrift\Span.cs(22,31,22,36): error CS0535: 'Span' does not implement interface member 'TBase.Read(TProtocol)'
1>D:\Daten\lsservice\Jaeger.Thrift\Span.cs(22,31,22,36): error CS0535: 'Span' does not implement interface member 'TAbstractBase.Write(TProtocol)'
1>D:\Daten\lsservice\Jaeger.Thrift\ClientStats.cs(19,38,19,43): error CS0535: 'ClientStats' does not implement interface member 'TBase.Read(TProtocol)'
1>D:\Daten\lsservice\Jaeger.Thrift\ClientStats.cs(19,38,19,43): error CS0535: 'ClientStats' does not implement interface member 'TAbstractBase.Write(TProtocol)'
1>D:\Daten\lsservice\Jaeger.Thrift\Collector.cs(34,27,34,38): error CS0246: The type or namespace name 'TBaseClient' could not be found (are you missing a using directive or an assembly reference?)
1>D:\Daten\lsservice\Jaeger.Thrift\Collector.cs(34,40,34,51): error CS0535: 'Collector.Client' does not implement interface member 'IDisposable.Dispose()'
1>D:\Daten\lsservice\Jaeger.Thrift\Collector.cs(73,35,73,51): error CS0246: The type or namespace name 'ITAsyncProcessor' could not be found (are you missing a using directive or an assembly reference?)
1>D:\Daten\lsservice\Jaeger.Thrift\Agent\Agent.cs(36,40,36,51): error CS0535: 'Agent.Client' does not implement interface member 'IDisposable.Dispose()'
1>D:\Daten\lsservice\Jaeger.Thrift\Log.cs(22,30,22,35): error CS0535: 'Log' does not implement interface member 'TBase.Read(TProtocol)'
1>D:\Daten\lsservice\Jaeger.Thrift\Log.cs(22,30,22,35): error CS0535: 'Log' does not implement interface member 'TAbstractBase.Write(TProtocol)'which is probably caused by thrift's different async implementation model.
Falco20019
commented
4 years ago Yes I checked the code base for thrift. net45 is only for comparability with code generated as „csharp“. We generate as „netstd“. Make sure to not reference ApacheThrift in your own csproj to avoid this problem.
See the link one post above for an explanation. It’s a problem with how NuGet selects packages. And there is no other workaround than having a separate csproj targeting netstandard and inheriting the dependencies. As jaeger only targets netstandard, you just have to remove your manual NuGet reference.
Lercher
commented
4 years ago Yeah. Made a new net461 project, added the nuget Successfully installed 'Jaeger 0.3.7' to JaegerFw461Sample and it reports
This is a FW 4.6.1 sample
Tracer(ServiceName=someService, Version=CSharp-0.3.7.0, Reporter=RemoteReporter(Sender=UdpSender(UdpTransport=ThriftUdpClientTransport(Client=127.0.0.1:6831))), Sampler=ConstSampler(True), IPv4=-1062731278, Tags=[jaeger.version, CSharp-0.3.7.0], [hostname, ITEM-S69570], [ip, 192.168.1.242], ZipkinSharedRpcSpan=False, ExpandExceptionLogs=False, UseTraceId128Bit=False)I'm just probably too dumb for this. So sorry. Thanks for your help!
Falco20019
commented
3 years ago @razbezhkin Glad that you found it out yourself. Just make sure to dispose your tracer (or call CloseAsync) when closing your application. This will ensure it at least tries to send the spans before exiting. Dispose calls CloseAsync with a CancellationToken that times out after 15 seconds. The RemoteReporter itself has a timeout of 10 seconds when disposed. So if sending the span is not possible immediately, it will still try for 10-15 seconds to finalize it.
yury-kozlov
commented
3 years ago @Lercher Can you please advise how the problem solved on your end ? I'm facing the same issue: when creating a console application for .net framework 4.6.1 and adding nuget Jaeger 1.0.0 with a code below I got exception Could not load file or assembly 'Thrift, Version=0.13.0.0, Culture=neutral, PublicKeyToken=5792536a45833b9f'
var loggerFactory = new Microsoft.Extensions.Logging.LoggerFactory();
Jaeger.Configuration.SenderConfiguration.DefaultSenderResolver = new Jaeger.Senders.SenderResolver(loggerFactory)
.RegisterSenderFactory<Jaeger.Senders.Thrift.ThriftSenderFactory>();
// this line throws exception:
var sender = new Jaeger.Configuration.SenderConfiguration(loggerFactory).GetSender();Only after I copied manually packages\ApacheThrift.0.13.0.1\lib\netstandard2.0\Thrift.dll into the bin folder the issue won't happen. But as mentioned in one of the posts above this is not supposed to be a solution.
Falco20019
commented
3 years ago @yury-kozlov Make sure to not reference ApacheThrift in your csproj to use the the one coming from referencing Jaeger. Otherwise, you might be running into the issue discussed here: https://github.com/dotnet/sdk/issues/1791#issuecomment-440895111
If you try to run it on another machine, make sure to use the publish option instead of copying just the bin folder. This will ensure that all DLLs are where they belong.
Let me know if that removes your problem, otherwise I will give it another look.
yury-kozlov
commented
3 years ago @Falco20019 what do you mean:
Make sure to not reference ApacheThrift directly but to use the the one coming from referencing Jaeger.
As a nuget reference I only added official Jaeger package without anything else (based on clean console application project with .net 4.61).
As a class reference I only used Jaeger.Senders.Thrift.ThriftSenderFactory which comes from jaeger package (see code example above).
Adding packages\ApacheThrift.0.13.0.1\lib\netstandard2.0\Thrift.dll to bin folder is just an experiment - without doing this I get an exception "Could not load file or assembly"
Falco20019
commented
3 years ago Can you check if you have a Thrift45.dll in your folder? This issue above happens when you have a .NET project, referencing ApacheThrift directly. This will lead to the project using the .NET Framework 4 version instead of the .NET Standard Version and therefore will not work.
This should not happen if you do not reference ApacheThrift directly and only reference Jaeger. Can you post your csproj? Is this a minimal dummy project that you created or an existing one?
.NET Framework 4.6.1 only has limited support for .NET Standard which is where this problem comes from. So if you are not bound to that version, I would always recommend a version of 4.7.2 or higher. A good explanation of why can be found here: https://weblog.west-wind.com/posts/2019/Feb/19/Using-NET-Standard-with-Full-Framework-NET
Other than that, debugging directly should work and using publish should work. Only running it from bin directly will not work with 4.6.1 since the proxy DLLs are missing. Also your app.config should contain some entries automatically created (ending up in the YourApplication.exe.config in bin).
yury-kozlov
commented
3 years ago Yes I do have Thrift45.dll in the bin\debug folder.
Yes this is a minimal .net framework 4.6.1 project created from a default Visual Studio 2017 template (console application).
The same issue also happens with a minimal .net framework 4.7.2 project.
The issue also happens during debugging.
App.config doesn't contain any binding redirects related to jaeger or thrift (only System.Buffers and System.Runtime.CompilerServices.Unsafe bindings which shouldn't be related to the issue).
Falco20019
commented
3 years ago I think you need to use PackageReference instead of packages.config for .NET Standard packages to correctly work. There should be an option to convert the project or it should have asked you what to use.
You can also use the new SDK-style csproj format with TargetFramework set to to net461.
See https://docs.microsoft.com/en-us/nuget/consume-packages/migrate-packages-config-to-package-reference on how to migrate.
yury-kozlov
commented
3 years ago Unfortunately PackageReference instead of packages.config didn't help:
<PropertyGroup>
<RestoreProjectStyle>PackageReference</RestoreProjectStyle>
</PropertyGroup>Thrift45.dll in the output folder instead of Thrift.dll
When running a sample application got the same error Could not load file or assembly 'Thrift...Lercher
commented
3 years ago Hi all, everything was already answered by https://github.com/jaegertracing/jaeger-client-csharp/issues/178#issuecomment-643266115 As far as I remember, I just removed all references and added only the Jaeger one via nuget. Here is an excerpt from one of my vb.net project files. Hope that helps.
<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="12.0">
<PropertyGroup>
...
<TargetFrameworkVersion>v4.6.1</TargetFrameworkVersion>
...
<ItemGroup>
<Reference Include="Jaeger, Version=0.3.7.0, Culture=neutral, PublicKeyToken=9516061555a213a8, processorArchitecture=MSIL">
<HintPath>..\packages\Jaeger.0.3.7\lib\netstandard2.0\Jaeger.dll</HintPath>
</Reference>
<Reference Include="Jaeger.Thrift, Version=0.3.7.0, Culture=neutral, PublicKeyToken=9516061555a213a8, processorArchitecture=MSIL">
<HintPath>..\packages\Jaeger.Thrift.0.3.7\lib\netstandard2.0\Jaeger.Thrift.dll</HintPath>
</Reference>
<Reference Include="Jaeger.Thrift.VendoredThrift, Version=1.0.0.1, Culture=neutral, PublicKeyToken=9516061555a213a8, processorArchitecture=MSIL">
<HintPath>..\packages\Jaeger.Thrift.VendoredThrift.0.3.7\lib\netstandard2.0\Jaeger.Thrift.VendoredThrift.dll</HintPath>
</Reference>
...
<Reference Include="OpenTracing, Version=0.12.1.0, Culture=neutral, PublicKeyToken=61503406977abdaf, processorArchitecture=MSIL">
<HintPath>..\packages\OpenTracing.0.12.1\lib\net45\OpenTracing.dll</HintPath>
</Reference>
...My app.config contains nothing special.
yury-kozlov
commented
3 years ago Thank you @Lercher. As I understand, your nuget is Jaeger, Version=0.3.7.0, where everything works fine.
The issue probably started happening in newer versions of Jaeger, for example I experience it in the latest "Jaeger" version="1.0.0".
@Falco20019 This may be related: Could not load file or assembly Thrift, Version=0.13.0.0. In their fix I see that they explicitly provide a hint for Thrift.dll:
<Reference Include="ApacheThrift">
<HintPath>$(PkgApacheThrift)\lib\netstandard2.0\Thrift.dll</HintPath>
</Reference>That‘s a manual workaround that usually should not be necessary. I will need to look into it but won‘t be able to do so until around Thursday :(
The problem here is the one that I linked earlier. The build chain is resolving the transitive dependency using .NET Framework 4.5 instead of .NET Standard 2.0 (which is wrong). But I don‘t really understand why it‘s doing that…
The fix is also adding the following line for setting PkgApacheThrift:
<PackageReference Include="ApacheThrift" Version="0.13.0.1" ExcludeAssets="all" GeneratePathProperty="true" />
yury-kozlov
commented
3 years ago Thanks!
On the example of OpenTelemetry.Exporter.Jaeger I see that in their later commits they removed the dependency ApacheThrift and copied apache sources directly into the package.
So now I'm not sure if their original fix of Thrift.dll was right :(
From the readme:
This was done because the official NuGet has two issues:
- . . .
- The nupkg contains a net45 library with a different API than the .NET Standard 2.0 library. This breaks .NET Framework consumers of OpenTelemetry using Jaeger unless we force selection of the lib/netstandard2.0 reference instead.
Ideally we would consume the official package but these issues made it difficult.
Falco20019
commented
3 years ago Since this issue was closed, let‘s continue the discussion on #212
Requirement - what kind of business use case are you trying to solve?
Instrumenting an existing .Net Framework 4.6.1 application and using UDP for communication with a local jaeger agent.
Problem - what in Jaeger blocks you from solving the requirement?
The Thrift reference. Adding
New Jaeger.Senders.Thrift.UdpSender()to the remote reporter builder raises the exceptionI've fiddled with this situation and I guess the root cause is that the nuget package of Apache thrift publishes a thrift.dll for netstandard and a thrift45.dll for net45, which is used for the net461 project. Moreover these two dlls expose quite a different public API. Manually referencing thrift's netstandard2 library seems to work formally, but I have a bad feeling about this.
Proposal - what do you suggest to solve the problem or improve the existing situation?
I really don't know. Experimentally copying the client's sources to new net461 cs-projects, using thrift's nuget and building it fails b/c of thrift's different API for net45/netstandard.
Any open questions to address
Importing thrift induces a lot of dependencies. If there is need to export a COM API all must be GAC-able and thus strong named. Moving all these dependencies to the jaeger agent (written in go, so there is no actual "moving") and having a very slim variant of the client that can only communicate with the agent via a single fixed protocol could be an option.