Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

No custom logs when using Serilog #42

Closed
Marusyk opened this issue May 10, 2019 · 5 comments
Closed

No custom logs when using Serilog #42

Marusyk opened this issue May 10, 2019 · 5 comments

Comments

@Marusyk
Copy link

Marusyk commented May 10, 2019

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?

@nblumhardt
Copy link

This is a limitation in the Serilog logger factory implementation; in particular, Serilog currently ignores added providers and assumes that Serilog Sinks will replace them instead. Some work is underway to alleviate this with: serilog/serilog-extensions-logging#132 - any and all input/assistance appreciated.

(Another, probably quicker short-term option might be to implement a simple WriteTo.OpenTracing() method to connect Serilog directly to OpenTracing without using MEL as the intermediary - happy to help out if someone is exploring that approach.)

@Marusyk
Copy link
Author

Marusyk commented May 16, 2019

@nblumhardt Thank you very much for response. It would be great to have some solution for now. Could you please suggest how to implement WriteTo.OpenTracing() or share any tutorials/articles?
Thank you in advance.

@nblumhardt
Copy link

Hi @Marusyk

WriteTo.OpenTracing() would just be a convenience wrapper for WriteTo.Sink(new OpenTracingSink(...)), so the sink class would be the best place to start.

This would need to implement Serilog's ILogEventSink, and do almost exactly what this method does - accept a LogEvent from Serilog, convert its properties into a fields dictionary, and pass these through _tracer.ActiveSpan.Log(...).

There are quite a lot of implementations of ILogEventSink out there to examine for examples (any Serilog.Sink.* will have one.

The trick will be how to get an ITracer into the sink, somehow - either via its constructor or some other mechanism. Order of initialization can be a bit tricky but generally there's a way :-)

Hope this helps!
Nick

@Marusyk
Copy link
Author

Marusyk commented May 20, 2019

Thanks a lot. It was very helpful. My implementations of that:

public class OpenTracingSink : ILogEventSink
{
	private readonly ITracer _tracer;
	private readonly IFormatProvider _formatProvider;

	public OpenTracingSink(ITracer tracer, IFormatProvider formatProvider)
	{
		_tracer = tracer;
		_formatProvider = formatProvider;
	}

	public void Emit(LogEvent logEvent)
	{
		ISpan span = _tracer.ActiveSpan;

		if (span == null)
		{
			// Creating a new span for a log message seems brutal so we ignore messages if we can't attach it to an active span.
			return;
		}

		var fields = new Dictionary<string, object>
		{
			{ "component", logEvent.Properties["SourceContext"] },
			{ "level", logEvent.Level.ToString() }
		};

		fields[LogFields.Event] = "log";

		try
		{
			fields[LogFields.Message] = logEvent.RenderMessage(_formatProvider);
			fields["message.template"] = logEvent.MessageTemplate.Text;

			if (logEvent.Exception != null)
			{
				fields[LogFields.ErrorKind] = logEvent.Exception.GetType().FullName;
				fields[LogFields.ErrorObject] = logEvent.Exception;
			}

			if (logEvent.Properties != null)
			{
				foreach (var property in logEvent.Properties)
				{
					fields[property.Key] = property.Value;
				}
			}
		}
		catch (Exception logException)
		{
			fields["mbv.common.logging.error"] = logException.ToString();
		}

		span.Log(fields);
	}
}

public static class OpenTracingSinkExtensions
{
	public static LoggerConfiguration OpenTracing(
			  this LoggerSinkConfiguration loggerConfiguration,
			  IFormatProvider formatProvider = null)
	{
		return loggerConfiguration.Sink(new OpenTracingSink(GlobalTracer.Instance, formatProvider));
	}
}

After merge your PR, should it work without this sink?

@nblumhardt
Copy link

Fantastic, thanks for the update @Marusyk

After we have the new functionality merged, there will need to be some downstream changes before this will be picked up, but ideally - yes, the goal is to not require this.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants