jaegertracing / jaeger-client-csharp

🛑 This library is DEPRECATED!
https://jaegertracing.io/
Apache License 2.0
302 stars 67 forks source link

span size calculation issue #216

Closed Meiri closed 3 years ago

Meiri commented 3 years ago

im not sure its a bug, might be missing/wrong config

span size keep increasing in ThriftSender/AppendAsync

Steps to reproduce the behavior:

  1. basic asp.net 5.0 web api template
  2. in startup config: `
    services.AddOpenTracing(builder => { builder.ConfigureAspNetCore(options => { options.Hosting.IgnorePatterns.Add(x => { return x.Request.Method == "GET"; }); options.Hosting.IgnorePatterns.Add(x => { return x.Request.Path == "/metrics"; }); }); }); services.AddSingleton(serviceProvider => { var serviceName = serviceProvider.GetRequiredService().ApplicationName; var loggerFactory = serviceProvider.GetRequiredService();

            var reporter = new RemoteReporter.Builder()
                .WithLoggerFactory(loggerFactory)
                .WithSender(new MyUdpSender("host", 6831))
                .Build();
    
            // This will log to a default localhost installation of Jaeger.
            var tracer = new Tracer.Builder(serviceName)
                .WithLoggerFactory(loggerFactory)
                .WithSampler(new ConstSampler(true))
                .WithReporter(reporter)
                .Build();
    
            // Allows code that can't use DI to also access the tracer.
            if(!GlobalTracer.IsRegistered())
                GlobalTracer.Register(tracer);
    
            return tracer;
        });`
  3. run a couple of http to the default controller action when you reach spansize the is over 65K, sender exception will be thrown.

the issue is in the thrift sender base, in the getsize function public int GetSize(TBase thriftBase) { _memoryTransport.Length = 0; thriftBase.WriteAsync(ProtocolFactory.GetProtocol(_memoryTransport), CancellationToken.None).GetAwaiter().GetResult(); return _memoryTransport.Length; }

my "fix" was to use a local variable instead of _memoryTransport

public int GetSize(TBase thriftBase) { using (var memoryTransport = new TMemoryBufferTransport(new TConfiguration())) { thriftBase.WriteAsync(ProtocolFactory.GetProtocol(memoryTransport), CancellationToken.None).GetAwaiter().GetResult(); return memoryTransport.Length; } }

im not sure its a bug, maybe config issue.

Falco20019 commented 3 years ago

Looks like a bug. We switched from our own MemoryTransport implementation to the one in ApacheThrift. Maybe they are not behaving the same.

I will give it a look the next hours.

Falco20019 commented 3 years ago

@Meiri Thanks for your report. This was leading to a memory leak. I fixed it on the PR and will release it as 1.0.2 as soon it's reviewed since it's a critical bug.

Falco20019 commented 3 years ago

I just released v1.0.2 which should resolve your issue. If you still run into issues, let us know!

Meiri commented 3 years ago

happy to help, thanks for the quick response