seq-tickets icon indicating copy to clipboard operation
seq-tickets copied to clipboard

Seq is rejecting payloads with large numbers

Open DanAvni opened this issue 3 years ago • 6 comments

if you declare a double variable and assign it a value of 8.2535170109512364E+28 then try to log this seq rejects the payload with this message

[2021-04-12T11:52:38.8713351+03:00 DBG] Rejecting payload from 127.0.0.1 due to invalid JSON, request body could not be parsed
Newtonsoft.Json.JsonReaderException: Input string '8.2535170109512364E+28' is not a valid decimal. Path 'Events[1].Properties.Cost', line 1, position 743.
   at Newtonsoft.Json.JsonTextReader.ParseReadNumber(ReadType readType, Char firstChar, Int32 initialPosition)
   at Newtonsoft.Json.JsonTextReader.ParseValue()
   at Newtonsoft.Json.JsonWriter.WriteToken(JsonReader reader, Boolean writeChildren, Boolean writeDateConstructorAsDate, Boolean writeComments)
   at Newtonsoft.Json.JsonWriter.WriteToken(JsonReader reader, Boolean writeChildren)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateJToken(JsonReader reader, JsonContract contract)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonSerializer.Deserialize[T](JsonReader reader)
   at Seq.Server.Web.Api.IngestionController.TryParseRawFormatBody(Input input, InputSettings inputSettings, StorageEventCreationData[]& events, String& errorMessage)

DanAvni avatar Apr 12 '21 09:04 DanAvni

Thanks, Dani 👍

nblumhardt avatar Apr 12 '21 23:04 nblumhardt

This is very similar to datalust/seq-tickets#496, which addresses a similar issue deeper in the ingestion process by converting any floating point too big to fit Seq's underlying decimal numeric representation into a string.

Later Seq versions switched to using decimal handling for floating point values right out in the ingestion API to avoid loss of precision due to floating point handling. I'm not sure what the intended/original behavior was in this case, it seems unlikely we'd have missed it entirely... More investigation required... 🤔

nblumhardt avatar Oct 05 '21 08:10 nblumhardt

This needs deeper work than we can squeeze into the final few days of 2021.3, so moving this out of the milestone for scheduling in our next triage.

nblumhardt avatar Oct 11 '21 04:10 nblumhardt

There are some substantial hurdles on the server-side that will prevent us from addressing it directly in Seq, but here in Serilog.Sinks.Seq, (and we think in the other client libraries,) we can customize how very large double values are serialized, by overriding FormatLiteral(), to avoid the problem:

https://github.com/serilog/serilog/blob/dev/src/Serilog/Formatting/Json/JsonValueFormatter.cs#L159

When the serializer encounters an un-representable double, it can record it as a string.

Because Seq uses decimal as its internal numeric representation, numbers larger than decimal.MaxValue will not be able to be manipulated as numeric, but this will at least prevent the event from being lost entirely.

nblumhardt avatar Mar 08 '22 03:03 nblumhardt

@nblumhardt , Great solution! The only caveat I see in it is that for the same event type the same property in Seq could sometimes be numeric and sometimes a string but the users of Seq will think of the value as numeric either way so as long as a Seq query handles that when I query for something like "Field = 1234567890" that is ok.

DanAvni avatar Mar 08 '22 05:03 DanAvni

I've put together a large-double-to-string enricher which implements the workaround client-side:

using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using Serilog;
using Serilog.Core;
using Serilog.Debugging;
using Serilog.Events;

Log.Logger = new LoggerConfiguration()
    .Enrich.With<LargeDoubleToStringEnricher>()
    .WriteTo.Console()
    .AuditTo.Seq("http://localhost:5341")
    .CreateLogger();

Log.Information("Okay {Double}", 123.0);
Log.Information("Replaced {Double}", 8.2535170109512364E+28);
Log.Information("Replaced Nested {@Structure}", new { Double = 8.2535170109512364E+28, Untouched = "Test" });

Log.CloseAndFlush();


class LargeDoubleToStringEnricher : ILogEventEnricher
{
    public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
    {
        foreach (var property in logEvent.Properties)
        {
            if (TryReplaceValue(property.Value, out var replacement))
            {
                SelfLog.WriteLine("Replacing large double value with string to prevent ingestion failure");
                logEvent.AddOrUpdateProperty(new LogEventProperty(property.Key, replacement));
            }
        }
    }

    static bool TryReplaceValue(LogEventPropertyValue value, [NotNullWhen(true)] out LogEventPropertyValue? replacement)
    {
        if (value is ScalarValue { Value: double d and (>= (double)decimal.MaxValue or <= (double)decimal.MinValue) })
        {
            replacement = new ScalarValue(d.ToString(null, CultureInfo.InvariantCulture));
            return true;
        }

        if (value is SequenceValue sequence)
        {
            if (sequence.Elements.Any(e => TryReplaceValue(e, out _)))
            {
                var updated = sequence.Elements.Select(element => 
                    TryReplaceValue(element, out var replacedElement) ? replacedElement : element).ToList();
                replacement = new SequenceValue(updated);
                return true;
            }
        }

        if (value is StructureValue structure)
        {
            if (structure.Properties.Any(p => TryReplaceValue(p.Value, out _)))
            {
                var updated = structure.Properties.Select(property => 
                    TryReplaceValue(property.Value, out var replacedProperty) ? new LogEventProperty(property.Name, replacedProperty) : property).ToList();
                replacement = new StructureValue(updated);
                return true;
            }
        }
        
        if (value is DictionaryValue dictionary)
        {
            if (dictionary.Elements.Any(p => TryReplaceValue(p.Value, out _)))
            {
                var updated = dictionary.Elements.Select(element => 
                    TryReplaceValue(element.Value, out var replacedProperty) ? KeyValuePair.Create(element.Key, replacedProperty) : element).ToList();
                replacement = new DictionaryValue(updated);
                return true;
            }
        }

        replacement = null;
        return false;
    }
}

I'll move this ticket over to the Seq repository, since it's not an issue with this sink, specifically. We'll figure out a longer-term solution over there.

Thanks again for reporting this, @DanAvni 👍

nblumhardt avatar Oct 21 '22 22:10 nblumhardt