Skip to content

Fallback to Microsoft Logging Formatter when mixing positional with structured #756

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

Merged
snakefoot merged 1 commit into from
Aug 8, 2024
Merged

Fallback to Microsoft Logging Formatter when mixing positional with structured #756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/NLog.Extensions.Logging/Logging/NLogLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private LogEventInfo CreateLogEventInfo<TState>(LogLevel nLogLogLevel, EventId e
?? CaputureBasicLogEvent(nLogLogLevel, formatter(state, exception), messagePropertyList, messageParameters);
CaptureEventIdProperties(logEvent, eventId);
return logEvent;
}
}
else
{
var logEvent = TryParseMessageTemplate(nLogLogLevel, messagePropertyList, messageParameters)
Expand Down Expand Up @@ -122,7 +122,7 @@ private LogEventInfo TryParseMessageTemplate(LogLevel nLogLogLevel, IReadOnlyLis

private LogEventInfo TryParsePostionalMessageTemplate(LogLevel nLogLogLevel, IReadOnlyList<KeyValuePair<string, object>> messageProperties, NLogMessageParameterList messageParameters)
{
if (messageParameters.IsPositional && _options.ParseMessageTemplates)
if (messageParameters.IsPositional && (messageParameters.HasComplexParameters || _options.ParseMessageTemplates))
{
string originalMessage = TryParsePositionalParameters(messageProperties, out var parameters);
if (originalMessage != null)
Expand Down Expand Up @@ -258,7 +258,7 @@ private static object[] CreateStructuredLogEventInfoParameters(NLogMessageParame
for (int i = 0; i < messageParameterCount; ++i)
{
var propertyName = messageParameters[i].Name;

bool extraProperty = true;
for (int j = startPos; j < messageTemplateParameters.Count; ++j)
{
Expand Down
62 changes: 29 additions & 33 deletions src/NLog.Extensions.Logging/Logging/NLogMessageParameterList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,26 @@ namespace NLog.Extensions.Logging
/// </summary>
internal class NLogMessageParameterList : IList<MessageTemplateParameter>
{
private static readonly NLogMessageParameterList EmptyList = new NLogMessageParameterList(Array.Empty<KeyValuePair<string, object>>(), default, default, default, default);
private static readonly NLogMessageParameterList EmptyPositionalList = new NLogMessageParameterList(Array.Empty<KeyValuePair<string, object>>(), default, default, default, isPositional: true);
private static readonly NLogMessageParameterList OriginalMessageList = new NLogMessageParameterList(new[] { new KeyValuePair<string, object>(NLogLogger.OriginalFormatPropertyName, string.Empty) }, 0, default, default, default);
private static readonly NLogMessageParameterList EmptyList = new NLogMessageParameterList(Array.Empty<KeyValuePair<string, object>>(), default, default, default);
private static readonly NLogMessageParameterList PositionalParameterList = new NLogMessageParameterList(Array.Empty<KeyValuePair<string, object>>(), default, hasComplexParameters: false, isPositional: true);
private static readonly NLogMessageParameterList PositionalComplexParameterList = new NLogMessageParameterList(Array.Empty<KeyValuePair<string, object>>(), default, hasComplexParameters: true, isPositional: true);
private static readonly NLogMessageParameterList OriginalMessageList = new NLogMessageParameterList(new[] { new KeyValuePair<string, object>(NLogLogger.OriginalFormatPropertyName, string.Empty) }, 0, default, default);

private readonly IReadOnlyList<KeyValuePair<string, object>> _parameterList;
private readonly int? _originalMessageIndex;
private readonly bool _hasMessageTemplateCapture;
private readonly bool _isMixedPositional;
private readonly bool _hasComplexParameters;
private readonly bool _isPositional;

public bool HasComplexParameters => _hasMessageTemplateCapture || _isMixedPositional;
public bool HasComplexParameters => _hasComplexParameters;
public bool IsPositional => _isPositional;
public int Count => _parameterList.Count - (_originalMessageIndex.HasValue ? 1 : 0);
public bool IsReadOnly => true;

private NLogMessageParameterList(IReadOnlyList<KeyValuePair<string, object>> parameterList, int? originalMessageIndex, bool hasMessageTemplateCapture, bool isMixedPositional, bool isPositional)
private NLogMessageParameterList(IReadOnlyList<KeyValuePair<string, object>> parameterList, int? originalMessageIndex, bool hasComplexParameters, bool isPositional)
{
_parameterList = parameterList;
_originalMessageIndex = originalMessageIndex;
_hasMessageTemplateCapture = hasMessageTemplateCapture;
_isMixedPositional = isMixedPositional;
_hasComplexParameters = hasComplexParameters;
_isPositional = isPositional;
}

Expand All @@ -45,21 +44,21 @@ public static NLogMessageParameterList TryParse(IReadOnlyList<KeyValuePair<strin
var parameterCount = parameterList.Count;
if (parameterCount > 1 || (parameterCount == 1 && !NLogLogger.OriginalFormatPropertyName.Equals(parameterList[0].Key)))
{
if (IsValidParameterList(parameterList, out var originalMessageIndex, out var hasMessageTemplateCapture, out var isMixedPositional, out var isPositional))
if (IsValidParameterList(parameterList, out var originalMessageIndex, out var hasComplexParameters, out var isPositional))
{
if (isPositional)
{
return EmptyPositionalList; // Skip allocation, not needed to create Parameters-array
return hasComplexParameters ? PositionalComplexParameterList : PositionalParameterList; // Skip allocation, not needed to create Parameters-array
}
else
{
return new NLogMessageParameterList(parameterList, originalMessageIndex, hasMessageTemplateCapture, isMixedPositional, isPositional);
return new NLogMessageParameterList(parameterList, originalMessageIndex, hasComplexParameters, isPositional);
}
}
else
{
return new NLogMessageParameterList(CreateValidParameterList(parameterList), originalMessageIndex, hasMessageTemplateCapture, isMixedPositional || isPositional, isPositional);
}
return new NLogMessageParameterList(CreateValidParameterList(parameterList), originalMessageIndex, hasComplexParameters, isPositional);
}
}
else if (parameterCount == 1)
{
Expand Down Expand Up @@ -88,12 +87,12 @@ public string GetOriginalMessage(IReadOnlyList<KeyValuePair<string, object>> mes
/// <summary>
/// Verify that the input parameterList contains non-empty key-values and the original-format-property at the end
/// </summary>
private static bool IsValidParameterList(IReadOnlyList<KeyValuePair<string, object>> parameterList, out int? originalMessageIndex, out bool hasMessageTemplateCapture, out bool isMixedPositional, out bool isPositional)
private static bool IsValidParameterList(IReadOnlyList<KeyValuePair<string, object>> parameterList, out int? originalMessageIndex, out bool hasComplexParameters, out bool isPositional)
{
hasMessageTemplateCapture = false;
isMixedPositional = false;
hasComplexParameters = false;
originalMessageIndex = null;
isPositional = false;
bool isMixedPositional = false;
string parameterName;

var parameterCount = parameterList.Count;
Expand All @@ -110,26 +109,23 @@ private static bool IsValidParameterList(IReadOnlyList<KeyValuePair<string, obje
{
if (!isPositional)
isMixedPositional = i != 0;
isMixedPositional |= i != (firstChar - '0');
hasComplexParameters |= i != (firstChar - '0');
isPositional = true;
}
else
else if (NLogLogger.OriginalFormatPropertyName.Equals(parameterName))
{
if (NLogLogger.OriginalFormatPropertyName.Equals(parameterName))
{
if (originalMessageIndex.HasValue)
{
originalMessageIndex = null;
return false;
}

originalMessageIndex = i;
}
else
if (originalMessageIndex.HasValue)
{
isMixedPositional = isPositional;
hasMessageTemplateCapture |= GetCaptureType(firstChar) != CaptureType.Normal;
originalMessageIndex = null;
return false;
}

originalMessageIndex = i;
}
else
{
isMixedPositional = isPositional;
hasComplexParameters |= GetCaptureType(firstChar) != CaptureType.Normal;
}
}

Expand Down Expand Up @@ -187,7 +183,7 @@ public MessageTemplateParameter this[int index]
index += 1;

var parameter = _parameterList[index];
return _hasMessageTemplateCapture ?
return _hasComplexParameters ?
GetMessageTemplateParameter(parameter.Key, parameter.Value) :
new MessageTemplateParameter(parameter.Key, parameter.Value, null, CaptureType.Normal);
}
Expand Down
18 changes: 17 additions & 1 deletion test/NLog.Extensions.Logging.Tests/LoggerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,27 @@ public void TestInvalidFormatString()
[Fact]
public void TestInvalidFormatString2()
{
var runner = GetRunner<Runner>(new NLogProviderOptions { CaptureMessageTemplates = false });
var runner = GetRunner<Runner>(new NLogProviderOptions { CaptureMessageTemplates = true });
var ex = Assert.Throws<AggregateException>(() => runner.Logger.LogDebug("{0}{1}", "Test"));
Assert.IsType<FormatException>(ex.InnerException);
}

[Fact]
public void TestInvalidMixFormatString()
{
var runner = GetRunner<Runner>();
runner.Logger.LogDebug("{0}{Mix}", "Mix", "Test");
Assert.Equal("NLog.Extensions.Logging.Tests.LoggerTests.Runner|DEBUG|MixTest|0=Mix, Mix=Test", runner.LastTargetMessage);
}

[Fact]
public void TestInvalidMixFormatString2()
{
var runner = GetRunner<Runner>(new NLogProviderOptions { CaptureMessageTemplates = true });
runner.Logger.LogDebug("{0}{Mix}", "Mix", "Test");
Assert.Equal("NLog.Extensions.Logging.Tests.LoggerTests.Runner|DEBUG|MixTest|0=Mix, Mix=Test", runner.LastTargetMessage);
}

[Fact]
public void TestExceptionWithMessage()
{
Expand Down
6 changes: 2 additions & 4 deletions test/NLog.Extensions.Logging.Tests/NLogMessageParameterListTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,9 @@ public void CreateNLogMessageParameterMixedPositional()
};
var list = NLogMessageParameterList.TryParse(items);

Assert.Equal(3, list.Count);
Assert.Equal(new MessageTemplateParameter("2", 1, null, CaptureType.Normal), list[0]);
Assert.Equal(new MessageTemplateParameter("1", 2, null, CaptureType.Normal), list[1]);
Assert.Equal(new MessageTemplateParameter("0", 3, null, CaptureType.Normal), list[2]);
Assert.Empty(list);
Assert.True(list.HasComplexParameters);
Assert.True(list.IsPositional);
}

[Fact]
Expand Down