Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… rename the logger file to rlog
…ore tests to verify other features. Also implemented the AddSource and ReplaceAttr features.
|
@cursor bugbot run |
This comment was marked as resolved.
This comment was marked as resolved.
chris-okuda
left a comment
chris-okuda
left a comment
There was a problem hiding this comment.
comments for @bbengfort on how I modified things
rlog/console/cmd/main.go
Outdated
| // Command console demonstrates [console.Handler] and every [console.Options] field. | ||
| package main |
There was a problem hiding this comment.
This is a demo; I wanted to see the output more easily for verification that it looked right
… way to merge rlog custom levels into a console.Options and optimised that merge
…mentation - Introduced `benchmark_test.go` to compare performance between `rlog.Logger` and `slog.Logger` with various logging scenarios. - Updated `README.md` to clarify module features, including new sections for `rlog.Logger`, subpackages, and usage examples. - Enhanced `rlog.go` with improved source attribution in logs. - Added tests to ensure correct source attribution in log records.
chris-okuda
left a comment
chris-okuda
left a comment
There was a problem hiding this comment.
notes about the fixes for the pc value to make the AddSource work properly
| var pcs [1]uintptr | ||
| runtime.Callers(2, pcs[:]) | ||
| l.emit(ctx, level, msg, pcs[0], args...) |
There was a problem hiding this comment.
These all just do a similar thing to how it's implemented in the slog library: https://cs.opensource.google/go/go/+/master:src/log/slog/logger.go;l=91
Only thing is, we can't access the underlying slog.Logger's options to see if AddSource was enabled or not, so we do this every time. We could add an option to disable this, but the benchmark shows it's not a performance impact.
There was a problem hiding this comment.
And this shouldn't be used for performance code anyway -- just development.
…t be set except when the user sets it
There was a problem hiding this comment.
Ah - I was actually planning on doing this and had forgotten - thank you!
| const timeFormat = "[15:04:05.000]" | ||
|
|
||
| // Handler writes a text prefix ([basename:line] when AddSource, bracketed time, level, message) and optional JSON attributes. | ||
| type Handler struct { |
There was a problem hiding this comment.
Appreciate you doing the full implementation of the Handler here. I was being lazy; I looked at the slog implementation, and both the TextHandler and the JSONHandler use the same commonLogger and I didn't know if I wanted to deal with that or not -- hence the embedding.
There was a problem hiding this comment.
Yea, nearly at the beginning I noticed that you were sending the embedded TextHandler the With and WithGroup calls, which basically loses those attributes to the ether since we can't access the internals of that handler, so I had to add those functions, and the topAttrs and segments internals, and once I did that the only thing left that the embedded handler was doing was the Enabled function, so I just dropped it.
bbengfort
left a comment
bbengfort
left a comment
There was a problem hiding this comment.
Your implementation was far better than mine! Thank you for adding it!
2 participants
Scope of changes
Clearly I'm procrastinating …
So I implemented simplified and colorized logs for slog.
@chris-okuda says: I am bored, so I rewrote it all to pass the
slogtest.TestHandlertests and also added a few features.Screenshot of examples (
go run ./rlog/console/cmd/):Type of change
Author checklist
Reviewer(s) checklist
Note
Low Risk
Low risk: additive logging handler code only, with no changes to core log level semantics or data paths; main concerns are formatting/ANSI compatibility and limited test coverage.
Overview
Adds a new
rlog/consolepackage providing a development-focusedslog.Handlerthat prints timestamp/level/message with ANSI colors and (optionally) a JSON-encoded attribute map.Introduces
console.Optionsto toggle coloring, JSON emission, and JSON indentation, and ensures concurrent-safe writes by buffering each record then writing under a shared mutex across handler clones (WithAttrs/WithGroup).Written by Cursor Bugbot for commit f8d173b. Configure here.