Skip to content
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

Logs: add SpanId and ActivityId to each LogEvent producer by ILoggingAdapter #6855

Open
Aaronontheweb opened this issue Jul 27, 2023 · 3 comments · May be fixed by #7476
Open

Logs: add SpanId and ActivityId to each LogEvent producer by ILoggingAdapter #6855

Aaronontheweb opened this issue Jul 27, 2023 · 3 comments · May be fixed by #7476

Comments

@Aaronontheweb
Copy link
Member

Is your feature request related to a problem? Please describe.

Related: serilog/serilog#1923

Will add some overhead to our logging system (System.Diagnostics.Activity is in the BCL, so no extra dependencies though) but might make it much easier to correlate Akka.NET system + user actor activity to traces in the future.

@nblumhardt
Copy link

Just circling around to the linked issue :-) ... would the end result be better/worse for Akka.net depending on whether we push that one forward?

@Aaronontheweb
Copy link
Member Author

Took a look at this some today and it's not going to be as simple as I thought:

  1. The way MSFT.EXT.Logging / OpenTelemetry appear to do this, is they have to be active at the callsite where the log is collected - there's not really a method to pass the ActivityContext into the ILogger.Log method (which is really the OpenTelemetryLogger after that's configured) after the fact.

Given the asynchronous way we process logs, this is problematic - we can capture the ActivityContext and attach it to the LogEvent record in Akka.NET and provide that as structured data to the logger, but that leads to a bridge to nowhere unfortunately.

  1. The other approach we can use is simply to pass the entire Activity into the LogRecord and then do a quick hack inside something like the SerilogLogger or the LoggerFactoryLogger:
 protected virtual void Log(LogEvent log, ActorPath path)
{
	// this is going to be `null` 99.9999999999999999% of the time
	var previous = Activity.Current;
	try{
		Activity.Current = log.Activity; // grab the activity from the ILoggingAdapter output

		// the OTEL logger, if configured, will pick it up here
		_akkaLogger.Log<LogEvent>(GetLogLevel(log.LogLevel()), new EventId(), log, log.Cause, (@event, exception) => @event.ToString());
	}
	finally{
		Activity.Current = previous; // put the old value back on the stack
	}
}

I don't see any reason why that wouldn't work - maybe we can give that a shot.

@Aaronontheweb
Copy link
Member Author

And we can still, of course, append the TraceId and SpanId to our own messages if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants