Skip to content
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.

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

Payload limiting #866

Merged
merged 12 commits into from Oct 5, 2016
@@ -27,20 +27,24 @@ public class LoggerDestructuringConfiguration
readonly Action<Type> _addScalar;
readonly Action<IDestructuringPolicy> _addPolicy;
readonly Action<int> _setMaximumDepth;
readonly Action<int> _setMaximumStringLength;

internal LoggerDestructuringConfiguration(
LoggerConfiguration loggerConfiguration,
Action<Type> addScalar,
Action<IDestructuringPolicy> addPolicy,
Action<int> setMaximumDepth)
Action<int> setMaximumDepth,
Action<int> setMaximumStringLength)
{
if (loggerConfiguration == null) throw new ArgumentNullException(nameof(loggerConfiguration));
if (addScalar == null) throw new ArgumentNullException(nameof(addScalar));
if (addPolicy == null) throw new ArgumentNullException(nameof(addPolicy));
if (setMaximumDepth == null) throw new ArgumentNullException(nameof(setMaximumStringLength));
_loggerConfiguration = loggerConfiguration;
_addScalar = addScalar;
_addPolicy = addPolicy;
_setMaximumDepth = setMaximumDepth;
_setMaximumStringLength = setMaximumStringLength;
}

/// <summary>
@@ -149,6 +153,21 @@ public LoggerConfiguration ToMaximumDepth(int maximumDestructuringDepth)
_setMaximumDepth(maximumDestructuringDepth);
return _loggerConfiguration;
}

/// <summary>
/// When destructuring objects, string values can be restricted to specified length
/// thus avoiding bloating payload. Limit is applied to each value separately,
/// sum of length of strings can exceed limit.
/// </summary>
/// <param name="maximumStringLength">The maximum string length.</param>
/// <returns>Configuration object allowing method chaining.</returns>
/// <exception cref="ArgumentOutOfRangeException">When passed length is less or equal to 2</exception>
public LoggerConfiguration ToMaximumStringLength(int maximumStringLength)
{
if (maximumStringLength < 2) throw new ArgumentOutOfRangeException(nameof(maximumStringLength), maximumStringLength, "Maximum string length have to be greater than 1");

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Sep 28, 2016

Member

Exception messages in Serilog are all sentences, so needs a . at the end of the message. Perhaps a slight rewording "Maximum string length must be at least two." ?

_setMaximumStringLength(maximumStringLength);
return _loggerConfiguration;
}
}
}

@@ -39,6 +39,7 @@ public class LoggerConfiguration
LogEventLevel _minimumLevel = LogEventLevel.Information;
LoggingLevelSwitch _levelSwitch;
int _maximumDestructuringDepth = 10;
int _maximumStringLength = 1000;

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Sep 28, 2016

Member

So that this doesn't need to be flagged as a breaking change, perhaps int.MaxValue is a better option here?

bool _loggerCreated;

void ApplyInheritedConfiguration(LoggerConfiguration child)
@@ -109,7 +110,8 @@ public LoggerDestructuringConfiguration Destructure
this,
_additionalScalarTypes.Add,
_additionalDestructuringPolicies.Add,
depth => _maximumDestructuringDepth = depth);
depth => _maximumDestructuringDepth = depth,
length => _maximumStringLength = length);
}
}

@@ -150,7 +152,12 @@ public Logger CreateLogger()
sink = new FilteringSink(sink, _filters, auditing);
}

var converter = new PropertyValueConverter(_maximumDestructuringDepth, _additionalScalarTypes, _additionalDestructuringPolicies, auditing);
var converter = new PropertyValueConverter(
_maximumDestructuringDepth,
_maximumStringLength,
_additionalScalarTypes,
_additionalDestructuringPolicies,
auditing);
var processor = new MessageTemplateProcessor(converter);

ILogEventEnricher enricher;
@@ -46,20 +46,24 @@ partial class PropertyValueConverter : ILogEventPropertyFactory, ILogEventProper
readonly IDestructuringPolicy[] _destructuringPolicies;
readonly IScalarConversionPolicy[] _scalarConversionPolicies;
readonly int _maximumDestructuringDepth;
readonly int _maximumStringLength;
readonly bool _propagateExceptions;

public PropertyValueConverter(
int maximumDestructuringDepth,
int maximumStringLength,
IEnumerable<Type> additionalScalarTypes,
IEnumerable<IDestructuringPolicy> additionalDestructuringPolicies,
bool propagateExceptions)
{
if (additionalScalarTypes == null) throw new ArgumentNullException(nameof(additionalScalarTypes));
if (additionalDestructuringPolicies == null) throw new ArgumentNullException(nameof(additionalDestructuringPolicies));
if (maximumDestructuringDepth < 0) throw new ArgumentOutOfRangeException(nameof(maximumDestructuringDepth));
if (maximumStringLength < 2) throw new ArgumentOutOfRangeException(nameof(maximumDestructuringDepth));

_maximumDestructuringDepth = maximumDestructuringDepth;
_propagateExceptions = propagateExceptions;
_maximumStringLength = maximumStringLength;

_scalarConversionPolicies = new IScalarConversionPolicy[]
{
@@ -121,11 +125,19 @@ LogEventPropertyValue CreatePropertyValue(object value, Destructuring destructur
return new ScalarValue(null);

if (destructuring == Destructuring.Stringify)
return new ScalarValue(value.ToString());
{
return Stringify(value);
}

var valueType = value.GetType();
var limiter = new DepthLimiter(depth, _maximumDestructuringDepth, this);

var stringValue = value as string;
if (stringValue != null)
{
value = TruncateIfNecessary(stringValue);
}

foreach (var scalarConversionPolicy in _scalarConversionPolicies)
{
ScalarValue converted;
@@ -183,7 +195,24 @@ LogEventPropertyValue CreatePropertyValue(object value, Destructuring destructur
return new StructureValue(GetProperties(value, limiter), typeTag);
}

return new ScalarValue(value.ToString());
return Stringify(value);
}

private LogEventPropertyValue Stringify(object value)

This comment has been minimized.

Copy link
@nblumhardt

nblumhardt Oct 5, 2016

Member

Nitpick (can pick this up later): the codebase doesn't use any explicit private/internal modifiers.

This comment has been minimized.

Copy link
@krajek

krajek Oct 5, 2016

Author Contributor

Sure, will be fixed.

{
var stringified = value.ToString();
var truncated = TruncateIfNecessary(stringified);
return new ScalarValue(truncated);
}

string TruncateIfNecessary(string text)
{
if (text.Length > _maximumStringLength)
{
return text.Substring(0, _maximumStringLength - 1) + "";
}

return text;
}

bool IsValueTypeDictionary(Type valueType)
@@ -241,3 +270,4 @@ internal static bool IsCompilerGeneratedType(Type type)
}
}
}

@@ -179,7 +179,7 @@ static IEnumerable<LogEventProperty> Capture(string messageTemplate, params obje
{
var mt = new MessageTemplateParser().Parse(messageTemplate);
var binder = new PropertyBinder(
new PropertyValueConverter(10, Enumerable.Empty<Type>(), Enumerable.Empty<IDestructuringPolicy>(), false));
new PropertyValueConverter(10, 1000, Enumerable.Empty<Type>(), Enumerable.Empty<IDestructuringPolicy>(), false));
return binder.ConstructProperties(mt, properties);
}

