[Akka.Streams] make default LogSources actually usable
#7168
Conversation
If a `LogSource` is passed into `LogSource.Create`, just return that.
| @@ -3440,6 +3440,7 @@ namespace Akka.Event | |||
| public System.Type Type { get; } | |||
| public static Akka.Event.LogSource Create(object o) { } | |||
| public static Akka.Event.LogSource Create(object o, Akka.Actor.ActorSystem system) { } | |||
| public static Akka.Event.LogSource Create(string source, System.Type t) { } | |||
There was a problem hiding this comment.
Public API - this basically calls the CTOR of LogSource.
|
|
||
| // check just the first log message | ||
| var logMessage = logProbe.ExpectMsg<Debug>(); | ||
| logMessage.LogSource.Should().Contain("StreamSupervisor"); |
There was a problem hiding this comment.
Laying down some regression tests here to make sure someone doesn't cleverly optimize-away useful data out of the LogSource name for these stages.
| public override ILoggingAdapter MakeLogger(object logSource) => Logging.GetLogger(System, logSource); | ||
| public override ILoggingAdapter MakeLogger(object logSource) | ||
| { | ||
| if (logSource is not LogSource s) return Logging.GetLogger(System, logSource); |
There was a problem hiding this comment.
Use the old code if someone's not using a LogSource - I'm open to changing this so everything does it the new way.
There was a problem hiding this comment.
Done, went ahead and made this change.
| @@ -402,7 +408,7 @@ public override void Shutdown() | |||
| Supervisor.Tell(PoisonPill.Instance); | |||
| } | |||
|
|
|||
| private ILoggingAdapter GetLogger() => _system.Log; | |||
| private ILoggingAdapter GetLogger() => MakeLogger(_supervisor); | |||
There was a problem hiding this comment.
Always create a new logger, don't filch and just use the ActorSystem's one.
| @@ -840,7 +840,7 @@ protected GraphStageLogic(int inCount, int outCount) | |||
| /// <param name="shape">TBD</param> | |||
| protected GraphStageLogic(Shape shape) : this(shape.Inlets.Count(), shape.Outlets.Count()) | |||
| { | |||
| LogSource = Akka.Event.LogSource.Create(shape); | |||
| LogSource = Akka.Event.LogSource.Create(shape.ToString()); | |||
There was a problem hiding this comment.
Get the full details on the shape so we can see input and output ports too.
| return new LogSource(FromType(t, system), t); | ||
| case LogSource logSource: | ||
| return logSource; // if someone's already created a LogSource, just use it |
There was a problem hiding this comment.
If someone calls LogSource.Create with a ... LogSource... as input, just use the LogSource. This was "eating" user-defined LogSource names before.
Changes
Fixes #7126
So TL;DR;, logging from within an Akka.Streams graph stage is basically unusable and has been since the inception of Akka.Streams.
Behold, what my logs look like from my Akka.Streams-powered MQTT client library:
There are about 10 different stream stages in there, each of which get created twice (rebooting a failed connection) - I can't tell any of them apart, and I know I have a bug in that code somewhere.
Thus, I've made this PR and now these logs will look more like this going forward:
The default
LogSourcefor any Akka.Streams stage, including built in ones is now{shape.ToString()}({streamSupervisorActorPath}).The
shape.ToString()method does the following by default:Which will print out the type of stage and a rough description of its inputs and outputs, in accordance with the names given to those Inlet and Outlet types. That will add a small amount of overhead to each stage we create (a
.ToString()call that iterates over some types that return hard-coded strings back) but in return you can now immediately understand:Moreover, if you override the
GraphStage.LogSourcewith a friendlier name we will still append theStreamSupervisor.Pathto it during logger construction, so you can still keep track of where this stream is within the actor hierarchy.TL;DR; this is a substantial usability improvement for Akka.Streams.
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
LogSourcename #7126