Skip to content

Commit 3636f98

Browse files
304NotModified304NotModified
authored
Refactor: split methods, docs, error handling, smaller methodes and consistency (#176)
* Refactor: split methods * refactor * refactor * proper exception handling * move docs * NotImplementedException -> NotSupportedException * refactor duplicate check * refactor to mulitple methods * CaptureMessageProperties consistent with CaptureEventId checks in method, parameter order * split method * fix nullref * merge 2 ifs * move method * reorder methods
1 parent 2f1c0c5 commit 3636f98

File tree

3 files changed

+133
-104
lines changed

3 files changed

+133
-104
lines changed

‎src/NLog.Extensions.Logging/ConfigureExtensions.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,13 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder factory, NLogProvider
6969
}
7070
#endif
7171

72+
/// <summary>
73+
/// Ignore assemblies for ${callsite}
74+
/// </summary>
7275
private static void ConfigureHiddenAssemblies()
7376
{
7477
try
7578
{
76-
//ignore these assemblies for ${callsite}
7779
InternalLogger.Trace("Hide assemblies for callsite");
7880

7981
#if NETCORE1_0

‎src/NLog.Extensions.Logging/NLogLogger.cs‎

Lines changed: 114 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using Microsoft.Extensions.Logging;
4+
using NLog.Common;
45

56
namespace NLog.Extensions.Logging
67
{
@@ -40,71 +41,89 @@ public void Log<TState>(Microsoft.Extensions.Logging.LogLevel logLevel, EventId
4041
LogEventInfo eventInfo = CreateLogEventInfo(nLogLogLevel, message, messageTemplate);
4142
eventInfo.Exception = exception;
4243

43-
CaptureEventId(eventId, eventInfo);
44+
CaptureEventId(eventInfo, eventId);
4445

45-
if (_options.CaptureMessageProperties && messageTemplate == null)
46-
{
47-
CaptureMessageProperties(state, eventInfo);
48-
}
46+
CaptureMessageProperties(eventInfo, state, messageTemplate);
4947

5048
_logger.Log(eventInfo);
5149
}
5250

51+
5352
private LogEventInfo CreateLogEventInfo(LogLevel nLogLogLevel, string message, IReadOnlyList<KeyValuePair<string, object>> parameterList)
5453
{
55-
if (parameterList != null && parameterList.Count > 1)
54+
if (parameterList != null && parameterList.Count > 1 && IsNonDigitValue(parameterList[0].Key))
5655
{
57-
// More than a single parameter (last parameter is the {OriginalFormat})
58-
var firstParameterName = parameterList[0].Key;
59-
if (!string.IsNullOrEmpty(firstParameterName))
60-
{
61-
if (firstParameterName.Length != 1 || !char.IsDigit(firstParameterName[0]))
62-
{
56+
return CreateLogEventInfoWithMultipleParameters(nLogLogLevel, message, parameterList);
57+
}
58+
return LogEventInfo.Create(nLogLogLevel, _logger.Name, message);
59+
}
60+
61+
private static bool IsNonDigitValue(string value)
62+
{
63+
return !string.IsNullOrEmpty(value) && (value.Length != 1 || !char.IsDigit(value[0]));
64+
}
65+
6366
#if !NETSTANDARD1_3
64-
var originalFormat = parameterList[parameterList.Count - 1];
65-
string originalMessage = null;
66-
if (originalFormat.Key == OriginalFormatPropertyName)
67-
{
68-
// Attempt to capture original message with placeholders
69-
originalMessage = originalFormat.Value as string;
70-
}
7167

72-
var messageTemplateParameters = new NLogMessageParameterList(parameterList, originalMessage != null);
73-
var eventInfo = new LogEventInfo(nLogLogLevel, _logger.Name, originalMessage ?? message, messageTemplateParameters);
74-
if (originalMessage != null)
75-
{
76-
eventInfo.Parameters = new object[messageTemplateParameters.Count + 1];
77-
for (int i = 0; i < messageTemplateParameters.Count; ++i)
78-
eventInfo.Parameters[i] = messageTemplateParameters[i].Value;
79-
eventInfo.Parameters[messageTemplateParameters.Count] = message;
80-
eventInfo.MessageFormatter = (l) => (string)l.Parameters[l.Parameters.Length - 1];
81-
}
82-
return eventInfo;
68+
/// <summary>
69+
/// Create Log Event with multiple parameters (last parameter is the {OriginalFormat})
70+
/// </summary>
71+
private LogEventInfo CreateLogEventInfoWithMultipleParameters(LogLevel nLogLogLevel, string message, IReadOnlyList<KeyValuePair<string, object>> parameterList)
72+
{
73+
var originalFormat = parameterList[parameterList.Count - 1];
74+
string originalMessage = null;
75+
if (originalFormat.Key == OriginalFormatPropertyName)
76+
{
77+
// Attempt to capture original message with placeholders
78+
originalMessage = originalFormat.Value as string;
79+
}
80+
81+
var messageTemplateParameters = new NLogMessageParameterList(parameterList, originalMessage != null);
82+
var eventInfo = new LogEventInfo(nLogLogLevel, _logger.Name, originalMessage ?? message, messageTemplateParameters);
83+
if (originalMessage != null)
84+
{
85+
SetEventInfoParameters(eventInfo, messageTemplateParameters);
86+
eventInfo.Parameters[messageTemplateParameters.Count] = message;
87+
eventInfo.MessageFormatter = (l) => (string)l.Parameters[l.Parameters.Length - 1];
88+
}
89+
return eventInfo;
90+
}
91+
92+
private static void SetEventInfoParameters(LogEventInfo eventInfo, NLogMessageParameterList messageTemplateParameters)
93+
{
94+
eventInfo.Parameters = new object[messageTemplateParameters.Count + 1];
95+
for (int i = 0; i < messageTemplateParameters.Count; ++i)
96+
eventInfo.Parameters[i] = messageTemplateParameters[i].Value;
97+
}
98+
8399
#else
84-
var eventInfo = LogEventInfo.Create(nLogLogLevel, _logger.Name, message);
85-
for (int i = 0; i < parameterList.Count; ++i)
86-
{
87-
var parameter = parameterList[i];
88-
if (string.IsNullOrEmpty(parameter.Key))
89-
break; // Skip capture of invalid parameters
90-
91-
var parameterName = parameter.Key;
92-
switch (parameterName[0])
93-
{
94-
case '@': parameterName = parameterName.Substring(1); break;
95-
case '$': parameterName = parameterName.Substring(1); break;
96-
}
97-
eventInfo.Properties[parameterName] = parameter.Value;
98-
}
99-
return eventInfo;
100-
#endif
101-
}
100+
101+
/// <summary>
102+
/// Create Log Event with multiple parameters (last parameter is the {OriginalFormat})
103+
/// </summary>
104+
private LogEventInfo CreateLogEventInfoWithMultipleParameters(LogLevel nLogLogLevel, string message, IReadOnlyList<KeyValuePair<string, object>> parameterList)
105+
{
106+
var eventInfo = LogEventInfo.Create(nLogLogLevel, _logger.Name, message);
107+
for (int i = 0; i < parameterList.Count; ++i)
108+
{
109+
var parameter = parameterList[i];
110+
if (string.IsNullOrEmpty(parameter.Key))
111+
break; // Skip capture of invalid parameters
112+
113+
var parameterName = parameter.Key;
114+
if (parameterName[0] == '@' || parameterName[0] == '$')
115+
{
116+
parameterName = parameterName.Substring(1);
102117
}
118+
eventInfo.Properties[parameterName] = parameter.Value;
103119
}
104-
return LogEventInfo.Create(nLogLogLevel, _logger.Name, message);
120+
return eventInfo;
105121
}
106122

107-
private void CaptureEventId(EventId eventId, LogEventInfo eventInfo)
123+
#endif
124+
125+
126+
private void CaptureEventId(LogEventInfo eventInfo, EventId eventId)
108127
{
109128
if (!_options.IgnoreEmptyEventId || eventId.Id != 0 || !string.IsNullOrEmpty(eventId.Name))
110129
{
@@ -114,11 +133,7 @@ private void CaptureEventId(EventId eventId, LogEventInfo eventInfo)
114133
if (!ReferenceEquals(eventIdPropertyNames.Item1, eventIdSeparator))
115134
{
116135
// Perform atomic cache update of the string-allocations matching the current separator
117-
eventIdPropertyNames = new Tuple<string, string, string>(
118-
eventIdSeparator,
119-
string.Concat("EventId", eventIdSeparator, "Id"),
120-
string.Concat("EventId", eventIdSeparator, "Name"));
121-
_eventIdPropertyNames = eventIdPropertyNames;
136+
_eventIdPropertyNames = eventIdPropertyNames = CreateEventIdPropertyNames(eventIdSeparator);
122137
}
123138

124139
var idIsZero = eventId.Id == 0;
@@ -128,9 +143,18 @@ private void CaptureEventId(EventId eventId, LogEventInfo eventInfo)
128143
}
129144
}
130145

131-
private static void CaptureMessageProperties<TState>(TState state, LogEventInfo eventInfo)
146+
private static Tuple<string, string, string> CreateEventIdPropertyNames(string eventIdSeparator)
147+
{
148+
var eventIdPropertyNames = new Tuple<string, string, string>(
149+
eventIdSeparator,
150+
string.Concat("EventId", eventIdSeparator, "Id"),
151+
string.Concat("EventId", eventIdSeparator, "Name"));
152+
return eventIdPropertyNames;
153+
}
154+
155+
private void CaptureMessageProperties<TState>(LogEventInfo eventInfo, TState state, IReadOnlyList<KeyValuePair<string, object>> messageTemplate)
132156
{
133-
if (state is IEnumerable<KeyValuePair<string, object>> messageProperties)
157+
if (_options.CaptureMessageProperties && messageTemplate == null && state is IEnumerable<KeyValuePair<string, object>> messageProperties)
134158
{
135159
foreach (var property in messageProperties)
136160
{
@@ -192,24 +216,25 @@ private static LogLevel ConvertLogLevel(Microsoft.Extensions.Logging.LogLevel lo
192216
class ScopeProperties : IDisposable
193217
{
194218
List<IDisposable> _properties;
195-
List<IDisposable> Properties { get { return _properties ?? (_properties = new List<IDisposable>()); } }
219+
List<IDisposable> Properties => _properties ?? (_properties = new List<IDisposable>());
196220

197-
class ScopeProperty : IDisposable
221+
222+
public static IDisposable CreateFromState<TState>(TState state, IEnumerable<KeyValuePair<string, object>> messageProperties)
198223
{
199-
string _key;
224+
ScopeProperties scope = new ScopeProperties();
200225

201-
public ScopeProperty(string key, object value)
226+
foreach (var property in messageProperties)
202227
{
203-
_key = key;
204-
MappedDiagnosticsLogicalContext.Set(key, value);
205-
}
228+
if (String.IsNullOrEmpty(property.Key))
229+
continue;
206230

207-
public void Dispose()
208-
{
209-
MappedDiagnosticsLogicalContext.Remove(_key);
231+
scope.AddProperty(property.Key, property.Value);
210232
}
211-
}
212233

234+
scope.AddDispose(NestedDiagnosticsLogicalContext.Push(state));
235+
return scope;
236+
}
237+
213238
public void AddDispose(IDisposable disposable)
214239
{
215240
Properties.Add(disposable);
@@ -232,12 +257,30 @@ public void Dispose()
232257
{
233258
property.Dispose();
234259
}
235-
catch
260+
catch (Exception ex)
236261
{
262+
InternalLogger.Trace(ex, "Exception in Dispose property {0}", property);
237263
}
238264
}
239265
}
240266
}
267+
268+
class ScopeProperty : IDisposable
269+
{
270+
string _key;
271+
272+
public ScopeProperty(string key, object value)
273+
{
274+
_key = key;
275+
MappedDiagnosticsLogicalContext.Set(key, value);
276+
}
277+
278+
public void Dispose()
279+
{
280+
MappedDiagnosticsLogicalContext.Remove(_key);
281+
}
282+
}
283+
241284
}
242285

243286
/// <summary>
@@ -252,23 +295,9 @@ public IDisposable BeginScope<TState>(TState state)
252295
throw new ArgumentNullException(nameof(state));
253296
}
254297

255-
if (_options.CaptureMessageProperties)
298+
if (_options.CaptureMessageProperties && state is IEnumerable<KeyValuePair<string, object>> messageProperties)
256299
{
257-
if (state is IEnumerable<KeyValuePair<string, object>> messageProperties)
258-
{
259-
ScopeProperties scope = new ScopeProperties();
260-
261-
foreach (var property in messageProperties)
262-
{
263-
if (string.IsNullOrEmpty(property.Key))
264-
continue;
265-
266-
scope.AddProperty(property.Key, property.Value);
267-
}
268-
269-
scope.AddDispose(NestedDiagnosticsLogicalContext.Push(state));
270-
return scope;
271-
}
300+
return ScopeProperties.CreateFromState(state, messageProperties);
272301
}
273302

274303
return NestedDiagnosticsLogicalContext.Push(state);

‎src/NLog.Extensions.Logging/NLogMessageParameterList.cs‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,16 @@ internal class NLogMessageParameterList : IList<NLog.MessageTemplates.MessageTem
1717

1818
public NLogMessageParameterList(IReadOnlyList<KeyValuePair<string, object>> parameterList, bool includesOriginalMessage)
1919
{
20-
List<KeyValuePair<string, object>> validParameterList = includesOriginalMessage ? null : new List<KeyValuePair<string, object>>();
20+
var validParameterList = includesOriginalMessage ? null : new List<KeyValuePair<string, object>>();
2121
for (int i = 0; i < parameterList.Count; ++i)
2222
{
23-
if (!string.IsNullOrEmpty(parameterList[i].Key) && (parameterList[i].Key != NLogLogger.OriginalFormatPropertyName || i == parameterList.Count - 1))
23+
var paramPair = parameterList[i];
24+
bool isNonOriginalFormatName;
25+
if (!string.IsNullOrEmpty(paramPair.Key) && ((isNonOriginalFormatName = paramPair.Key != NLogLogger.OriginalFormatPropertyName) || i == parameterList.Count - 1))
2426
{
25-
if (validParameterList != null)
27+
if (validParameterList != null && isNonOriginalFormatName)
2628
{
27-
if (parameterList[i].Key != NLogLogger.OriginalFormatPropertyName)
28-
validParameterList.Add(parameterList[i]);
29+
validParameterList.Add(paramPair);
2930
}
3031
}
3132
else
@@ -38,10 +39,7 @@ public NLogMessageParameterList(IReadOnlyList<KeyValuePair<string, object>> para
3839
}
3940
}
4041
}
41-
if (validParameterList != null)
42-
{
43-
validParameterList.Add(new KeyValuePair<string, object>());
44-
}
42+
validParameterList?.Add(new KeyValuePair<string, object>());
4543
_parameterList = validParameterList ?? parameterList;
4644
}
4745

@@ -65,7 +63,7 @@ public NLog.MessageTemplates.MessageTemplateParameter this[int index]
6563
}
6664
return new NLog.MessageTemplates.MessageTemplateParameter(parameter.Key, parameter.Value, null, captureType);
6765
}
68-
set => throw new NotImplementedException();
66+
set => throw new NotSupportedException();
6967
}
7068

