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

logf should emulate printf format specifiers #6

Closed
3 of 7 tasks
jwosty opened this issue Jun 8, 2023 · 1 comment
Closed
3 of 7 tasks

logf should emulate printf format specifiers #6

jwosty opened this issue Jun 8, 2023 · 1 comment
Labels

Comments

@jwosty
Copy link
Owner

jwosty commented Jun 8, 2023

It would be very nice if logf were to fully honor printf-style format specifiers in the message template.

let consoleLogger = LoggerFactory.Create(fun builder -> builder.AddConsole() |> ignore).CreateLogger<obj>()

printfn "Duration: %.2fms" (5. / 3.)
// Duration: 1.67ms

logfi consoleLogger "Duration: %.2f{durationMs}ms" (5. / 3.)
// info: object[0]
//      Duration: 1.6666666666666667ms

We have to be careful to actually translate the format specifier into an equivalent .NET/ILogger format string, rather than just transforming the parameter itself, because it's not equivalent for some types of loggers. For example:

let plainConsoleLogger = (new Serilog.Extensions.Logging.SerilogLoggerFactory(LoggerConfiguration().WriteTo.Console().CreateLogger())).CreateLogger<obj>()
let jsonConsoleLogger = (new Serilog.Extensions.Logging.SerilogLoggerFactory(LoggerConfiguration().WriteTo.Console(formatter = JsonFormatter()).CreateLogger())).CreateLogger<obj>()

plainConsoleLogger.LogInformation("Duration: {durationMs:0.##}", (5. / 3.))
// [10:03:56 INF] Duration: 1.67

// This is what pre-formatting the argument inside logf and then passing it in would behave like
jsonConsoleLogger.LogInformation("Duration: {durationMs}", "1.67")
// {"Timestamp":"2023-06-08T10:09:27.6988000-06:00","Level":"Information","MessageTemplate":"Duration: {durationMs}","Properties":{"durationMs":"1.67","SourceContext":"object"}}

// This is what it *should* behave like. Notice that the output contains the full original object.
jsonConsoleLogger.LogInformation("Duration: {durationMs:0.##}", (5. / 3.))
// {"Timestamp":"2023-06-08T10:04:07.1231060-06:00","Level":"Information","MessageTemplate":"Duration: {durationMs:0.##}","Properties":{"durationMs":1.6666666666666667,"SourceContext":"object"},"Renderings":{"durationMs":[{"Format":"0.##","Rendering":"1.67"}]}}

Should probably create smaller-grained issues, since there are a lot of format specifiers of varying difficulty to implement.

Alternatives

An alternative approach could be to simply allow a user to specify a .NET-style format specifier themselves, by identifying when that is happening and just passing it through to the logger itself:

logfi ml "Duration: %f{durationMs:0.##}" (5. / 3.)

(This currently outputs [10:15:33 INF] Duration: 1.666667{durationMs:0.##})

This approach could even be implemented in conjunction with the more "correct" approach.

Advantages:

  • Very easy to implement
  • Allows full compatibility with existing ILogger.Log calls, for no case-by-case effort (i.e. perhaps there's something .NET-style format specifiers can do that F#-style format specifiers cannot)

Disadvantages:

  • User can't use their knowledge of F#-style format specifiers with this feature
  • Might have to consider how this would interact with string interpolation support in the future

Workarounds

One quick workaround is to format it yourself outside of the logf call:

logfi ml "Duration: %s{durationMs}" (sprintf "%.2f" (5. / 3.))

Tasks

@jwosty
Copy link
Owner Author

jwosty commented Jul 8, 2023

Closing, as the basics were implemented in #20, and the rest are blocked by external issues.

Moving these last few format specifiers to #24.

@jwosty jwosty closed this as completed Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant