Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Add more overloads to LoggerMessage.Define #296

Closed
wants to merge 1 commit into from
Closed

Add more overloads to LoggerMessage.Define #296

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
212 changes: 212 additions & 0 deletions src/Microsoft.Extensions.Logging.Abstractions/LoggerMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,26 @@ public static Func<ILogger, T1, T2, T3, IDisposable> DefineScope<T1, T2, T3>(str
return (logger, arg1, arg2, arg3) => logger.BeginScopeImpl(new LogValues<T1, T2, T3>(formatter, arg1, arg2, arg3));
}

/// <summary>
/// Creates a delegate which can be invoked for logging a message.
/// </summary>
/// <param name="logLevel">The <see cref="LogLevel"/></param>
/// <param name="eventId">The event id</param>
/// <param name="formatString">The named format string</param>
/// <returns>A delegate which when invoked creates a log message.</returns>
public static Action<ILogger, Exception> Define(LogLevel logLevel, int eventId, string formatString)
Copy link
Member

Choose a reason for hiding this comment

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

This extension should not be required as the intention of the extensions in this file is to optimize for the scenarios where we pass parameters to the format string (where as here we do not pass any)...

Example:

public static void OrderPlaced(this ILogger logger)
{
    if (logger.IsEnabled(LogLevel.Information)
    { 
        logger.LogInformation(eventId: 10, "Order placed");
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but we get LoggingExtensions with two usage patterns inside which doesn't look good.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kichalla I said it was ok 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

@kichalla in this particular case it's more about consistency and not any actual perf concern.

Copy link
Member

Choose a reason for hiding this comment

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

Sound good...I agree.

{
var formatter = new LogValuesFormatter(formatString);

return (logger, exception) =>
{
if (logger.IsEnabled(logLevel))
{
logger.Log(logLevel, eventId, new LogValues(formatter), exception, LogValues.Callback);
}
};
}

/// <summary>
/// Creates a delegate which can be invoked for logging a message.
/// </summary>
Expand Down Expand Up @@ -132,6 +152,81 @@ public static Action<ILogger, T1, T2, T3, Exception> Define<T1, T2, T3>(LogLevel
};
}

/// <summary>
/// Creates a delegate which can be invoked for logging a message.
/// </summary>
/// <typeparam name="T1">The type of the first parameter passed to the named format string.</typeparam>
/// <typeparam name="T2">The type of the second parameter passed to the named format string.</typeparam>
/// <typeparam name="T3">The type of the third parameter passed to the named format string.</typeparam>
/// <typeparam name="T4">The type of the fourth parameter passed to the named format string.</typeparam>
/// <param name="logLevel">The <see cref="LogLevel"/></param>
/// <param name="eventId">The event id</param>
/// <param name="formatString">The named format string</param>
/// <returns>A delegate which when invoked creates a log message.</returns>
public static Action<ILogger, T1, T2, T3, T4, Exception> Define<T1, T2, T3, T4>(LogLevel logLevel, int eventId, string formatString)
{
var formatter = new LogValuesFormatter(formatString);

return (logger, arg1, arg2, arg3, arg4, exception) =>
{
if (logger.IsEnabled(logLevel))
{
logger.Log(logLevel, eventId, new LogValues<T1, T2, T3, T4>(formatter, arg1, arg2, arg3, arg4), exception, LogValues<T1, T2, T3, T4>.Callback);
}
};
}

/// <summary>
/// Creates a delegate which can be invoked for logging a message.
/// </summary>
/// <typeparam name="T1">The type of the first parameter passed to the named format string.</typeparam>
/// <typeparam name="T2">The type of the second parameter passed to the named format string.</typeparam>
/// <typeparam name="T3">The type of the third parameter passed to the named format string.</typeparam>
/// <typeparam name="T4">The type of the fourth parameter passed to the named format string.</typeparam>
/// <typeparam name="T5">The type of the fifth parameter passed to the named format string.</typeparam>
/// <param name="logLevel">The <see cref="LogLevel"/></param>
/// <param name="eventId">The event id</param>
/// <param name="formatString">The named format string</param>
/// <returns>A delegate which when invoked creates a log message.</returns>
public static Action<ILogger, T1, T2, T3, T4, T5, Exception> Define<T1, T2, T3, T4, T5>(LogLevel logLevel, int eventId, string formatString)
{
var formatter = new LogValuesFormatter(formatString);

return (logger, arg1, arg2, arg3, arg4, arg5, exception) =>
{
if (logger.IsEnabled(logLevel))
{
logger.Log(logLevel, eventId, new LogValues<T1, T2, T3, T4, T5>(formatter, arg1, arg2, arg3, arg4, arg5), exception, LogValues<T1, T2, T3, T4, T5>.Callback);
}
};
}

/// <summary>
/// Creates a delegate which can be invoked for logging a message.
/// </summary>
/// <typeparam name="T1">The type of the first parameter passed to the named format string.</typeparam>
/// <typeparam name="T2">The type of the second parameter passed to the named format string.</typeparam>
/// <typeparam name="T3">The type of the third parameter passed to the named format string.</typeparam>
/// <typeparam name="T4">The type of the fourth parameter passed to the named format string.</typeparam>
/// <typeparam name="T5">The type of the fifth parameter passed to the named format string.</typeparam>
/// <typeparam name="T5">The type of the sixth parameter passed to the named format string.</typeparam>
/// <param name="logLevel">The <see cref="LogLevel"/></param>
/// <param name="eventId">The event id</param>
/// <param name="formatString">The named format string</param>
/// <returns>A delegate which when invoked creates a log message.</returns>
public static Action<ILogger, T1, T2, T3, T4, T5, T6, Exception> Define<T1, T2, T3, T4, T5, T6>(LogLevel logLevel, int eventId, string formatString)
{
var formatter = new LogValuesFormatter(formatString);

return (logger, arg1, arg2, arg3, arg4, arg5, arg6, exception) =>
{
if (logger.IsEnabled(logLevel))
{
logger.Log(logLevel, eventId, new LogValues<T1, T2, T3, T4, T5, T6>(formatter, arg1, arg2, arg3, arg4, arg5, arg6), exception, LogValues<T1, T2, T3, T4, T5, T6>.Callback);
}
};
}

private class LogValues : ILogValues
{
public static Func<object, Exception, string> Callback = (state, exception) => ((LogValues)state)._formatter.Format(((LogValues)state).ToArray());
Expand Down Expand Up @@ -268,6 +363,123 @@ public IEnumerable<KeyValuePair<string, object>> GetValues() => new[]

public override string ToString() => _formatter.Format(ToArray());
}

private class LogValues<T0, T1, T2, T3, T4> : ILogValues
{
public static Func<object, Exception, string> Callback = (state, exception) => ((LogValues<T0, T1, T2, T3, T4>)state)._formatter.Format(((LogValues<T0, T1, T2, T3, T4>)state).ToArray());

private readonly LogValuesFormatter _formatter;
public T0 _value0;
public T1 _value1;
public T2 _value2;
public T3 _value3;
public T4 _value4;

public LogValues(LogValuesFormatter formatter, T0 value0, T1 value1, T2 value2, T3 value3, T4 value4)
{
_formatter = formatter;
_value0 = value0;
_value1 = value1;
_value2 = value2;
_value3 = value3;
_value4 = value4;
}

public IEnumerable<KeyValuePair<string, object>> GetValues() => new[]
{
new KeyValuePair<string, object>(_formatter.ValueNames[0], _value0),
new KeyValuePair<string, object>(_formatter.ValueNames[1], _value1),
new KeyValuePair<string, object>(_formatter.ValueNames[2], _value2),
new KeyValuePair<string, object>(_formatter.ValueNames[3], _value3),
new KeyValuePair<string, object>(_formatter.ValueNames[4], _value4),
new KeyValuePair<string, object>("{OriginalFormat}", _formatter.OriginalFormat),
};

public object[] ToArray() => new object[] { _value0, _value1, _value2, _value3, _value4 };

public override string ToString() => _formatter.Format(ToArray());
}

private class LogValues<T0, T1, T2, T3, T4, T5> : ILogValues
{
public static Func<object, Exception, string> Callback = (state, exception) => ((LogValues<T0, T1, T2, T3, T4, T5>)state)._formatter.Format(((LogValues<T0, T1, T2, T3, T4, T5>)state).ToArray());

private readonly LogValuesFormatter _formatter;
public T0 _value0;
public T1 _value1;
public T2 _value2;
public T3 _value3;
public T4 _value4;
public T5 _value5;

public LogValues(LogValuesFormatter formatter, T0 value0, T1 value1, T2 value2, T3 value3, T4 value4, T5 value5)
{
_formatter = formatter;
_value0 = value0;
_value1 = value1;
_value2 = value2;
_value3 = value3;
_value4 = value4;
_value5 = value5;
}

public IEnumerable<KeyValuePair<string, object>> GetValues() => new[]
{
new KeyValuePair<string, object>(_formatter.ValueNames[0], _value0),
new KeyValuePair<string, object>(_formatter.ValueNames[1], _value1),
new KeyValuePair<string, object>(_formatter.ValueNames[2], _value2),
new KeyValuePair<string, object>(_formatter.ValueNames[3], _value3),
new KeyValuePair<string, object>(_formatter.ValueNames[4], _value4),
new KeyValuePair<string, object>(_formatter.ValueNames[5], _value5),
new KeyValuePair<string, object>("{OriginalFormat}", _formatter.OriginalFormat),
};

public object[] ToArray() => new object[] { _value0, _value1, _value2, _value3, _value4, _value5 };

public override string ToString() => _formatter.Format(ToArray());
}

private class LogValues<T0, T1, T2, T3, T4, T5, T6> : ILogValues
{
public static Func<object, Exception, string> Callback = (state, exception) => ((LogValues<T0, T1, T2, T3, T4, T5, T6>)state)._formatter.Format(((LogValues<T0, T1, T2, T3, T4, T5, T6>)state).ToArray());

private readonly LogValuesFormatter _formatter;
public T0 _value0;
public T1 _value1;
public T2 _value2;
public T3 _value3;
public T4 _value4;
public T5 _value5;
public T6 _value6;

public LogValues(LogValuesFormatter formatter, T0 value0, T1 value1, T2 value2, T3 value3, T4 value4, T5 value5, T6 value6)
{
_formatter = formatter;
_value0 = value0;
_value1 = value1;
_value2 = value2;
_value3 = value3;
_value4 = value4;
_value5 = value5;
_value6 = value6;
}

public IEnumerable<KeyValuePair<string, object>> GetValues() => new[]
{
new KeyValuePair<string, object>(_formatter.ValueNames[0], _value0),
new KeyValuePair<string, object>(_formatter.ValueNames[1], _value1),
new KeyValuePair<string, object>(_formatter.ValueNames[2], _value2),
new KeyValuePair<string, object>(_formatter.ValueNames[3], _value3),
new KeyValuePair<string, object>(_formatter.ValueNames[4], _value4),
new KeyValuePair<string, object>(_formatter.ValueNames[5], _value5),
new KeyValuePair<string, object>(_formatter.ValueNames[6], _value6),
new KeyValuePair<string, object>("{OriginalFormat}", _formatter.OriginalFormat),
};

public object[] ToArray() => new object[] { _value0, _value1, _value2, _value3, _value4, _value5, _value6 };

public override string ToString() => _formatter.Format(ToArray());
}
}
}

