Custom/logger #18258

Rick-Anderson · 2020-05-11T22:05:33Z

Public review:

Go to this DropBox link
In the upper right, select Download -> Direct download
Click on HTML files.

* Add a section in the docs aspnetcore/fundamentals/logging/index.md * Add a sample at aspnetcore/fundamentals/logging/loggermessage/samples/3.1/CustomLogger

Rick-Anderson · 2020-05-12T02:46:50Z

@JuergenGutsch please review the changed files and the public build.

cc @serpent5

JuergenGutsch · 2020-05-12T04:43:54Z

@Rick-Anderson Awesome. Looks good from my perspective 👍 Many Thanks 😊

JuergenGutsch · 2020-05-12T04:42:13Z

aspnetcore/fundamentals/logging/index/samples/3.x/CustomLogger/Controllers/HomeController.cs

-            _logger.LogWarning("The Index action was just started");
-            _logger.LogError("Not really an error. Just to show the custom logger");            
-            _logger.LogDebug("End Index action in the HomeController");
+            var routeInfo = ControllerContext.ToCtxString();


Much better 😄

Rick-Anderson · 2020-05-12T19:41:05Z

@serpent5 I had similar thoughts on the organization, but I just wanted to get it in a more readable state.

I think your suggestions would be a nice improvement.

I'll ping the PU and see if they can do a quick review.

Rick-Anderson · 2020-05-12T19:43:09Z

@BrennanConroy can you do a quick review or suggest a reviewer.

BrennanConroy

Looks pretty good

aspnetcore/fundamentals/logging/index.md

BrennanConroy · 2020-05-12T20:38:59Z

aspnetcore/fundamentals/logging/index.md

+
+[!code-csharp[](index/samples/3.x/CustomLogger/Startup1.cs?name=snippet)]
+
+The preceding code is verbose. The logger registration can be encapsulated, resulting in the following:


Most of this seems unneeded for this doc 🤷

BrennanConroy · 2020-05-12T20:39:35Z

aspnetcore/fundamentals/logging/index.md

