Skip to content

Adding support for scopes #85

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.

Sign up for GitHub

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

Open
spasarto wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Adding support for scopes #85

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d1b999f
Adding support for scopes
Jun 10, 2019
50323db
Defaulting options to false to provide seemless upgrade experience to…
Jun 10, 2019
0588fde
using a consistent delimiter to better support sumo's parse
Jun 10, 2019
0591684
Adding a scope separator option
Jun 11, 2019
1a0a63e
PR Feedback
Dec 10, 2019
d276c0e
Merge branch 'master' into master
spasarto Dec 10, 2019
23dd3ed
Adding support for scopes
Jun 10, 2019
5f5a943
Defaulting options to false to provide seemless upgrade experience to…
Jun 10, 2019
fa2164d
using a consistent delimiter to better support sumo's parse
Jun 10, 2019
a366e8c
Adding a scope separator option
Jun 11, 2019
b4eef14
PR Feedback
Dec 10, 2019
c03c7ca
merge
Dec 10, 2019
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
2 changes: 1 addition & 1 deletion SumoLogic.Logging.AspNetCore/Logger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
return;
}
var line = $"{formatter(state, exception)}";
provider.WriteLine(line, categoryName);
provider.WriteLine(line, categoryName, logLevel);
}
}
}
2 changes: 2 additions & 0 deletions SumoLogic.Logging.AspNetCore/LoggerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,7 @@ public string Uri
/// </summary>
public ILogger DebuggingLogger { get; set; }

public bool IncludeScopes { get; set; }
public bool IncludeCategory { get; set; }
}
}
49 changes: 42 additions & 7 deletions SumoLogic.Logging.AspNetCore/LoggerProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
using SumoLogic.Logging.Common.Queue;
using SumoLogic.Logging.Common.Sender;
using System;
using System.Text;
using System.Threading;

namespace SumoLogic.Logging.AspNetCore
Expand All @@ -37,7 +38,7 @@ namespace SumoLogic.Logging.AspNetCore
/// Sumo Logic Logger Provider implementation
/// </summary>
[ProviderAlias("SumoLogic")]
public class LoggerProvider : ILoggerProvider
public class LoggerProvider : ILoggerProvider, ISupportExternalScope
{
public LoggerOptions LoggerOptions { get; private set; }

Expand All @@ -50,6 +51,11 @@ public class LoggerProvider : ILoggerProvider
private SumoLogicMessageSenderBufferFlushingTask flushBufferTask = null;

private volatile BufferWithEviction<string> messagesQueue = null;
private bool includeScopes;
private bool includeCategory;
private IExternalScopeProvider scopeProvider;

internal IExternalScopeProvider ScopeProvider => includeScopes ? scopeProvider : null;

public LoggerProvider(IOptionsMonitor<LoggerOptions> options)
{
Expand Down Expand Up @@ -80,7 +86,7 @@ public void Dispose()
/// </summary>
/// <param name="message">the message line to be sent</param>
/// <param name="categoryName">not used for now</param>
public void WriteLine(String message, String categoryName)
public void WriteLine(string message, string categoryName, LogLevel logLevel)
{
if (null == message)
{
Expand All @@ -93,17 +99,40 @@ public void WriteLine(String message, String categoryName)
return;
}

String line = string.Concat(
message.TrimEnd(Environment.NewLine.ToCharArray()),
Environment.NewLine);
var builder = new StringBuilder();
if (includeCategory)
{
builder.Append(" [");
builder.Append(logLevel.ToString());
builder.Append("] ");
builder.Append(categoryName);
}

var scopeProvider = ScopeProvider;
if (scopeProvider != null)
{
scopeProvider.ForEachScope((scope, stringBuilder) =>
{
stringBuilder.Append(" => ").Append(scope);
}, builder);

builder.AppendLine(":");
}
else if(includeCategory)
{
builder.Append(": ");
}


builder.AppendLine(message.Trim());
Copy link
Contributor

@guptakeshav-sumo guptakeshav-sumo Dec 10, 2019

Choose a reason for hiding this comment

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

Why are we changing the default logic from

String line = string.Concat(
                message.TrimEnd(Environment.NewLine.ToCharArray()),
                Environment.NewLine);

to just
message.Trim()?

Copy link
Author

@spasarto spasarto Dec 10, 2019

Choose a reason for hiding this comment

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

I'm trying to remember my exact reason - it's been a while :)

The changes were based off of the ConsoleLogger implementation of scopes:
https://github.com/aspnet/Extensions/blob/master/src/Logging/Logging.Console/src/ConsoleLogger.cs

Specifically, they replace all new lines in message, not just the beginning and end. I know I encountered a need for this during testing, but I can't remember the case anymore.

What are your concerns in regards to trimming the start?

Copy link
Contributor

@guptakeshav-sumo guptakeshav-sumo Dec 11, 2019

Choose a reason for hiding this comment

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

I am unsure on why the initial code was written like that. There might have been a reason too! :) So I am just worried about touching something which we are not completely aware of. And if we can't justify this change right now, I'll prefer to revert this change. We can obviously add this change later on in some other PR, if needed.


if (LoggerOptions.IsBuffered)
{
messagesQueue.Add(line);
messagesQueue.Add(builder.ToString());
}
else
{
WriteLineToSumo(line);
WriteLineToSumo(builder.ToString());
}
}

Expand All @@ -120,6 +149,8 @@ private void ReConfig(LoggerOptions options)
{
InitBuffer(options);
}
includeScopes = options.IncludeScopes;
includeCategory = options.IncludeCategory;
LoggerOptions = options;
}

Expand Down Expand Up @@ -174,5 +205,9 @@ private void WriteLineToSumo(String body)
.GetResult();
}

void ISupportExternalScope.SetScopeProvider(IExternalScopeProvider scopeProvider)
{
this.scopeProvider = scopeProvider;
}
}
}
1 change: 1 addition & 0 deletions SumoLogic.Logging.AspNetCore/SumoLogic.Logging.AspNetCore.netstandard.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<PackageReleaseNotes></PackageReleaseNotes>
<RepositoryType>git</RepositoryType>
<SignAssembly>true</SignAssembly>
<Version>1.0.1</Version>
</PropertyGroup>
<ItemGroup>
<None Include="..\SumoLogic.Logging.snk">
Expand Down