Skip to content

Commit

Permalink
Merge pull request #210 from sungam3r/cleanup
Browse files Browse the repository at this point in the history
Little cleanup
  • Loading branch information
nblumhardt authored Mar 3, 2023
2 parents 6539e2d + 1269b85 commit da7b61b
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 14 deletions.
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" />
<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 @@ -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

0 comments on commit da7b61b

Please sign in to comment.