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

Mention MinimumLevel.Debug in README re #17 #18

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Reeceeboii
Copy link

Fixes #17.

Please let me know if you think anything needs adding or wording needs changing.

Added instructions for chaining debug level call.
Also the same for the XML and JSON config.
@@ -15,6 +15,7 @@ Then enable the sink using `WriteTo.Debug()`:
```csharp
Log.Logger = new LoggerConfiguration()
.WriteTo.Debug()
.MinimumLevel.Debug()
Copy link
Member

@bartelink bartelink Aug 19, 2024

Choose a reason for hiding this comment

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

have been looking at the serilog readme, and the MinimumLevel is not mentioned there either.

Space in there is at a premium, so even though it might trip quite a few people up, there's a very high bar for making the readme longer.

Also conflicted about whether I'd put this line here as it might suggest to people some linkage between this Sink being called Debug and the min level being Debug ?

@@ -24,6 +25,10 @@ Log events will be printed to the debug output:

![Debug Output](https://raw.githubusercontent.com/serilog/serilog-sinks-debug/dev/assets/Screenshot.png)

The chaining of the `MinimumLevel.Debug()` call is required in order to log events of level `LogEventLevel.Debug`, as the default minimum level for a new `LoggerConfiguration` is `LogEventLevel.Information`.
Copy link
Member

@bartelink bartelink Aug 19, 2024

Choose a reason for hiding this comment

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

Perhaps instead of this, if we got a screenshot that did not include Debug output?

Or linked:

https://github.com/serilog/serilog/wiki/Configuration-Basics#minimum-level

with a message like

⚠️ The default minimum log level is Information. To output Debug (DBG in the above screenshot), the LoggerConfiguration must opt into the lower level, or the log message won't enter the output pipeline.

@bartelink
Copy link
Member

Thanks @Reeceeboii - I added two notes on the PR.

Your PR does the job, but on reflection, maybe the notice/example re the minimum level for a LoggerConfiguration belongs somewhere else (in Serilog Core?)

I do agree that it's misleading and infuriating for the config in the readme not to be able to produce the output in the screenshot without any hint as to how to bridge the gap - that will definitely wind up resolved. I'm not sure what the overall view is - let's wait for others to review and we'll get there.

@bartelink bartelink changed the title #17 README.md updates. Mention MinimumLevel.Debug in README re #17 Aug 19, 2024
@Reeceeboii
Copy link
Author

Roger that. Will hang tight.

This PR is open for maintainer changes so if I can't be reached in a timely manner and you have a way forward then I don't mind if others run this up 👍

@nblumhardt
Copy link
Member

I agree that the inclusion of MinimumLevel.Debug() here could further muddy the meaning of "debug" in the context of this sink 👍

What if we were to (at worst 😅) just edit the screenshot to show [INF] where [DBG] is currently shown?

Ideally, I think the page https://github.com/serilog/serilog/wiki/Getting-Started, which we link prominently from the README, would carry some information about minimum level settings.

@Reeceeboii
Copy link
Author

Reeceeboii commented Aug 20, 2024

I think part of the confusion from my end (that led to me raising #17 originally) was that I wanted to write Debug logs to the Debug console. In my mind, Information and up appearing there would have been a bonus, but I was coming at it from the perspective of wanting to see my Debug logs in an obvious place within VS.

I suppose my mind made the implicit link between the debug console sink and logs at a Debug level - despite the fact that the two don't necessarily go hand in hand by any means.

I do wonder if this is actually an issue worthy of a fix - if nobody else has raised this before, we could chalk it up to the way my brain was wired when attempting to integrate this sink.

It was confusing me until I had mentally uncoupled the two separate uses of 'debug'.
In hindsight, I don't think I would have actually raised the issue, as the explanation by @bartelink under #17 made sense the second I read it.

@bartelink
Copy link
Member

bartelink commented Aug 20, 2024

I do wonder if this is actually an issue worthy of a fix - if nobody else has raised this before, we could chalk it up to the way my brain was wired when attempting to integrate this sink.

I have not actually used the sink myself (and I should)
I've handrolled the equivalent, and had similar confusions / desires
sometimes I'm also looking to feed some traces to the XUnit TestOutput at the same time

Sometimes I want to be able to up the Debug window trace level when Debugger.IsAttached
And/or when I'm running a DEBUG build.

I dont know if there are good patterns for that written up somewhere in a blog post I missed somewhere.

I do think there is a case to answer, and this lib can be a place for a given right paragraph or two of examples or overviews to be housed as part of the overall offering.

But handwaving that vs actually writing it well are very different things!

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

Successfully merging this pull request may close these issues.

Debug logs not appearing without explicity setting minimum level
3 participants