-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trace fields are seems to be incorrect #44
Comments
thanks for reporting @StasPerekrestov fixed now in version 3.3.3: https://www.nuget.org/packages/Serilog.Sinks.GoogleCloudLogging/3.3.3 |
Thank you so much! |
In my initial PR, i used that piece of code (works / tested): // Maybe we could cache this, but the traceId will change often
entry.Trace = $"projects/{_projectId}/traces/{traceId.StringValue}";
propStruct.Fields.Remove("TraceId");
...
entry.SpanId = spanId.StringValue;
propStruct.Fields.Remove("SpanId"); It seems, it went broken after merge & refactor: But glad to see, that @manigandham fixed that so fast :-) Thanks!! :-) |
I'm not sure if it should be part of this repository, It's something that doesn't work for me because of: Also, please note there are some behavioral differences between asp.net core 3.1 and 5.0, which should be taken into consideration. internal class GoogleTraceActivityEnricher : ILogEventEnricher
{
private readonly IHttpContextAccessor _httpContextAccessor;
public GoogleTraceActivityEnricher(IHttpContextAccessor httpContextAccessor)
{
_httpContextAccessor = httpContextAccessor;
}
public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
{
HttpContext? ctx = _httpContextAccessor.HttpContext;
if (ctx == null)
{
Activity? activity = Activity.Current;
if (activity != null)
{
//..fallback: grab values from Activity: TraceId, SpanId, ParentId
}
}
else
{
var managedTracer = ctx.RequestServices.GetRequiredService<IManagedTracer>();
string? traceId = managedTracer.GetCurrentTraceId();
ulong? spanId = managedTracer.GetCurrentSpanId();
//in .net 3.1 TraceId/SpanId are propagated automatically and we need to override it with Google TraceId
if (traceId != null)
{
logEvent.AddOrUpdateProperty(new LogEventProperty("TraceId", new ScalarValue(traceId)));
}
if (spanId != null)
{
logEvent.AddOrUpdateProperty(new LogEventProperty("SpanId", new ScalarValue(spanId)));
}
}
}
} @manigandham FYI :) |
Hey @StasPerekrestov This repo is just for the sink which sends the logs to GCP. Adding the activity ids is the responsibility of the upstream logging provider. It's only an issue because serilog doesn't (yet) use |
Yeah, sorry, I perhaps confused you. I referenced the issue just to illustrate why some workarounds exist in the code above. If I use it allows seeing information logs in Trace List: |
Hello,
I've found out that according to
https://cloud.google.com/trace/docs/trace-log-integration
traces should have the following format
projects/[PROJECT_ID]/traces/[TRACE_ID]
however, in LogFormatter.cs code
produces Trace references like
projects/vitupay-oos/traces/TraceId
SpanIds are also suspicious.
As I understand, it should be
The text was updated successfully, but these errors were encountered: