Fix MongoDB Bson properties with dots

[IvaskevychYuriy]

This is a fixed version of the following fork - https://github.com/DarkXoro/serilog-sinks-mongodb Proposed in PR - https://github.com/serilog/serilog-sinks-mongodb/pull/45/ (not merged) Previous issue (closed but not resolved) - https://github.com/serilog/serilog-sinks-mongodb/issues/29

New issue - https://github.com/serilog/serilog-sinks-mongodb/issues/58

[IvaskevychYuriy]

Hi @nblumhardt , I'm wondering if this this repo is still maintained?

[nblumhardt]

Hi Yuriy, thanks for the nudge. :+1:

Not really, unfortunately - it's very minimally maintained.

This problem is best addressed by passing a suitable ITextFormatter in the WriteTo.MongoDB() call. It's the job of the formatter to determine how properties are represented in the event documents.

One option would be to create an ITextFormatter specifically for MongoDB, e.g. by copying the basics from https://github.com/serilog/serilog-formatting-compact/blob/dev/src/Serilog.Formatting.Compact/Formatting/Compact/CompactJsonFormatter.cs.

This could be linked from the README, or if it's widely-used, included in this package.

(Another option, entirely within the sink, would be to copy and modify the LogEvent before passing it to the formatter.)

Using RegEx for this seems like a last resort, given the usual role of ITextFormatter to do this job.

[Jaben]

@nblumhardt FYI, recently, I did a bit of reimagining on this project. To me, it's grown too complex due to its "double conversion" ways (to Json and then to Bson). I made a version that directly stores the logging output in mongo using native Bson: https://github.com/ChangemakerStudios/serilog-sinks-mongodb/tree/dev

I prefer the more native output, and it solves some of the issues with the current "formatting" strategy, but it's a radical change -- so I'm not entirely sure what to do with the changes:

[nblumhardt]

@Jaben sounds interesting... I keep forgetting that you still use this one - a refresh could be what this sink needs?

One possibility would be to introduce new overloads for WriteTo.MongoDB() that don't accept ITextFormatter, and use the new mechanism under the hood. The old overloads could then have all the parameters non-defaulted, so that existing code would still compile and link successfully, while the new overloads would be picked by the compiler unless the formatter` argument is supplied?

Just an idle thought!

[Jaben]

@nblumhardt I don't use it, really -- Seq is too good ;) I do use MongoDb a lot, though.

So I added MongoDbBson() extensions which output using the Bson structured data. The extensions also have a simpler configuration system (fluent). The current sink extension functions are huge -- figured best to keep them API compatible, though, so there are fewer surprises on upgrade.

I'll put in a PR for it.

[IvaskevychYuriy]

@Jaben Yeah, I like your approach, maybe it would be better to use your proposal indeed. I've checked out your fork and it handles properties as expected but fails when there's exception object with same issue. So here's the test I've come up with (requires Mongo setup):

// ----------------------- helper --------------------------------------------
        class CustomException : Exception
        {
            public CustomException(string message) : base(message) { }
            public object SomeContext { get; set; }
        }

// ---------------------- setup ----------------------------------------------
        var logger = new LoggerConfiguration()
                .WriteTo.MongoDbBson("mongodb://localhost/logs", cfg =>
                {
                    cfg.SetCollectionName("log");
                    cfg.SetBatchPeriod(TimeSpan.FromSeconds(1));
                })
                .CreateLogger();

 // --------------------- actual test ----------------------------------------
            // Arrange
            string errorMessage = null;
            Debugging.SelfLog.Enable(msg => errorMessage = msg);

            var exception = new CustomException("error");
            exception.Data["test.exc"] = "let's try";
            exception.SomeContext = new Dictionary<string, object>() { ["custom.exc"] = "jhsdgf" };

            var data = new Dictionary<string, object>()
            {
                ["test.abc"] = "abc",
                ["other"] = new Dictionary<string, string>() { ["test2.abc2"] = "abc2" }
            };

            // Act
            logger.Information(exception, "Message Template 1 {@Prop_1}", data);

            // wait for logger to flush
            Thread.Sleep(1500);

            // Assert
            Assert.IsNull(errorMessage, errorMessage);

[IvaskevychYuriy]

Btw @Jaben , here's what I come up with (using your fork) - updated LogEntry.cs like so:

public BsonDocument Exception { get; set; }

Exception = logEvent.Exception == null ? null : SanitizeDocumentRecursive(logEvent.Exception.ToBsonDocument()),

private static BsonDocument SanitizeDocumentRecursive(BsonDocument document)
{
    var sanitizedElements = document.Select(e => new BsonElement(
        SanitizedElementName(e.Name),
        e.Value.IsBsonDocument ? SanitizeDocumentRecursive(e.Value.AsBsonDocument) : e.Value));

    return new BsonDocument(sanitizedElements);
}

This fixes an issue with exception serialization in test I've mentioned. What do you think about this?

[IvaskevychYuriy]

Actually doing the same in my fork insted of regex matching or custom formatter does the same thing. file MongoDBHelpers.cs, last line in method GenerateBsonDocuments

return bson["logEvents"].AsBsonArray.Select(x => SanitizeDocumentRecursive(x.AsBsonDocument)).ToList();

[Jaben]

Btw @Jaben , here's what I come up with (using your fork) - updated LogEntry.cs like so:

public BsonDocument Exception { get; set; }

Exception = logEvent.Exception == null ? null : SanitizeDocumentRecursive(logEvent.Exception.ToBsonDocument()),

private static BsonDocument SanitizeDocumentRecursive(BsonDocument document)
{
    var sanitizedElements = document.Select(e => new BsonElement(
        SanitizedElementName(e.Name),
        e.Value.IsBsonDocument ? SanitizeDocumentRecursive(e.Value.AsBsonDocument) : e.Value));

    return new BsonDocument(sanitizedElements);
}

This fixes an issue with exception serialization in test I've mentioned. What do you think about this?

That's great! Thanks for finding and fixing this issue -- which I've added to the PR. 👍

[IvaskevychYuriy]

Sounds good, if you open a PR then this one can be closed.

[Jaben]

Sounds good, if you open a PR then this one can be closed.

Done. It's here: https://github.com/serilog/serilog-sinks-mongodb/pull/60

[IvaskevychYuriy]

Great, closing this one!