Skip to content

Commit

Permalink
Fallback to Microsoft Logging Formatter when mixing positional with s…
Browse files Browse the repository at this point in the history 
…tructured (#756)
  • Loading branch information
@snakefoot
snakefoot authored Aug 8, 2024
1 parent e41019e commit 9c877a7
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 41 deletions.
6 changes: 3 additions & 3 deletions src/NLog.Extensions.Logging/Logging/NLogLogger.cs
View file
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
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
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
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

0 comments on commit 9c877a7

Please sign in to comment.