-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
Add support for Key-Value pairs #362
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Lets try and get some more tests in there and the documentation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #362 +/- ##
==========================================
+ Coverage 63.39% 64.09% +0.70%
==========================================
Files 24 24
Lines 1557 1593 +36
==========================================
+ Hits 987 1021 +34
- Misses 570 572 +2 ☔ View full report in Codecov by Sentry. |
207d3be
to
230f2ea
Compare
Hi, sorry for the delay, I'm still keen to get this merged. I have rebased this PR and squashed the commits. If more documentation is needed I can get that in ASAP as well. |
I'll take a look at this soon. I'll do my best to get a hold of the project owner and work through our recent batches of PRs |
What's the status of this PR? |
@sdroege I am targeting next week to start working with the owner of the repo. Appreciate everyone's hard work on the recent batch of MRs and can't wait to get things moving again! |
That sounds great, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple of comments, but looking really good.
One more general question, did you attempt to wrap log::kv::Source
in a newtype and impl serde::Serialize on it?
Just wondering because I was thinking about the tradeoffs of allocating a whole BTree vs the annoyance of adding lifetimes to Message
.
…encoding. Factor pattern parsing code between MDC and key_value targets
Thanks for the review @estk, I have followed your suggestion and investigated directly playing around with Serde traits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nailed it
blocked on #367 |
Hi! What is the current status of the MR? |
@SL-RU we're still waiting on the blocked PR |
@@ -1,7 +1,10 @@ | |||
[package] | |||
name = "log4rs" | |||
version = "1.3.0" | |||
authors = ["Steven Fackler <[email protected]>", "Evan Simmons <[email protected]>"] | |||
authors = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to revert changes in the formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes have been approved by the maintainers, I'm reluctant to push to the PR at this stage for that reason
Hi! What is the current status of the MR? |
Hey! Sorry but all PRs are blocked by the PR fixing the deprecated API. I've been too busy of late to bug @estk and get it moving again. Please feel free to take a look at that PR and suggest options you might see in helping bring it to closure. |
Hi there,
As discussed in #329, this PR implements support for the
log::kv
structured logging API.These are some user-facing choices I have made:
log_kv
featurejson
andpattern
:json
message has anattributes
field which is a serialized map of the record's structured logspattern
encoder has a newK
orkey_value
formatter which functions exactly like themdc
formatter.If this sounds okay, I can also edit the documentation.