Nlog: Support C# 6 interpolated strings through IFormattable and/or FormattableString

[stylpe]

The new string interpolation syntax in C# 6 has a drawback when used with NLog: It performs the interpolation regardless of which log levels are enabled (since it has to be passed as a string to NLog; I haven't found any evidence to the contrary at least). It should be possible to avoid this if we can pass the interpolated string as IFormattable or FormattableString.

https://msdn.microsoft.com/en-us/library/dn961160.aspx describes how interpolated strings can be implicitly cast to IFormattable and FormattableString, which gave me the idea.

I haven't had a chance to dive deep into the details about performance, but I found this interesting discussion: https://github.com/dotnet/roslyn/issues/518

I leave it to you to judge if it's a good idea, but I couldn't find any mention of IFormattable or FormattableString here, and felt there should at least be an issue discussing it :)

[304NotModified]

Thanks!

We really want to support the interpolated strings for C#

I think we could make overloads to the logger methods for that?

[stylpe]

Possibly, but you should test overload resolution order (especially since this is mentioned under implicit type conversion) to make sure it works as intended

[304NotModified]

I think the compiler is smart enough, but of course we should unit test it :)

Can you make a prototype for this? (and send a PR?)

[304NotModified]

Useful stuff: http://www.thomaslevesque.com/2015/02/24/customizing-string-interpolation-in-c-6/

https://www.nuget.org/packages/StringInterpolationBridge/

[304NotModified]

I tried this, but it won't work as expected.

The Roslyn compiler takes the string overload, so Error($"{a}") is calling Error(string s) instead of Error(FormattableString fs)... (tested with debugging)

Maybe I should use the IFormattable interface

also IFormattable wont work...

[stylpe]

Looks like you're right: http://source.roslyn.codeplex.com/#Roslyn.Compilers.CSharp.Semantic.UnitTests/Semantics/InterpolationTests.cs,730

That's too bad. I don't see the reason behind this, and I can't determine if it was a design decision or just a consequence of standard overloading rules. I'm closing this, but if anyone discovers the root of this I'd be interested in hearing it.

[304NotModified]

https://github.com/dotnet/roslyn/issues/46

We generally believe that libraries will mostly be written with different API names for methods which do different things. Therefore overload resolution differences between FormattableString and String don't matter, so string might as well win. Therefore we should stick with the simple principle that an interpolated string is a string. End of story.

Assumptions are the Base of evil.

[304NotModified]

Reopen als I really searching for a work around

[304NotModified]

There's a work around, but it doesn't work well with an interface

http://pvlerick.github.io/2016/01/poking-the-csharp-compiler-overload-resolution-for-string-and-formattablestring/

[Dana-Ferguson]

I did some tests, and I saw that the call priority is string > FormattableString > IFormattable > object.
But if an interpolated string get captured as an object it will be converted to a string.

Would it be okay to drop strings types for object argument types? (as a quick sanity check, I verified that in the reference source, String.ToString() directly returns itself)

[304NotModified]

I don't think we would allow that as it is a breaking change in the most important part of NLog.

[godrose]

So there won't be string interpolation in NLog?

[304NotModified]

Well logger.Info($"{info}") and logger.Info($"{doExpensiveThing()}") does work, but it's not lazy like
logger.Info("{0}", info) and logger.Info("{0}",doExpensiveThing())

The problem is that the C# compiler will prefer Info(string) over Info(FormattableString). This is a "feature" of the C# compiler.

NB: this is lazy: logger.Info(() => $"{doExpensiveThing()}")

[robert-j-engdahl]

You could make this work though. You just need to trick the compiler into preferring the FormattableString overload. I've explained it in details here: http://blog.engdahls.dk/2016/08/how-to-overload-string-and.html

[304NotModified]

Cool!

Is this backwards-compatible and any idea if this will infects performance?

[robert-j-engdahl]

The resolution of an overload should be done at compile time. The conversion, however, will be done on-the-fly, so for normal strings we add the overhead of making an intermediate instance of StringIfNotFormattableStringAdapter (I have no clue if that is to expensive for you guys, it might be less expensive if you use a struct, though I havn't checked). The benefit is that you can remove the overhead you might had had with FormattableStrings being formatted prior to calling the log methods.

[robert-j-engdahl]

This is not backwards-compatible... you will break the binary API (overload resolution is compile time at the caller site). Code that worked before will continue to work after a recompile.

[304NotModified]

Thanks for the info.

I think we need binary backwards-compatible and also the performance is important.

Also I think formatable strings are nice, but don't play well with structural logging

[robert-j-engdahl]

binary backwards compatibility might be possible with a private string overload, but overloads with different accesslevels might be a can of worms.

Anyway, it is not really an issue, everone can just wrap your interface with their own supporting the FormattableString overload.

[jtheisen]

This is an important feature because it allows to identity two different log messages that differ only in their arguments. That's quite useful for a logging system, as you want to maybe throttle log messages per logger and template string, but not per arguments.

[304NotModified]

This is an important feature because it allows to identity two different log messages that differ only in their arguments. That's quite useful for a logging system, as you want to maybe throttle log messages per logger and template string, but not per arguments.

I think that's already possible with structured logging? Even with templates using {0} etc?

[jtheisen]

The point is it's not possible using template strings. They get converted to simple strings and the information is lost.

[snakefoot]

@jtheisen The ship has sailed for modifying ILogger (and Logger), but we might be able to create a new common interface that allows use of FormattableString. See also #2527 (With LoggerBase that has no format-helpers, so it is possible to inject whatever Logger-interface-implementation you like)

[304NotModified]

@jtheisen The ship has sailed for modifying ILogger (and Logger), but we might be able to create a new common interface that allows use of FormattableString. See also #2527 (With LoggerBase that has no format-helpers, so it is possible to inject whatever Logger-interface-implementation you like)

I'm curious how that will look, as two overloads with a string + formattablestring -> string overload ill be used.

[jtheisen]

@304NotModified And Strings don't automatically convert to FormattableStrings either, which I just realized. So nlog can't really do anything clever here as far as I can tell. I wonder what the rationale was for those semantics.

[robert-j-engdahl]

@304NotModified and @jtheisen Overloading string and FormattableString is done using the following trick: https://blog.engdahls.dk/2016/08/how-to-overload-string-and.html

The real question is if overloading is desirable at all.

Another approach would be ILogger extension methods where the FormattableString would have special names. Writing a literal formattable string is after all simpler and less error-prone than writing a format string and then a bunch of parameters.

[304NotModified]

Cool! But I think it's a breaking change, and also an performance thing (always a StringIfNotFormattableStringAdapter for string parameters)

[304NotModified]

There isn't a good way for now so closing this

[Drizin]

I've just published a library (with extensions to Serilog, NLog, and Microsoft ILogger) that allows writing log messages using interpolated strings and yet defining the property names through different syntaxes.
Would love to hear some feedback.
https://github.com/Drizin/InterpolatedLogging