serilog: Named parameters with not only alphanumeric symbols are ignored

Does this issue relate to a new feature or an existing bug?

Bug
New Feature

What version of Serilog is affected? Please list the related NuGet package.

2.8.0

What is the target framework and operating system? See target frameworks & net standard matrix.

netCore 2.0
netCore 1.0
4.7
4.6.x
4.5.x

Please describe the current behavior?

Log.Information("Error {Rpc.Error} occured", "boom");

renders to Error {Rpc.Error} occured

Please describe the expected behavior?

Error "boom" occured

Relates to #404

I discovered that this is due to IsValidInPropertyName method inside ParsePropertyToken:

for (var i = 0; i < propertyName.Length; ++i)
{
      var c = propertyName[i];
      if (!IsValidInPropertyName(c))   <--- ???
           return new TextToken(rawText, first);
}

I originally used LogContext.PushProperty("Rpc.Error", "boom") and everything worked fine. I am interested in the question - why, when parsing a message template, knowingly narrow the set of valid characters of property name if the neighboring api (Push) allows you to add properties with any characters? This asymmetry led to an error when I moved the property name inside the format string and began to pass the value of the property in the params.

I read #404, but I don’t understand the motivation. First, this is no structured data, just plain string.

Dots within property names themselves aren’t supported by many target sinks, so we chose the lowest common denominator here.

Second, I use Console and Kafka sink which both support dots in property names. However, this talk is not even about the dots. The template string does not work if you use any character other than an alphanumeric character or underscore - :, -, etc. What problem do you solve when you talk about lowest common denominator? And here I come back to the fact that Push allows you to pass an arbitrary property name (and that’s cool). Therefore, as I see it, all this concern for the lowest common denominator does not make sense.

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 1
Comments: 21 (17 by maintainers)

Most upvoted comments

@nblumhardt that helps indeed, thanks a lot!

I came to the same idea, but my implementation of it was rather cumbersome, I did the replacement in a class derived from Serilog.Formatting.Elasticsearch.ExceptionAsObjectJsonFormatter.

otto-gebb on Oct 8, 2020

or a structured property Rpc with field Error

Structuredeness of a property is determined not by its name, but is specified by service characters - @, $.

Would multiple Rpc.* properties that appear in one message be merged into a single structured value?

Again, structuredeness of a property is determined not by its name, but is specified by service characters - @, $.

I see that you are referring to a grammar in which Name does not allow dots and other syntactic characters. Whatever the specification is, it exists and it is much more difficult to change it than implementation, in this I agree with you.

Nevertheless, I see that there is IMessageTemplateParser interface. MessageTemplateCache gets it in ctor but in fact MessageTemplateCache is initialized using a specific parser implementation in MessageTemplateProcessor:

 readonly MessageTemplateCache _parser = new MessageTemplateCache(new MessageTemplateParser());

I suggest passing another parameter of the type IMessageTemplateParser into MessageTemplateProcessor constructor and create MessageTemplateCache with it. This will require making the IMessageTemplateParser public interface and moving out of Core namespace (by the way, which one?). MessageTemplate is already public so I don’t think there will be problems.

Next, MessageTemplateProcessor is created inside LoggerConfiguration where one can easily add a new configuration property to set the parser like LoggerSinkConfiguration WriteTo does. Thus, backward compatibility will not be violated, and those customers who need to use additional characters (dot, hyphen etc) in the message template will get this opportunity. They explicitly specify this in the provided configuration.

Will you accept such a PR?

sungam3r on Nov 13, 2019