+
+[!code-csharp[](index/samples/3.x/CustomLogger/ColoredConsoleLogger/ColoredConsoleLoggerProvider.cs?name=snippet)]
+
+In the preceding code, <xref:Microsoft.Build.Logging.LoggerDescription.CreateLogger*> creates a single instance of the `ColoredConsoleLogger` per category name and stores it in the <xref:System.Collections.Concurrent.ConcurrentDictionary`1>.


The ConcurrentDictionary xref seems broken

BrennanConroy · 2020-05-12T20:42:05Z

...amentals/logging/index/samples/3.x/CustomLogger/ColoredConsoleLogger/ColoredConsoleLogger.cs

+
+        public bool IsEnabled(LogLevel logLevel)
+        {
+            return logLevel == _config.LogLevel;


Might want to use >= for the check instead

Not, if you want to set the color for a specific log level

@JuergenGutsch reverted to your original code

BrennanConroy · 2020-05-12T20:44:42Z

aspnetcore/fundamentals/logging/index.md

+
+[!code-csharp[](index/samples/3.x/CustomLogger/ColoredConsoleLogger/ColoredConsoleLoggerConfiguration.cs?name=snippet)]
+
+The preceding code sets the default level to `Warning` and the color to `Yellow`. If the `EventId` is set to 0, we will log all events.


Why is the EventId being used to limit what is being logged?

It's just a stupid demo, to show how to add a custom demo. Maybe some things doesn't really make sense. This logger is just coloring the log outputs.
In that case you are able to create an different colored output for a specific event id.

BrennanConroy · 2020-05-12T20:45:28Z

+@JunTaoLuo in case you want to take a look

Co-authored-by: Brennan <brecon@microsoft.com>

Rick-Anderson · 2020-05-12T20:59:44Z

@BrennanConroy can you review @serpent5 outline

Rick-Anderson · 2020-05-13T20:54:56Z

@JuergenGutsch can you review my last commit?

JuergenGutsch · 2020-05-13T21:15:14Z

...amentals/logging/index/samples/3.x/CustomLogger/ColoredConsoleLogger/ColoredConsoleLogger.cs

@@ -22,7 +22,7 @@ public IDisposable BeginScope<TState>(TState state)

        public bool IsEnabled(LogLevel logLevel)
        {
-            return logLevel == _config.LogLevel;
+            return logLevel >= _config.LogLevel;


As mentioned in a comment of the review by @BrennanConroy this would enable the the output for a log level higher or equal to the configured one, instead of a specific one. This should be reflected in the text.

I call that out in the text.

JuergenGutsch · 2020-05-13T21:16:29Z

aspnetcore/fundamentals/logging/index.md

+The preceding code:
+
+* Creates a logger instance per category name.
+* Checks `logLevel == _config.LogLevel` in `IsEnabled`, so each `logLevel` has a unique logger. Generally, loggers should also be enabled for all higher log levels:


Checks logLevel == _config.LogLevel in IsEnabled

This doesn't match the code and the code snippet anymore, right?

@JuergenGutsch I reverted to your original code. The snippets are imported into the doc.

@JuergenGutsch oops, forgot to push that change. It's fixed now

JuergenGutsch and others added 2 commits May 11, 2020 21:18

Add custom logger documentaions (#11842)

a90e614

* Add a section in the docs aspnetcore/fundamentals/logging/index.md * Add a sample at aspnetcore/fundamentals/logging/loggermessage/samples/3.1/CustomLogger

Custom loggiing

2f4d4e9

Rick-Anderson mentioned this pull request May 11, 2020

Add custom logger documentation #18252

Closed

Rick-Anderson added 5 commits May 11, 2020 14:17

Custom loggiing

fa43485

Custom loggiing

1e55161

Custom loggiing

dd5b3e1

Custom loggiing

24978cd

Custom loggiing

cb0892c

JuergenGutsch approved these changes May 12, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

Rick-Anderson marked this pull request as ready for review May 12, 2020 19:41

BrennanConroy reviewed May 12, 2020

View reviewed changes

Apply suggestions from code review

ee2e8f5

Co-authored-by: Brennan <brecon@microsoft.com>

Rick-Anderson added 2 commits May 12, 2020 11:09

react to feedback

f509331

react to feedback

c97962a

JuergenGutsch reviewed May 13, 2020

View reviewed changes

react to feedback

3e78a33

JuergenGutsch approved these changes May 13, 2020

View reviewed changes

Rick-Anderson merged commit 9ab5adf into master May 13, 2020

Rick-Anderson deleted the custom/logger branch May 13, 2020 22:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom/logger #18258

Custom/logger #18258

Rick-Anderson commented May 11, 2020 •

edited

Loading

Rick-Anderson commented May 12, 2020

JuergenGutsch commented May 12, 2020

JuergenGutsch May 12, 2020

This comment has been minimized.

Rick-Anderson commented May 12, 2020

Rick-Anderson commented May 12, 2020

BrennanConroy left a comment

BrennanConroy May 12, 2020

BrennanConroy May 12, 2020

BrennanConroy May 12, 2020

JuergenGutsch May 13, 2020

Rick-Anderson May 13, 2020

BrennanConroy May 12, 2020

JuergenGutsch May 13, 2020

BrennanConroy commented May 12, 2020

Rick-Anderson commented May 12, 2020

Rick-Anderson commented May 13, 2020

JuergenGutsch May 13, 2020

Rick-Anderson May 13, 2020

JuergenGutsch May 13, 2020 •

edited

Loading

Rick-Anderson May 13, 2020

Rick-Anderson May 13, 2020


		[!code-csharp[](index/samples/3.x/CustomLogger/Startup1.cs?name=snippet)]

		The preceding code is verbose. The logger registration can be encapsulated, resulting in the following:


		[!code-csharp[](index/samples/3.x/CustomLogger/ColoredConsoleLogger/ColoredConsoleLoggerProvider.cs?name=snippet)]

		In the preceding code, <xref:Microsoft.Build.Logging.LoggerDescription.CreateLogger*> creates a single instance of the `ColoredConsoleLogger` per category name and stores it in the <xref:System.Collections.Concurrent.ConcurrentDictionary`1>.


		[!code-csharp[](index/samples/3.x/CustomLogger/ColoredConsoleLogger/ColoredConsoleLoggerConfiguration.cs?name=snippet)]

		The preceding code sets the default level to `Warning` and the color to `Yellow`. If the `EventId` is set to 0, we will log all events.

Custom/logger #18258

Custom/logger #18258

Conversation

Rick-Anderson commented May 11, 2020 • edited Loading

Rick-Anderson commented May 12, 2020

JuergenGutsch commented May 12, 2020

Choose a reason for hiding this comment

This comment has been minimized.

Rick-Anderson commented May 12, 2020

Rick-Anderson commented May 12, 2020

BrennanConroy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrennanConroy commented May 12, 2020

Rick-Anderson commented May 12, 2020

Rick-Anderson commented May 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JuergenGutsch May 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rick-Anderson commented May 11, 2020 •

edited

Loading

JuergenGutsch May 13, 2020 •

edited

Loading