Skip to content

Commit

Permalink
Address PR comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
normj committed Apr 2, 2024
1 parent 21d32b5 commit 924b1cd
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 82 deletions.
12 changes: 6 additions & 6 deletions Libraries/src/Amazon.Lambda.Core/ILambdaLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ public interface ILambdaLogger
private const string ParameterizedPreviewMessage =
"Parameterized logging is in preview till a new version of .NET Lambda runtime client that supports parameterized logging " +
"has been deployed to the .NET Lambda managed runtime. Till deployment has been made the feature can be used by deploying as an " +
"executable including the latest version of Amazon.Lambda.RuntimeSupport and setting the \"LangVersion\" in the Lambda " +
"project file to \"preview\"";
"executable including the latest version of Amazon.Lambda.RuntimeSupport and setting the \"EnablePreviewFeatures\" in the Lambda " +
"project file to \"true\"";

/// <summary>
/// Log message catagorized by the given log level
Expand All @@ -164,7 +164,7 @@ public interface ILambdaLogger
/// <param name="message">Message to log.</param>
/// <param name="args">Values to be replaced in log messages that are parameterized.</param>
[RequiresPreviewFeatures(ParameterizedPreviewMessage)]
void Log(string level, string message, params object[] args) => Log(level, message);
void Log(string level, string message, params object[] args) => Log(level, message, args);

/// <summary>
/// Log message catagorized by the given log level
Expand All @@ -177,11 +177,11 @@ public interface ILambdaLogger
/// <param name="exception">Exception to include with the logging.</param>
/// <param name="message">Message to log.</param>
/// <param name="args">Values to be replaced in log messages that are parameterized.</param>
[RequiresPreviewFeatures("Parameterized logging is in preview till new version of .NET Lambda runtime client is deployed to the .NET Lambda managed runtime.")]
[RequiresPreviewFeatures(ParameterizedPreviewMessage)]
void Log(string level, Exception exception, string message, params object[] args)
{
Log(level, message);
Log(level, exception.ToString());
Log(level, message, args);
Log(level, exception.ToString(), args);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,30 @@ public interface IConsoleLoggerWriter
/// <summary>
/// The current aws request id
/// </summary>
/// <param name="awsRequestId"></param>
/// <param name="awsRequestId">The AWS request id for the function invocation added to each log message.</param>
void SetCurrentAwsRequestId(string awsRequestId);

/// <summary>
/// Format message with default log level
/// </summary>
/// <param name="message"></param>
/// <param name="message">Message to log.</param>
void FormattedWriteLine(string message);

/// <summary>
/// Format message with given log level
/// </summary>
/// <param name="level"></param>
/// <param name="message"></param>
/// <param name="args"></param>
/// <param name="level">The level of the log message.</param>
/// <param name="message">Message to log.</param>
/// <param name="args">Arguments to be applied to the log message.</param>
void FormattedWriteLine(string level, string message, params object[] args);

/// <summary>
/// Format message with given log level
/// </summary>
/// <param name="level"></param>
/// <param name="exception"></param>
/// <param name="message"></param>
/// <param name="args"></param>
/// <param name="level">The level of the log message.</param>
/// <param name="exception">Exception to log.</param>
/// <param name="message">Message to log.</param>
/// <param name="args">Arguments to be applied to the log message.</param>
void FormattedWriteLine(string level, Exception exception, string message, params object[] args);
}

Expand Down Expand Up @@ -112,7 +112,10 @@ public void FormattedWriteLine(string level, string message, params object[] arg
public void FormattedWriteLine(string level, Exception exception, string message, params object[] args)
{
_writer.WriteLine(message);
_writer.WriteLine(exception.ToString());
if (exception != null)
{
_writer.WriteLine(exception.ToString());
}
}
}

Expand Down Expand Up @@ -310,6 +313,10 @@ public WrapperTextWriter(TextWriter innerWriter, string defaultLogLevel)
{
_minmumLogLevel = result;
}
else
{
InternalLogger.GetDefaultLogger().LogInformation($"Failed to parse log level enum value: {envLogLevel}");
}
}