7169
public int Count => _parameterList.Count - 1;
@@ -74,22 +72,22 @@ public NLog.MessageTemplates.MessageTemplateParameter this[int index]
7472

7573
public void Add(NLog.MessageTemplates.MessageTemplateParameter item)
7674
{
77-
throw new NotImplementedException();
75+
throw new NotSupportedException();
7876
}
7977

8078
public void Clear()
8179
{
82-
throw new NotImplementedException();
80+
throw new NotSupportedException();
8381
}
8482

8583
public bool Contains(NLog.MessageTemplates.MessageTemplateParameter item)
8684
{
87-
throw new NotImplementedException();
85+
throw new NotSupportedException();
8886
}
8987

9088
public void CopyTo(NLog.MessageTemplates.MessageTemplateParameter[] array, int arrayIndex)
9189
{
92-
throw new NotImplementedException();
90+
throw new NotSupportedException();
9391
}
9492

9593
public IEnumerator<NLog.MessageTemplates.MessageTemplateParameter> GetEnumerator()
@@ -99,22 +97,22 @@ public void CopyTo(NLog.MessageTemplates.MessageTemplateParameter[] array, int a
9997

10098
public int IndexOf(NLog.MessageTemplates.MessageTemplateParameter item)
10199
{
102-
throw new NotImplementedException();
100+
throw new NotSupportedException();
103101
}
104102

105103
public void Insert(int index, NLog.MessageTemplates.MessageTemplateParameter item)
106104
{
107-
throw new NotImplementedException();
105+
throw new NotSupportedException();
108106
}
109107

110108
public bool Remove(NLog.MessageTemplates.MessageTemplateParameter item)
111109
{
112-
throw new NotImplementedException();
110+
throw new NotSupportedException();
113111
}
114112

115113
public void RemoveAt(int index)
116114
{
117-
throw new NotImplementedException();
115+
throw new NotSupportedException();
118116
}
119117

120118
IEnumerator IEnumerable.GetEnumerator()

0 commit comments

Comments
 (0)