search

ChangemakerStudios / serilog-sinks-mongodb

A sink for Serilog that writes events to MongoDB
Apache License 2.0
92 stars 53 forks source link

Needed to parse the values of the dictionary, as well. #41

Closed IcanBENCHurCAT closed 2 years ago

IcanBENCHurCAT commented 5 years ago

The previous fix would work if the offending characters were in the key of the element to be written. When that element contains dictionaries, those keys need to be parsed as well (recursively)

IcanBENCHurCAT commented 5 years ago

@nblumhardt mind taking a look at this?

nblumhardt commented 5 years ago

Hi @IcanBENCHurCAT - would overriding WriteDictionary() be simpler and require fewer allocations?

https://github.com/serilog/serilog/blob/dev/src/Serilog/Formatting/Json/JsonFormatter.cs#L329

IcanBENCHurCAT commented 5 years ago

Yes, I like that approach much better. I'll update and let you know.

IcanBENCHurCAT commented 5 years ago

@nblumhardt I think I've done a better job with this commit. It definitely seems more readable, but I couldn't see much that I could cut down on allocations.

This code works well in our main package, executes without error and I cannot notice any kind of performance differences.

Jaben commented 3 years ago

@IcanBENCHurCAT Do you feel this PR is still needed with direct BSON support in the latest version?