var envLogFormat = GetEnviromentVariable(NET_RIC_LOG_FORMAT_ENVIRONMENT_VARIABLE, LAMBDA_LOG_FORMAT_ENVIRONMENT_VARIABLE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ private enum LogFormatParserState : byte
/// <summary>
/// Parse the message template for all message properties.
/// </summary>
/// <param name="messageTemplate"></param>
/// <returns></returns>
/// <param name="messageTemplate">The message template users passed in as the log message.</param>
/// <returns>List of MessageProperty objects detected by parsing the message template.</returns>
public virtual IReadOnlyList<MessageProperty> ParseProperties(string messageTemplate)
{
// Check to see if this message template has already been parsed before.
Expand Down Expand Up @@ -101,17 +101,21 @@ public virtual IReadOnlyList<MessageProperty> ParseProperties(string messageTemp
/// <summary>
/// Subclasses to implement to format the message given the requirements of the subclass.
/// </summary>
/// <param name="state"></param>
/// <returns></returns>
/// <param name="state">The state of the message to log.</param>
/// <returns>The full log message to send to CloudWatch Logs.</returns>
public abstract string FormatMessage(MessageState state);

internal const string DateFormat = "yyyy-MM-ddTHH:mm:ss.fffZ";

internal const string DateOnlyFormat = "yyyy-MM-dd";

internal const string TimeOnlyFormat = "HH:mm:ss.fff";

/// <summary>
/// Format the timestamp of the log message in format Lambda service prefers.
/// </summary>
/// <param name="state"></param>
/// <returns></returns>
/// <param name="state">The state of the message to log.</param>
/// <returns>Timestamp formatted for logging.</returns>
protected string FormatTimestamp(MessageState state)
{
return state.TimeStamp.ToString(DateFormat, CultureInfo.InvariantCulture);
Expand All @@ -123,7 +127,7 @@ protected string FormatTimestamp(MessageState state)
/// <param name="messageTemplate"></param>
/// <param name="messageProperties"></param>
/// <param name="messageArguments"></param>
/// <returns></returns>
/// <returns>The log message with logging arguments replaced with the values.</returns>
public string ApplyMessageProperties(string messageTemplate, IReadOnlyList<MessageProperty> messageProperties, object[] messageArguments)
{
if(messageProperties.Count == 0)
Expand Down Expand Up @@ -243,14 +247,15 @@ public string ApplyMessageProperties(string messageTemplate, IReadOnlyList<Messa
/// Formatted Message: "Lewis 15 Washington"
/// </summary>
/// <param name="messageProperties"></param>
/// <returns></returns>
/// <returns>True of the logging arguments are positional</returns>
public bool UsingPositionalArguments(IReadOnlyList<MessageProperty> messageProperties)
{
var min = int.MaxValue;
int max = int.MinValue;
HashSet<int> positions = new HashSet<int>();
foreach(var property in messageProperties)
{
// If any logging arguments use non-numeric identifier then they are not using positional arguments.
if (!int.TryParse(property.Name, NumberStyles.Integer, CultureInfo.InvariantCulture, out var position))
{
return false;
Expand All @@ -267,6 +272,10 @@ public bool UsingPositionalArguments(IReadOnlyList<MessageProperty> messagePrope
}
}

// At this point the HashSet is the collection of all of the int logging arguments.
// If there are no gaps or duplicates in the logging statement then the smallest value
// in the hashset should be 0 and the max value equals the count of the hashset. If
// either of those conditions are not true then it can't be positional arguments.
if(positions.Count != (max + 1) || min != 0)
{
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,69 +109,71 @@ private void WriteMessageAttributes(Utf8JsonWriter writer, IReadOnlyList<Message
// "User bought {0} of {1}" is positional as opposed to "User bought {count} of {product"}.
var usePositional = UsingPositionalArguments(messageProperties);

if (messageProperties != null)
if (messageProperties == null)
{
for (var i = 0; i < messageProperties.Count; i++)
return;
}

for (var i = 0; i < messageProperties.Count; i++)
{
object messageArgument;
if(usePositional)
{
object messageArgument;
if(usePositional)
// If usePositional is true then we have confirmed the `Name` property is an int.
var index = int.Parse(messageProperties[i].Name, CultureInfo.InvariantCulture);
if (index < state.MessageArguments.Length)
{
// If usePositional is true then we have confirmed the `Name` property is an int.
var index = int.Parse(messageProperties[i].Name, CultureInfo.InvariantCulture);
if (index < state.MessageArguments.Length)
{
// Don't include null JSON properties
if (state.MessageArguments[index] == null)
continue;

messageArgument = state.MessageArguments[index];
}
else
{
// Don't include null JSON properties
if (state.MessageArguments[index] == null)
continue;
}

messageArgument = state.MessageArguments[index];
}
else
{
// There are more message properties in the template then values for the properties. Skip
// adding anymore JSON properties since there are no more values.
if (state.MessageArguments.Length <= i)
break;
continue;
}
}
else
{
// There are more message properties in the template then values for the properties. Skip
// adding anymore JSON properties since there are no more values.
if (state.MessageArguments.Length <= i)
break;

// Don't include null JSON properties
if (state.MessageArguments[i] == null)
continue;
// Don't include null JSON properties
if (state.MessageArguments[i] == null)
continue;

messageArgument = state.MessageArguments[i];
messageArgument = state.MessageArguments[i];

}
}

writer.WritePropertyName(messageProperties[i].Name);
writer.WritePropertyName(messageProperties[i].Name);

if (messageArgument is IList && messageArgument is not IList<byte>)
if (messageArgument is IList && messageArgument is not IList<byte>)
{
writer.WriteStartArray();
foreach (var item in ((IList)messageArgument))
{
writer.WriteStartArray();
foreach (var item in ((IList)messageArgument))
{
FormatJsonValue(writer, item, messageProperties[i].FormatArgument, messageProperties[i].FormatDirective);
}
writer.WriteEndArray();
FormatJsonValue(writer, item, messageProperties[i].FormatArgument, messageProperties[i].FormatDirective);
}
else if (messageArgument is IDictionary)
writer.WriteEndArray();
}
else if (messageArgument is IDictionary)
{
writer.WriteStartObject();
foreach (DictionaryEntry entry in ((IDictionary)messageArgument))
{
writer.WriteStartObject();
foreach (DictionaryEntry entry in ((IDictionary)messageArgument))
{
writer.WritePropertyName(entry.Key.ToString());
writer.WritePropertyName(entry.Key.ToString());

FormatJsonValue(writer, entry.Value, messageProperties[i].FormatArgument, messageProperties[i].FormatDirective);
}
writer.WriteEndObject();
}
else
{
FormatJsonValue(writer, messageArgument, messageProperties[i].FormatArgument, messageProperties[i].FormatDirective);
FormatJsonValue(writer, entry.Value, messageProperties[i].FormatArgument, messageProperties[i].FormatDirective);
}
writer.WriteEndObject();
}
else
{
FormatJsonValue(writer, messageArgument, messageProperties[i].FormatArgument, messageProperties[i].FormatDirective);
}
}
}
Expand Down Expand Up @@ -271,6 +273,12 @@ private void FormatJsonValue(Utf8JsonWriter writer, object value, string formatA
case DateTimeOffset dateTimeOffsetValue:
writer.WriteStringValue(dateTimeOffsetValue.ToString(DateFormat, CultureInfo.InvariantCulture));
break;
case DateOnly dateOnly:
writer.WriteStringValue(dateOnly.ToString(DateOnlyFormat, CultureInfo.InvariantCulture));
break;
case TimeOnly timeOnly:
writer.WriteStringValue(timeOnly.ToString(TimeOnlyFormat, CultureInfo.InvariantCulture));
break;
case byte[] byteArrayValue:
writer.WriteStringValue(MessageProperty.FormatByteArray(byteArrayValue));
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class MessageProperty
private static readonly char[] PARAM_FORMAT_DELIMITERS = { ':' };

/// <summary>
/// Parse the string string representation of the message property without the brackets
/// Parse the string representation of the message property without the brackets
/// to construct the MessageProperty.
/// </summary>
/// <param name="messageToken"></param>
Expand Down Expand Up @@ -57,7 +57,19 @@ public MessageProperty(ReadOnlySpan<char> messageToken)
/// </summary>
public string MessageToken { get; private set; }

public enum Directive { Default, JsonSerialization };
/// <summary>
/// Enum for controlling the formatting of complex logging arguments.
/// </summary>
public enum Directive {
/// <summary>
/// Perform a string formatting for the logging argument.
/// </summary>
Default,
/// <summary>
/// Perform a JSON serialization on the logging argument.
/// </summary>
JsonSerialization
};

/// <summary>
/// The Name of the message property.
Expand Down Expand Up @@ -110,6 +122,14 @@ public string FormatForMessage(object value)
{
return dto.ToString(AbstractLogMessageFormatter.DateFormat, CultureInfo.InvariantCulture);
}
if (value is DateOnly dateOnly)
{
return dateOnly.ToString(AbstractLogMessageFormatter.DateOnlyFormat, CultureInfo.InvariantCulture);
}
if (value is TimeOnly timeOnly)
{
return timeOnly.ToString(AbstractLogMessageFormatter.TimeOnlyFormat, CultureInfo.InvariantCulture);
}

return value.ToString();
}
Expand Down
Loading

0 comments on commit 924b1cd

Please sign in to comment.