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

Customizable ILogMessageFormatter across entire ActorSystem #6413

Merged
merged 10 commits into from Feb 20, 2023

Conversation

Aaronontheweb
Copy link
Member

@Aaronontheweb Aaronontheweb commented Feb 19, 2023

Changes

Makes it possible to configure the ILoggerMessageFormatter across the entire ActorSystem via HOCON. This is one of the ingredients we need in order to enable semantic logging in Akka.NET by default.

This PR also has the benefit of removing this extremely annoying piece of Akka.Logging.Serilog syntax:

private readonly ILoggingAdapter _logger = Context.GetLogger<SerilogLoggingAdapter>();

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

Copy link
Member Author

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detailed my changes

@@ -1720,6 +1720,7 @@ namespace Akka.Actor
public int LogDeadLetters { get; }
public bool LogDeadLettersDuringShutdown { get; }
public System.TimeSpan LogDeadLettersSuspendDuration { get; }
public Akka.Event.ILogMessageFormatter LogFormatter { get; }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposed the ILogMessageFormatter on the Settings class.

@@ -3000,7 +3001,7 @@ namespace Akka.Event
public Akka.Actor.IActorRef Sender { get; }
public override string ToString() { }
}
public class BusLogging : Akka.Event.LoggingAdapterBase
public sealed class BusLogging : Akka.Event.LoggingAdapterBase
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sealed BusLogging and I'm considering making the constructor private in a subsequent PR so it can't be new'd up any longer - and will instead have to be populated by the ActorSystem. Considering doing that in order to enforce that configurable components like the ILogMessageFormatter and others are injected consistently across all call sites.

}

[Fact]
public async Task ShouldUseValidCustomLogFormatter()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validates that custom ILogMessageFormatter output actually makes it into the output events

}

[Fact]
public async Task ShouldDetectCustomLogFormatterOutputInEventFilter()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validates that the TestKit's EventFilter can also pick up custom formatter output.

@Aaronontheweb
Copy link
Member Author

I've updated the logging page and the serilog page in the documentation to reflect use of this feature. Have not updated the "what's new in v1.5" page yet (planning on doing that closer to release time.)

@Aaronontheweb Aaronontheweb changed the title Pluggable logging Customizable ILogMessageFormatter across entire ActorSystem Feb 19, 2023
Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, just one question on the documentation

---

# Logging

For more info see real Akka's documentation: <http://doc.akka.io/docs/akka/2.0/scala/logging.html>
> ![NOTE]
> For info
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty documentation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I got interrupted mid-sentence. I'll go finish that.

@Arkatufus Arkatufus enabled auto-merge (squash) February 20, 2023 18:03
@Aaronontheweb Aaronontheweb merged commit e8e3569 into akkadotnet:dev Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants