[op]

This adds support for slog attributes and fixes #119.

[codecov]

Codecov Report

Attention: Patch coverage is 85.71429% with 6 lines in your changes missing coverage. Please review.

Project coverage is 82.63%. Comparing base (2338a13) to head (6594782).
Report is 23 commits behind head on main.

Files	Patch %	Lines
json.go	85.36%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #127      +/-   ##
==========================================
+ Coverage   80.92%   82.63%   +1.71%     
==========================================
  Files          11       11              
  Lines         739      714      -25     
==========================================
- Hits          598      590       -8     
+ Misses        126      108      -18     
- Partials       15       16       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[aymanbagabas]

This adds support for slog attributes and fixes #119.

TODO

Current PR assume we're on Go 1.21. Open question:

1. Upgrade to require Go 1.21. This would allow merging of logger.go and logger_121.go and simplify existing code base.
2. Split all slog stuff out into json_go121.go and further complicate code base.

@aymanbagabas what do you think?

I would say keep supporting Go 1.19 and split code. We can use type aliases to simplify things. For example, slog.Attr can be defined as type slogAttr = slog.Attr for both Go 1.19 and Go 1.21. The same goes for the other imported slog types. This way we don't need to import slog in json.go.

[op]

@aymanbagabas very cool idea! that worked pretty nice.

[aymanbagabas]

Some nit picks and questions, looks good otherwise!

[op]

@aymanbagabas hi, did you see that all comments has been addressed? :)

[aymanbagabas]

Hey @op! Thanks for addressing the comments. I tried running the example from #119, but I think the implementation still doesn't support arbitrary maps, which is the whole point of the issue.

[op]

Hey @op! Thanks for addressing the comments. I tried running the example from #119, but I think the implementation still doesn't support arbitrary maps, which is the whole point of the issue.

No worries. Good catch @aymanbagabas! Didn't realise the slog Handle function worked differently. It was previously calling attr.Value.String(). I dropped the .String() call to allow the unmodified slog.Value flow through all formatters. For the JSON formatter, I've added a check to make sure it's output is identical in "slog mode" too now.

Fixing the text handler seem like a future improvement to me. Sorry, I don't have time to look at that.

[op]

@aymanbagabas ping! 🐧

[aymanbagabas]

LGTM, some last comments before merging

[op]

@aymanbagabas seems like govulncheck doesn't work with 1.19. Looks like golang/vuln@745db65 introduces slices.

[aymanbagabas]

@aymanbagabas seems like govulncheck doesn't work with 1.19. Looks like golang/vuln@745db65 introduces slices.

Hmm, seems like golang/vuln needs to update their go.mod

Anyway, this looks good, thanks again @op!