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

Little cleanup #210

Merged
merged 1 commit into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ foreach ($src in ls src/*) {
} else {
& dotnet pack -c Release --include-source -o ..\..\artifacts
}
if($LASTEXITCODE -ne 0) { exit 1 }

if($LASTEXITCODE -ne 0) { exit 1 }

Pop-Location
}
Expand Down
9 changes: 5 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ _Microsoft.Extensions.Logging_ provides the `BeginScope` API, which can be used

Using the extension method will add a `Scope` property to your log events. This is most useful for adding simple "scope strings" to your events, as in the following code:

```cs
```csharp
using (_logger.BeginScope("Transaction")) {
_logger.LogInformation("Beginning...");
_logger.LogInformation("Completed in {DurationMs}ms...", 30);
Expand All @@ -101,7 +101,7 @@ using (_logger.BeginScope("Transaction")) {

If you simply want to add a "bag" of additional properties to your log events, however, this extension method approach can be overly verbose. For example, to add `TransactionId` and `ResponseJson` properties to your log events, you would have to do something like the following:

```cs
```csharp
// WRONG! Prefer the dictionary approach below instead
using (_logger.BeginScope("TransactionId: {TransactionId}, ResponseJson: {ResponseJson}", 12345, jsonString)) {
_logger.LogInformation("Completed in {DurationMs}ms...", 30);
Expand All @@ -119,11 +119,12 @@ using (_logger.BeginScope("TransactionId: {TransactionId}, ResponseJson: {Respon
// }
```

Not only does this add the unnecessary `Scope` property to your event, but it also duplicates serialized values between `Scope` and the intended properties, as you can see here with `ResponseJson`. If this were "real" JSON like an API response, then a potentially very large block of text would be duplicated within your log event! Moreover, the template string within `BeginScope` is rather arbitrary when all you want to do is add a bag of properties, and you start mixing enriching concerns with formatting concerns.
Not only does this add the unnecessary `Scope` property to your event, but it also duplicates serialized values between `Scope` and the intended properties, as you can see here with `ResponseJson`. If this were "real" JSON like an API response, then a potentially very large block of text would be duplicated within your log event!
Moreover, the template string within `BeginScope` is rather arbitrary when all you want to do is add a bag of properties, and you start mixing enriching concerns with formatting concerns.

A far better alternative is to use the `BeginScope<TState>(TState state)` method. If you provide any `IEnumerable<KeyValuePair<string, object>>` to this method, then Serilog will output the key/value pairs as structured properties _without_ the `Scope` property, as in this example:

```cs
```csharp
var scopeProps = new Dictionary<string, object> {
{ "TransactionId", 12345 },
{ "ResponseJson", jsonString },
Expand Down
1 change: 0 additions & 1 deletion samples/Sample/Sample.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="2.0.0" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="2.0.0" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Microsoft.Extensions.Logging.Console references it.

<PackageReference Include="Microsoft.Extensions.Logging.Console" Version="2.0.0" />
<PackageReference Include="Serilog.Sinks.Console" Version="3.1.1" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace Serilog.Extensions.Logging
class CachingMessageTemplateParser
{
readonly MessageTemplateParser _innerParser = new MessageTemplateParser();

readonly object _templatesLock = new object();
readonly Hashtable _templates = new Hashtable();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ void Write<TState>(LogEventLevel level, EventId eventId, TState state, Exception
{
if (logger.BindProperty(property.Key, property.Value, false, out var bound))
properties.Add(bound);
}
}
}

var stateType = state.GetType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public class SerilogLoggerFactory : ILoggerFactory
{
readonly LoggerProviderCollection _providerCollection;
readonly SerilogLoggerProvider _provider;

/// <summary>
/// Initializes a new instance of the <see cref="SerilogLoggerFactory"/> class.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public SerilogLoggerScope(SerilogLoggerProvider provider, object state, IDisposa
}

public SerilogLoggerScope Parent { get; }

public void Dispose()
{
if (!_disposed)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static class LoggerSinkConfigurationExtensions
public static LoggerConfiguration Providers(
this LoggerSinkConfiguration configuration,
LoggerProviderCollection providers,
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
LoggingLevelSwitch levelSwitch = null)
{
if (configuration == null) throw new ArgumentNullException(nameof(configuration));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</PropertyGroup>

<ItemGroup>
<None Include="..\..\assets\serilog-extension-nuget.png" Pack="true" PackagePath="" />
<None Include="..\..\assets\serilog-extension-nuget.png" Pack="true" PackagePath="" Visible="false" />
</ItemGroup>

<ItemGroup>
Expand Down
4 changes: 2 additions & 2 deletions test/Serilog.Extensions.Logging.Tests/SerilogLoggerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public void BeginScopeDestructuresObjectsWhenDestructurerIsUsedInMessageTemplate
public void BeginScopeDestructuresObjectsWhenDestructurerIsUsedInDictionary()
{
var (logger, sink) = SetUp(LogLevel.Trace);

using (logger.BeginScope(new Dictionary<string, object> {{ "@Person", new Person { FirstName = "John", LastName = "Smith" }}}))
{
logger.Log(LogLevel.Information, 0, TestMessage, null, null);
Expand Down Expand Up @@ -463,7 +463,7 @@ public void LowAndHighNumberedEventIdsAreMapped(int id)
var scalar = Assert.IsType<ScalarValue>(idValue);
Assert.Equal(id, scalar.Value);
}

[Fact]
public void MismatchedMessageTemplateParameterCountIsHandled()
{
Expand Down