Open tuansoibk opened 4 years ago
Thanks for the comment. I understand the proposed fixes, but before making a decision I'd like to understand the issue a bit more - can you explain the TraceId and SpanId properties that serilog is adding and what are they used for? I hadn't seen these when I created this lib... if we go with option #A we'd be clobbering the values set by serilog with our own values (which might not be desirable)...
Hi, I did a deeper investigation and find out that the TraceId & SpanId were actually added by Asp.Net Core. Check this out: https://github.com/dotnet/aspnetcore/blob/master/src/Hosting/Hosting/src/Internal/HostingLoggerExtensions.cs
The ids are added via a HostingLogScope
, then wrapped into SerilogLoggerScope
. Later, when SerilogLoggerScope
enriches the log event, it takes those ids from the internal HostingLogScope
.
Let me know if it is still not clear.
TraceId
&SpanId
have already been added to the SerilogLogEvent
bySerilogLoggerProvider
(viaSerilogLoggerScope
), so theTraceId
&SpanId
appear in log message are from Serilog scope but not OpenTracing span.Proposed fixes: either A or B A. In
OpenTracingContextLogEventEnricher.cs
, we can uselogEvent.AddOrUpdateProperty
instead oflogEvent.AddPropertyIfAbsent
, in order to overwritten Serilog scope properties B. Use new property names, such as: OtTraceId & OtSpanId