ChangemakerStudios / serilog-sinks-mongodb

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

Mongodb sink throws when a dictionary key contains "." #29

Closed devsko closed 5 years ago

devsko commented 7 years ago

Hi, the keys of a Dictionary<string,...> are used as field names. This throws an exception when a key contains "." or starts with "$". These characters should be escaped somehow. Thanks

steveofficer commented 7 years ago

I would like it if keys with '.' characters were converted into nested objects. So something like:

{ 
  "Source.Target1" : "Text1",
  "Source.Target2" : "Text2",
  "Source.Details.Setting" : "ABC"
}

Would become:

{ 
  "Source" : {
     "Target1" : "Text1",
     "Target2" : "Text2",
     "Details" : {
         "Setting" : "ABC"
     }
  }
}
nblumhardt commented 7 years ago

@steveofficer I think it's a very interesting scenario to consider, but probably belongs in the general Serilog code rather than as a special case in this sink (otherwise, for example, a message template like "Hello {User.Name}" could not be rendered correctly from the resulting data in MongoDB).

If you could raise a issue in the main Serilog repo with information about your scenario I think it'll be an interesting discussion to have.

In the case of this sink, escaping whatever property names Serilog hands to it still seems like the right behaviour to me.

carmbrester commented 5 years ago

I've run into this scenario as well, though in our case it appears to be while trying to serialize an exception object. The exception message thrown in the sink is...

Exception while emitting periodic batch from Serilog.Sinks.MongoDB.MongoDBSink: System.AggregateException: One or more errors occurred. (Element name 'HelpLink.ProdName' is not valid'.) ---> MongoDB.Bson.BsonSerializationException: Element name 'HelpLink.ProdName' is not valid'.

The exception type is SqlException, so it's not something we have much control over. When I serialize it to json, I see the property in question along with 5 other properties that also contain the "HelpLink" dotted prefix...

{ "ClassName": "System.Data.SqlClient.SqlException", "Message": "Error converting data type varchar to numeric.", "Data": { "HelpLink.ProdName": "Microsoft SQL Server", "HelpLink.ProdVer": "11.00.7462", "HelpLink.EvtSrc": "MSSQLServer", "HelpLink.EvtID": "8114", "HelpLink.BaseHelpUrl": "http://go.microsoft.com/fwlink", "HelpLink.LinkId": "20476", "SqlError 1": "System.Data.SqlClient.SqlError: Error converting data type varchar to numeric." }, "InnerException": null, "HelpURL": null, "StackTraceString": " at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)\r\n at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)\r\n at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)\r \n at System.Data.SqlClient.SqlDataReader.TryHasMoreRows(Boolean& moreRows)\r\n at System.Data.SqlClient.SqlDataReader.TryReadInternal(Boolean setTimeout, Boolean& more)\r\n at System.Data.SqlClient.SqlDataReader.Read()...", "RemoteStackTraceString": null, "RemoteStackIndex": 0, "ExceptionMethod": null, "HResult": -2146232060, "Source": "Core .Net SqlClient Data Provider", "WatsonBuckets": null, "Errors": null, "ClientConnectionId": "5e009d57-6057-439b-9018-258d8c2f5d5e" }

Given that the property name is embedded within an exception that we do not control, is there really anything that can be done?

IcanBENCHurCAT commented 5 years ago

So, I only solved for the Keys having the offensive characters, but each object within that value can be a dictionary whose keys also contain invalid characters.

Looking into a solution, will send another PR when I have something working.

DarkXoro commented 5 years ago

For now I just imported the repo as a class lib and added the following to GenerateBsonDocuments

        var payloadString = payload.ToString();
        var matches = Regex.Matches(payloadString, "\\\"\\.([^\"]*)\\\":|\\\"([^\"]*)\\.\\\":|\\\"([^\"]*)\\.([^\"]*)\\\":");

        if (matches?.Any() ?? false)
        {
            var matchedValues = matches.Select(x => x.Value).Distinct().ToArray();
            foreach (var match in matchedValues)
            {
                payloadString = payloadString.Replace(match, match.Replace(".", "-"));
            }
        }

        var bson = BsonDocument.Parse(payloadString);

It tries to find any key with a "." in it in any location and replaces it with a hyphen. Its a dirty solution and probably not the most optimal, but it works.

IcanBENCHurCAT commented 5 years ago

Only note is that $ is another reserved character.

khushbudesai003 commented 5 years ago

@IcanBENCHurCAT Is this fix given for .Net Core 2.1 ?

IcanBENCHurCAT commented 5 years ago

@IcanBENCHurCAT Is this fix given for .Net Core 2.1 ?

@khushbudesai003 I'm not sure what you mean. The fix that I created is in one of the dev releases for this repo. I didn't like the direction that fix was going, so I abandoned my branch.

@DarkXoro If you haven't already, please fork the repository and create a pull request for this change, I'd like to see the repo owners put this into a dev release for testing.

DarkXoro commented 5 years ago

@IcanBENCHurCAT I have created a PR with the modified pattern that will fix both the period and the dollar sign

khushbudesai003 commented 5 years ago

Thanks a lot