@@ -126,7 +126,7 @@ static string Render(string messageTemplate, params object[] properties)
static string Render(IFormatProvider formatProvider, string messageTemplate, params object[] properties)
{
var mt = new MessageTemplateParser().Parse(messageTemplate);
var binder = new PropertyBinder(new PropertyValueConverter(10, Enumerable.Empty<Type>(), Enumerable.Empty<IDestructuringPolicy>(), false));
var binder = new PropertyBinder(new PropertyValueConverter(10, 1000, Enumerable.Empty<Type>(), Enumerable.Empty<IDestructuringPolicy>(), false));
var props = binder.ConstructProperties(mt, properties);
var output = new StringBuilder();
var writer = new StringWriter(output);
@@ -27,7 +27,7 @@ namespace Serilog.Tests.Events
public class LogEventPropertyValueTests
{
readonly PropertyValueConverter _converter =
new PropertyValueConverter(10, Enumerable.Empty<Type>(), Enumerable.Empty<IDestructuringPolicy>(), false);
new PropertyValueConverter(10, 1000, Enumerable.Empty<Type>(), Enumerable.Empty<IDestructuringPolicy>(), false);

[Fact]
public void AnEnumIsConvertedToANonStringScalarValue()
@@ -3,6 +3,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Runtime.InteropServices.ComTypes;
using Xunit;
using Serilog.Core;
using Serilog.Core.Filters;
@@ -277,6 +278,102 @@ public void MaximumDestructuringDepthIsEffective()
Assert.DoesNotContain(xs, "D");
}

[Fact]
public void MaximumStringLengthThrowsForLimitLowerThan2()
{
var ex = Assert.Throws<ArgumentOutOfRangeException>(
() => new LoggerConfiguration().Destructure.ToMaximumStringLength(1));
Assert.Equal(1, ex.ActualValue);
}

[Fact]
public void MaximumStringLengthEffectiveForString()
{
var x = "ABCD";

LogEvent evt = null;
var log = new LoggerConfiguration()
.WriteTo.Sink(new DelegatingSink(e => evt = e))
.Destructure.ToMaximumStringLength(3)
.CreateLogger();

log.Information("{X}", x);
var limitedText = evt.Properties["X"].ToString();

Assert.Equal("\"AB…\"", limitedText);
}

[Theory]
[InlineData("1234", "12…", 3)]
[InlineData("123", "123", 3)]
public void MaximumStringLengthEffectiveForCapturedObject(string text, string textAfter, int limit)
{
var x = new
{
TooLongText = text
};

LogEvent evt = null;
var log = new LoggerConfiguration()
.WriteTo.Sink(new DelegatingSink(e => evt = e))
.Destructure.ToMaximumStringLength(limit)
.CreateLogger();

log.Information("{@X}", x);
var limitedText = evt.Properties["X"].ToString();

Assert.Contains(textAfter, limitedText);
}

[Fact]
public void MaximumStringLengthEffectiveForStringifiedObject()
{
var x = new ToStringOfLength(4);

LogEvent evt = null;
var log = new LoggerConfiguration()
.WriteTo.Sink(new DelegatingSink(e => evt = e))
.Destructure.ToMaximumStringLength(3)
.CreateLogger();

log.Information("{$X}", x);
var limitedText = evt.Properties["X"].ToString();

Assert.Contains("##…", limitedText);
}

[Fact]
public void MaximumStringLengthEffectiveForObject()
{
var x = new ToStringOfLength(4);

LogEvent evt = null;
var log = new LoggerConfiguration()
.WriteTo.Sink(new DelegatingSink(e => evt = e))
.Destructure.ToMaximumStringLength(3)
.CreateLogger();

log.Information("{X}", x);
var limitedText = evt.Properties["X"].ToString();

Assert.Contains("##…", limitedText);
}

class ToStringOfLength
{
private int _toStringOfLength;

public ToStringOfLength(int toStringOfLength)
{
_toStringOfLength = toStringOfLength;
}

public override string ToString()
{
return new string('#', _toStringOfLength);
}
}

[Fact]
public void DynamicallySwitchingSinkRestrictsOutput()
{
@@ -13,7 +13,7 @@ namespace Serilog.Tests.Parameters
public class PropertyValueConverterTests
{
readonly PropertyValueConverter _converter =
new PropertyValueConverter(10, Enumerable.Empty<Type>(), Enumerable.Empty<IDestructuringPolicy>(), false);
new PropertyValueConverter(10, 1000, Enumerable.Empty<Type>(), Enumerable.Empty<IDestructuringPolicy>(), false);

[Fact]
public void UnderDestructuringAByteArrayIsAScalarValue()
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.