Add context action to move string interpolation variables to params

[taori]

It would be useful if this option existed, because in the past i have gotten pretty used to writing interpolated strings and only recently learned about structured logging.

[olsh]

Hi @taori,

Sounds like a great feature. Let me clarify some questions. Before fix:

_log.Information($"User {_user} with {_context.Permissions}");

After fix:

_log.Information("User {User} with {ContextPermissions}", _user, _context.Permissions);

Am I correct?

If yes, then we need to figure out what to do with more complicated cases. Here are a couple examples:

_log.Information($"SomeData {new int[] { 1, 2, 3 }}");

_log.Information($"Some int {0}");

_log.Information($"Method result {PrintGeneric(user)} {PrintGeneric(data)}");

Please, let me know what you think.

[greuelpirat]

@olsh this would be a great help!

I think the logic should be as simple as possible, to keep it understandable

Unnamed arguments should be just named Argument. May be add an additional generic warning for the magic name Argument

_log.Information($"SomeData {new int[] { 1, 2, 3 }}");

_log.Information($"SomeData {Argument}", new int[] { 1, 2, 3 });

Do not check for constants, add sequence number for duplicates

_log.Information($"Some int {0}, {1}, {5}");

_log.Information($"SomeData {Argument} {Argument1}, {Argument2}", 0, 1, 5);

Build names using only characters from expression:

_log.Information($"Method result {PrintGeneric(user)} {PrintGeneric(data)}");

_log.Information($"Method result {PrintGenericUser} {PrintGenericData}", PrintGeneric(user), PrintGeneric(data));

What do you think?

[olsh]

Hi @greuelpirat

Thanks for the suggestion! 👍🏻

Unnamed arguments should be just named Argument.

Well, I think this is pretty dangerous if you are using Elasticsearch sink, let's take a look at this code

_log.Information($"Another log {"hello"}");
_log.Information($"Some log {42}");

The code above will work just fine.

After refactoring that you suggested, the code will look like

_log.Information("Another log {Argument}", "hello");
_log.Information("Some log {Argument}", 42);

And this is an issue for Elasticsearch https://github.com/serilog/serilog-sinks-elasticsearch#a-note-about-fields-inside-elasticsearch

Be aware that there is an explicit and implicit mapping of types inside an Elasticsearch index. A value called X as a string will be indexed as being a string. Sending the same X as an integer in a next log message will not work.

So, I'm not sure that generating constant names is a good solution.

[vilinski]

Hi, great suggestion for the string interpolation fix! The workaround for Elasticsearch would be letting the user to decide how to name the arguments during refactoring. Like it will be done with "introduce variable" refactoring. Maybe the mechanism could be reused here? Resharper is very smart in naming things. Otherwise, I think it's a problem of Elasticsearch. We use it too, but I have learned about this problem first from your comment :)

[pfeigl]

I'd go the following route. Makes it easy for the default use-cases and pretty user-friendly and flexible for the not so easy cases:

• Put structured loggings names based on variable names in PascalCase where applicable
• Ask the user for the variable name using something like this in more complex situations (very minimalistic mockup from the rename variable dialog)

Optionally, the dialog could be prefilled with something meaningful where possible (i.e. the PrintGeneric(user) case)

[olsh]

The feature was implemented in #65 and will be available in the next extension version soon. Thanks to @matkoch 🔝

[olsh]

Closing this out due to inactivity. Feel free to re-open the issue.