I'm inclined not to merge the pull request as is, because it would make the MDC behavior different than everything else. All of the other keys are added to the document only if they are non-null. This will be desirable for some people and undesirable for others. But, it's the behavior that was originally established and has been subsequently used. For the most part, this has little impact on a user, as a missing key is treated like a key with a null value. One exception is if you check whether the key exists.
In addition, addMDCInformation does a toString() on everything in the map. This is useful, because the keys in the BSON document need to be strings. There are a few cases where BSON encoding can cause issues (e.g., dynamic proxies), so this saves the user from having to ensure in advance that all keys can be converted to Strings properly. If the user is logging to Mongo from code they don't control, they might not have the ability to do this themselves.
However, the current code that forces the value to be a String has a significant downside. For example, I may want to put a BSON Date object into the logged document so that I can later manipulate it as a date more easily. Or I may want some other BSON type other than a String. For example, if I set an array of ints and a Date as the value for two MDC keys, the log document will contain:
That is enough of an advantage that I think it is preferable to the risk of someone accidentally passing in a weird object for the value that the Mongo Java driver BSON encoder can't handle.
So, I would approve a pull request that sticks with the original behavior, but removes the toString on the value.
I'm inclined not to merge the pull request as is, because it would make the MDC behavior different than everything else. All of the other keys are added to the document only if they are non-null. This will be desirable for some people and undesirable for others. But, it's the behavior that was originally established and has been subsequently used. For the most part, this has little impact on a user, as a missing key is treated like a key with a null value. One exception is if you check whether the key exists.
In addition, addMDCInformation does a toString() on everything in the map. This is useful, because the keys in the BSON document need to be strings. There are a few cases where BSON encoding can cause issues (e.g., dynamic proxies), so this saves the user from having to ensure in advance that all keys can be converted to Strings properly. If the user is logging to Mongo from code they don't control, they might not have the ability to do this themselves.
However, the current code that forces the value to be a String has a significant downside. For example, I may want to put a BSON Date object into the logged document so that I can later manipulate it as a date more easily. Or I may want some other BSON type other than a String. For example, if I set an array of ints and a Date as the value for two MDC keys, the log document will contain:
"properties" : { "array" : "[I@38717323", "now" : "Fri Mar 30 12:13:52 PDT 2012" }
but if the toString() is removed I get the much nicer result:
"properties" : { "array" : [ 1, 2, 3 ], "now" : ISODate("2012-03-30T19:12:18.446Z") }
That is enough of an advantage that I think it is preferable to the risk of someone accidentally passing in a weird object for the value that the Mongo Java driver BSON encoder can't handle.
So, I would approve a pull request that sticks with the original behavior, but removes the toString on the value.