Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider re-adding parameterized logging methods #835

Closed
Piedone opened this issue Apr 23, 2024 · 3 comments
Closed

Consider re-adding parameterized logging methods #835

Piedone opened this issue Apr 23, 2024 · 3 comments

Comments

@Piedone
Copy link

Piedone commented Apr 23, 2024

E.g. ILogManager.Info(string message, params object[] args) is deprecated and it's recommended to use Info(string message) with string interpolation instead.

However, generally in .NET, it's recommended to follow structured logging patterns and utilize logging scopes. While Atata doesn't provide such logging out of the box, ILogManager makes it possible to substitute the logging implementation with one that does. That way, one can send structured log entries to other log engines, or even remote telemetry services like Azure Application Insights. There, structured logging matters a lot for filtering.

So, please re-add the removed overloads accepting parameters.

@Piedone
Copy link
Author

Piedone commented Apr 23, 2024

Hmm, structured logging needs named parameters though, not numbered ones. So maybe, support needs to be added for that.

@YevgeniyShunevych
Copy link
Member

Right, I decided to remove log methods with (string message, params object[] args) parameters. These methods were added to Atata before interpolated strings feature was introduced to C#. Under the hood the regular string.Format method did the formatting. Basically, Log.Info("Test {0}", id) was a shorthand to Log.Info(string.Format("Test {0}", id)). But nowadays string.Format is rarely used in favor of interpolated strings, and the previous example can be easily rewritten to Log.Info($"Test {id}"), which looks and reads better.

Also, nowadays, as you mentioned, structured logging became very popular. And it is not supported by Atata at the moment. But having old method signatures like Info(string message, params object[] args) can confuse people assuming that message supports structured logging. That was another reason to remove args parameters.

I also would like to have a support of structured logging in Atata, as well as an integration with Microsoft.Extensions.Logging. But it requires a lot of effort to rework existing functionality and add support of the new. On the other hand, I don't see a huge benefit from structured logging in terms of Atata, like in ASP.NET application, where you may have hundreds of thousands of log entries. Using Atata in testing you usually have a separate log file (or console output) per test, which is just a couple of screens high.

Also, ILogManager is not expected to be substituted, and there is no API for that. Custom log consumers - is a way for custom logging.

I hope that args parameters are not so critical for you. If it is, let's maybe try to think about some alternative.

@Piedone
Copy link
Author

Piedone commented Apr 29, 2024

I see, thanks for explaining, this makes sense. We don't need these methods, then.

@Piedone Piedone closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants