jaegertracing / jaeger-client-csharp

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

No custom logs when using Serilog #146

Closed Marusyk closed 5 years ago

Marusyk commented 5 years ago

Requirement - what kind of business use case are you trying to solve?

I need to send to Jaeger custom logs. Now it works only when I use Microsoft.Extensions.Logging.ILoggerFactory

My Startup

public void ConfigureServices(IServiceCollection services)
{
    services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_2);

    services.AddOpenTracing();
    services.AddSingleton<ITracer>(sp =>
    {
        var loggerFactory = sp.GetRequiredService<ILoggerFactory>();

        var reporter = new RemoteReporter.Builder()
            .WithSender(new UdpSender())
            .WithLoggerFactory(loggerFactory)
            .Build();       

        var tracer = new Tracer
                .Builder("Sample service")
            .WithReporter(reporter)
            .WithSampler(new ConstSampler(true))
            .Build();

        GlobalTracer.Register(tracer);
        return tracer;
    });
}

Somewhere in controller: using Microsoft.Extensions.Logging;

namespace WebApplication2.Controllers
{
    [Route("api/[controller]")]
    public class ValuesController : ControllerBase
    {
        private readonly ILogger<ValuesController> _logger;

        public ValuesController(ILogger<ValuesController> logger)
        {
            _logger = logger;
        }

        // GET api/values/5
        [HttpGet("{id}")]
        public ActionResult<string> Get(int id)
        {
            _logger.LogWarning("Get values by id: {valueId}", id);
            return "value";
        }

Result image

Problem - what in Jaeger blocks you from solving the requirement?

But when I use Serilog, there are no any custom logs. I just install serilog and add

public static IWebHostBuilder CreateWebHostBuilder(string[] args)
{
    return WebHost.CreateDefaultBuilder(args)
        .UseStartup<Startup>()
        .UseSerilog();
}

Could you please suggest why custom logging doesn't work with Serilog?

Falco20019 commented 5 years ago

Please try moving the UseSerilog call before the UseStartup call. I assume since you are requesting the logger factory before registering it, that it should still be null.

Marusyk commented 5 years ago

didn't help. the same result

Marusyk commented 5 years ago

@Falco20019 Also I tried

return WebHost.CreateDefaultBuilder(args)
    .UseSerilog()
    .UseStartup<Startup>()
Falco20019 commented 5 years ago

I will have a more detailed look at it on Monday.

Marusyk commented 5 years ago

Ok, thank you in advance

Falco20019 commented 5 years ago

You call services.AddOpenTracing(); before registering the Singleton. According to the example this needs to be set afterwards since it’s accessing the global tracer which would not be registered yet, defaulting to a generic Tracer.

Additionally, you only assign the logger factory to the reporter but not the Tracer. You should use the configuration class to avoid having to do this manually.

I will write you an example tomorrow for your use case that also allows you to use the appsettings since you already rely on it

Marusyk commented 5 years ago

@Falco20019 Thanks but nothing help. I've tried call services.AddOpenTracing(); afterr egistering the Singleton - no results. Add to Tracer WithLoggerFactory. Tried use the configuration class. No results. It seems the problem somewhere else, because default micrisoft logger works well

public void ConfigureServices(IServiceCollection services)
{
    services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_2);

    services.AddSingleton<ITracer>(sp =>
    {
        var loggerFactory = sp.GetRequiredService<ILoggerFactory>();
        string serviceName = sp.GetRequiredService<IHostingEnvironment>().ApplicationName;

        var samplerConfiguration = new Configuration.SamplerConfiguration(loggerFactory)
            .WithType(ConstSampler.Type)
            .WithParam(1);

        var senderConfiguration = new Configuration.SenderConfiguration(loggerFactory)
            .WithAgentHost("localhost")
            .WithAgentPort(6831);

        var reporterConfiguration = new Configuration.ReporterConfiguration(loggerFactory)
            .WithLogSpans(true)
            .WithSender(senderConfiguration);

        var tracer = (Tracer)new Configuration(serviceName, loggerFactory)
            .WithSampler(samplerConfiguration)
            .WithReporter(reporterConfiguration)
            .GetTracer();

        //GlobalTracer.Register(tracer);

        return tracer;
    });
    services.AddOpenTracing();
}
Marusyk commented 5 years ago

Sample project https://github.com/Marusyk/JaegerSerilog

Falco20019 commented 5 years ago

Thanks for the sample. I will look into it but it sounds more like Serilog (or it‘s usage) is the problem if the normale ILogger works.

Thinking a bit more about what I wrote yesterday, it shouldn’t make a difference since the Singleton instance is created when needed and not at the place of the call.

I look into it in the next 2-3 hours.

Falco20019 commented 5 years ago

Your example is not taking the configuration from appsettings.json. According to the official example from serilog, you need to read from the configuration manually.

using Microsoft.AspNetCore;
using Microsoft.AspNetCore.Hosting;
using Serilog;
using System;
using System.IO;
using Microsoft.Extensions.Configuration;

namespace WebApplication2
{
    public class Program
    {
        // This was missing:
        public static IConfiguration Configuration { get; } = new ConfigurationBuilder()
            .SetBasePath(Directory.GetCurrentDirectory())
            .AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
            .AddJsonFile($"appsettings.{Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? "Production"}.json", optional: true)
            .AddEnvironmentVariables()
            .Build();

        public static int Main(string[] args)
        {
            Log.Logger = new LoggerConfiguration()
                // This was missing:
                .ReadFrom.Configuration(Configuration)
                .Enrich.FromLogContext()
                .WriteTo.Console()
                .CreateLogger();

            try
            {
                Log.Information("Starting host");
                CreateWebHostBuilder(args).Build().Run();
                return 0;
            }
            catch (Exception ex)
            {
                Log.Fatal(ex, "Host terminated unexpectedly");
                return 1;
            }
            finally
            {
                Log.CloseAndFlush();
            }
        }

        public static IWebHostBuilder CreateWebHostBuilder(string[] args)
        {
            return WebHost.CreateDefaultBuilder(args)
                .UseStartup<Startup>()
                // This was missing:
                .UseConfiguration(Configuration)
                .UseSerilog();
        }
    }
}

For configuring the ITracer from appsetting.json please use the following code:

services.AddSingleton<ITracer>(sp =>
{
    var loggerFactory = sp.GetRequiredService<ILoggerFactory>();

    var tracer = Jaeger.Configuration
        .FromIConfiguration(loggerFactory, Configuration.GetSection("Jaeger"))
        .GetTracer();

    GlobalTracer.Register(tracer);

    return tracer;
});

Your appsettings.json might look something like this then:

{
    "Serilog": {
        "MinimumLevel": {
            "Default": "Warning",
            "Override": {
                "Microsoft": "Warning",
                "System": "Warning"
            }
        }
    },
    "Jaeger": {
        "JAEGER_SERVICE_NAME": "service",
        "JAEGER_AGENT_HOST": "localhost",
        "JAEGER_AGENT_PORT": "6831",
        "JAEGER_SAMPLER_TYPE": "const",
        "JAEGER_SAMPLER_PARAM": "1"
    },
    "AllowedHosts": "*"
}

It looks like Serilog is not using the normal ILoggerProvider singleton that are regularily used and that is registered with AddOpenTracing(). I haven't used Serilog myself, so I can't give you any support on it. Especially since it is not a problem in Jaeger. Feel free to ask any questions, even if you are unsure if it results from Jaeger or any other library that you use and I will gladly look into it.

So please wait for a response on the correct issue that you already created: https://github.com/opentracing-contrib/csharp-netcore/issues/42