Make sure stdout-loglevel setting is honored through the whole actor system lifecycle by Arkatufus · Pull Request #5251 · akkadotnet/akka.net

Arkatufus · 2021-09-03T19:24:52Z

…system lifecycle

Arkatufus · 2021-09-03T19:26:27Z

+        {
+            try
+            {
+                _output.WriteLine(e.ToString());


Add missing format exception catcher to Akka.TestKit.Xunit that was added to Akka.TestKit.Xunit2

Arkatufus · 2021-09-03T19:26:57Z

        /// <returns>TBD</returns>
        protected override bool Receive(object message)
        {
-            if(message is InitializeLogger)


Change if...then...else structure to switch

Arkatufus · 2021-09-03T19:29:13Z

            LogLevel = Config.GetString("akka.loglevel", null);
            StdoutLogLevel = Config.GetString("akka.stdout-loglevel", null);
+
+            var stdoutClassName = Config.GetString("akka.stdout-logger-class", null);


Moved the hard-wired StandardOutLogger instance from the Logging class to the Settings class.
Default StandardOutLogger is now customizable through the HOCON configuration

I believe this was @zbynek001's suggestion in one of those threads - we should document this inside https://getakka.net/articles/utilities/logging.html#standard-loggers

Arkatufus · 2021-09-03T19:29:50Z

+
+  # Fully qualified class name (FQCN) of the very basic logger used on startup and shutdown.
+  # You can substitute the logger by supplying an FQCN to a custom class that implements Akka.Event.MinimumLogger
+  stdout-logger-class = "Akka.Event.StandardOutLogger"


New HOCON setting to customize the standard out logger

Arkatufus · 2021-09-03T19:31:50Z

+        /// <summary>
+        /// The off log level.
+        /// </summary>
+        OffLevel = int.MaxValue


Added OffLevel to LogLevel enum, this would act as a marker for the off state

I'd revert this change - it's a major part of the public API and will require an update to all downstream loggers probably.

Arkatufus · 2021-09-03T19:33:12Z


 namespace Akka.Event
 {
+    public abstract class MinimalLogger : MinimalActorRef


MinimalLogger is the abstract base class for implementing minimal loggers.

In a separate PR, we should add some documentation for this

Aaronontheweb

We should probably leave the LogLevel enum alone - as that's a pretty viral change without much benefit, but other than that this PR looks good. Need to add some documentation as well.

Aaronontheweb · 2021-09-06T14:31:51Z

+        {
+            try
+            {
+                _output.WriteLine(e.ToString());


Aaronontheweb · 2021-09-06T14:38:26Z

+        }
+
+        [Fact(DisplayName = "StandardOutLogger should not be called on shutdown when stdout-loglevel is set to OFF")]
+        public async Task StandardOutLoggerShouldNotBeCalled()


Aaronontheweb · 2021-09-06T14:39:17Z

            LogLevel = Config.GetString("akka.loglevel", null);
            StdoutLogLevel = Config.GetString("akka.stdout-loglevel", null);
+
+            var stdoutClassName = Config.GetString("akka.stdout-logger-class", null);


I believe this was @zbynek001's suggestion in one of those threads - we should document this inside https://getakka.net/articles/utilities/logging.html#standard-loggers

Aaronontheweb · 2021-09-06T14:39:43Z

+                }
+                catch (MissingMethodException)
+                {
+                    throw new MissingMethodException(


Aaronontheweb · 2021-09-06T14:40:44Z

    /// This class represents a message used to notify subscribers that a logger has been initialized.
    /// </summary>
-    public class LoggerInitialized : INoSerializationVerificationNeeded
+    public class LoggerInitialized : INoSerializationVerificationNeeded, IDeadLetterSuppression


Aaronontheweb · 2021-09-06T14:42:36Z

+        /// <summary>
+        /// The off log level.
+        /// </summary>
+        OffLevel = int.MaxValue


I'd revert this change - it's a major part of the public API and will require an update to all downstream loggers probably.

Aaronontheweb

Some more cleanup needed to undo the LogLevel.Off change

Aaronontheweb · 2021-09-06T15:50:55Z

                case LogLevel.ErrorLevel:
                    return Error;
-                case OffLogLevel:
-                    return Off;


Need to add this back

Aaronontheweb · 2021-09-06T15:51:13Z

                    return LogLevel.ErrorLevel;
                case Off:
-                    return OffLogLevel;
+                    return (LogLevel) int.MaxValue;


Should add back previous call

…ufus/akka.net into Add_new_StandardOutLogger_features

Aaronontheweb · 2021-09-06T16:00:00Z

        /// </summary>
        /// <param name="logEvent">The event to print</param>
-        public static void PrintLogEvent(LogEvent logEvent)
+        internal static void PrintLogEvent(LogEvent logEvent)


Aaronontheweb

LGTM

Aaronontheweb · 2021-09-06T16:00:34Z


 namespace Akka.Event
 {
+    public abstract class MinimalLogger : MinimalActorRef


In a separate PR, we should add some documentation for this

Arkatufus · 2021-09-06T19:52:32Z

~~Don't merge just yet, I just spotted a few problems that needs to be changed, plus the documentation.~~

Arkatufus · 2021-09-06T20:19:02Z

Done, documentation added.

Arkatufus · 2021-09-07T15:48:57Z

Should be ready for review now

Aaronontheweb

LGTM

Aaronontheweb · 2021-09-08T15:07:46Z

+`akka.stdout-loglevel` HOCON settings to `OFF` if you do not need these feature in your application.
+
+### Advanced MinimalLogger Setup
+You can also replace `StandardOutLogger` by making your own logger class with an empty constructor 


Probably need to do more showing rather than telling here, but this is good for now

tometchy · 2021-09-15T10:04:50Z

        {
            var logLevel = Logging.LogLevelFor(config.StdoutLogLevel);
-            SubscribeLogLevelAndAbove(logLevel, Logging.StandardOutLogger);
+            SubscribeLogLevelAndAbove(logLevel, config.StdoutLogger);


This line change is what PR title says, right?
Before change in this line (229) akka.stdout-loglevel wasn't honored and now, after this line has changed, it is honored, so off means off even during shutdown, right?

Now, when you set stdout-loglevel to off, nothing will be outputted by the StandardOutLogger, correct.

tometchy · 2021-09-15T10:06:22Z

        /// </summary>
-        /// <param name="config">The configuration used to configure the <see cref="StandardOutLogger"/>.</param>
+        /// <param name="config">The configuration used to configure the <see cref="MinimalLogger"/>.</param>
        public void StartStdoutLogger(Settings config)


If I get it right, it's a bit misleading, that if you set your own MinimalLogger then you need to configure it's logging level through akka.stdout-loglevel not for example akka.minimallogger-loglevel.

StandardOutLogger derives from MinimalLogger, it is the barest minimum logger used to log how the system behave without actual loggers running, ie. a logger to log the loggers themselves, it runs at the very start and the very end of the ActorSystem life cycle. The only thing you can make by inheriting MinimalLogger is a replacement for the StandardOutLogger, thus it will use the same settings as StandardOutLogger.

You would never do this under normal circumstances, it is considered as an advanced configuration/customization

Make sure stdout-loglevel setting is honored through the whole actor …

0289b2b

…system lifecycle

Arkatufus commented Sep 3, 2021

View reviewed changes

Merge branch 'dev' into Add_new_StandardOutLogger_features

3f962db

Arkatufus requested a review from Aaronontheweb September 3, 2021 19:35

Arkatufus added 5 commits September 4, 2021 02:45

Add settings spec for the new StandardOutLogger setting

a2e2505

Update API Approver list

b36f917

Test logger can still output logs

38b93b2

Suppress LoggerInitialized from deadletter

a444df7

Update API Approver list

135ec8b

Aaronontheweb requested changes Sep 6, 2021

View reviewed changes

Arkatufus and others added 3 commits September 6, 2021 22:13

Remove LogLevel.OffLevel for backward compatibility

ba1d0d4

Update API Approver list

16db579

Merge branch 'dev' into Add_new_StandardOutLogger_features

086a2bf

Aaronontheweb requested changes Sep 6, 2021

View reviewed changes

Arkatufus added 2 commits September 6, 2021 22:57

Re-add OffLogLevel private const

bdcb56d

Merge branch 'Add_new_StandardOutLogger_features' of github.com:Arkat…

bf5ea5a

…ufus/akka.net into Add_new_StandardOutLogger_features

Aaronontheweb reviewed Sep 6, 2021

View reviewed changes

Aaronontheweb approved these changes Sep 6, 2021

View reviewed changes

Aaronontheweb enabled auto-merge (squash) September 6, 2021 16:00

Arkatufus added 2 commits September 7, 2021 01:28

Merge branch 'dev' into Add_new_StandardOutLogger_features

926aab3

Merge branch 'dev' into Add_new_StandardOutLogger_features

42d5d59

Arkatufus disabled auto-merge September 6, 2021 19:51

Arkatufus added 2 commits September 7, 2021 03:17

Fix XML doc and previously hardwired StandardOutLogger type checking.

a09a755

Add documentation to the new MinimalLogger implementation

c1a739a

Merge branch 'dev' into Add_new_StandardOutLogger_features

fd223c3

Arkatufus requested a review from Aaronontheweb September 6, 2021 20:19

Arkatufus and others added 6 commits September 7, 2021 05:29

Merge branch 'dev' into Add_new_StandardOutLogger_features

52d5b3c

Merge branch 'dev' into Add_new_StandardOutLogger_features

7babaf3

Merge branch 'dev' into Add_new_StandardOutLogger_features

541d9dd

Change equality to inheritance check

8e196c9

Add MinimalLogger spec

2108dfe

Merge branch 'dev' into Add_new_StandardOutLogger_features

783d192

Merge branch 'dev' into Add_new_StandardOutLogger_features

0d24266

Aaronontheweb approved these changes Sep 8, 2021

View reviewed changes

Aaronontheweb merged commit 8279ed4 into akkadotnet:dev Sep 8, 2021

tometchy reviewed Sep 15, 2021

View reviewed changes

Conversation

Arkatufus commented Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arkatufus commented Sep 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Arkatufus commented Sep 6, 2021

Uh oh!

Arkatufus commented Sep 7, 2021

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arkatufus Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arkatufus Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Arkatufus commented Sep 3, 2021 •

edited

Loading

Arkatufus commented Sep 6, 2021 •

edited

Loading

Arkatufus Sep 15, 2021 •

edited

Loading

Arkatufus Sep 15, 2021 •

edited

Loading