42 changes: 42 additions & 0 deletions test/Microsoft.Extensions.Logging.Test/LoggerMessageTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.Remoting.Messaging;
using Microsoft.Extensions.Logging.Test;
using Xunit;
using Xunit.Sdk;
Expand Down Expand Up @@ -162,6 +163,47 @@ public void LogScope_WithThreeParameters()
actualLogValues.ToString());
}

[Theory]
[MemberData(nameof(LogMessagesData))]
public void LogMessages(Delegate messageDelegate, int argumentCount)
{
// Arrange
var testSink = new TestSink();
var testLogger = new TestLogger("testlogger", testSink, enabled: true);
var exception = new Exception("TestException");
var parameterNames = Enumerable.Range(0, argumentCount).Select(i => "P" + i).ToArray();
var parameters = new List<object>();
parameters.Add(testLogger);
parameters.AddRange(parameterNames);
parameters.Add(exception);

var expectedFormat = "Log " + string.Join(" ", parameterNames.Select(p => "{" + p + "}"));
var expectedToString = "Log " + string.Join(" ", parameterNames);
var expectedValues = parameterNames.Select(p => new KeyValuePair<string, object>(p, p)).ToList();
expectedValues.Add(new KeyValuePair<string, object>("{OriginalFormat}", expectedFormat));

// Act
messageDelegate.DynamicInvoke(parameters.ToArray());

// Assert
Assert.Equal(1, testSink.Writes.Count);
var write = testSink.Writes.First();
var actualLogValues = Assert.IsAssignableFrom<ILogValues>(write.State);
AssertLogValues(expectedValues, actualLogValues.GetValues());
Assert.Equal(expectedToString, actualLogValues.ToString());
}

public static IEnumerable<object[]> LogMessagesData => new[]
{
new object[] { LoggerMessage.Define(LogLevel.Error, 0, "Log "), 0 },
new object[] { LoggerMessage.Define<string>(LogLevel.Error, 1, "Log {P0}"), 1 },
new object[] { LoggerMessage.Define<string, string>(LogLevel.Error, 2, "Log {P0} {P1}"), 2 },
new object[] { LoggerMessage.Define<string, string, string>(LogLevel.Error, 3, "Log {P0} {P1} {P2}"), 3 },
new object[] { LoggerMessage.Define<string, string, string, string>(LogLevel.Error, 4, "Log {P0} {P1} {P2} {P3}"), 4 },
new object[] { LoggerMessage.Define<string, string, string, string, string>(LogLevel.Error, 5, "Log {P0} {P1} {P2} {P3} {P4}"), 5 },
new object[] { LoggerMessage.Define<string, string, string, string, string, string>(LogLevel.Error, 6, "Log {P0} {P1} {P2} {P3} {P4} {P5}"), 6 },
};

private void AssertLogValues(
IEnumerable<KeyValuePair<string, object>> expected,
IEnumerable<KeyValuePair<string, object>> actual)
Expand Down