tracing
tracing copied to clipboard
Allow string literal field identifiers
Fixes #2438
Motivation
Currently, you can only use Idents
delimited by .
for fields.
This prevents us creating an empty field containing rust keywords such
as type
. (e.g. graphql.operation.type
)
The span!
and event!
macros allow the usage of string literals to circumvent
this limitation.
tracing::event!(Level::TRACE, "field.type" = "test", "event name");
Solution
To allow for a similar API as with the macros, this PR replaces the Field.name
with an enum FieldName
.
The enum can be the current Punctuated
type or a LitStr
.
Possible other solution
Another option would be to unraw the identifiers when emitting the field name. But this might cause confusion when an emitted field name does not match a struct field name in code.
I am open to providing a PR targeting the master branch as well.
hey @valkum thanks for the code 🍻 I'm really interested in getting this into the crate, because it solves the issue of key names with type
. Do you still want to work on it, otherwise I would create a new PR with your changes.
@hawkw as one of the tokio members, how do you feel about the code changes in the general?
I can see if I can rebase the current PR in the coming days. Do you want two PRs one for 0.1.x and one for master?
@valkum @hawkw Any progress on this one?
Hey sorry. Nothing new. I hope I can make some progress tomorrow or next Thursday. I set myself a deadline for next weekend.
@valkum all good 🙂 let me know, otherwise I will create a new PR branching off yours 💪
I'm not entirely sure, but is this the same #2924 and #2925? This seems to be similar especially to the latter but the former seems to fix the same thing in a simpler way (unless there is a hidden issue with that approach).
Arguably this has been the first PR, but I'd just like to bring it to attention here.
I'm not entirely sure, but is this the same #2924 and #2925? This seems to be similar especially to the latter but the former seems to fix the same thing in a simpler way (unless there is a hidden issue with that approach).
Arguably this has been the first PR, but I'd just like to bring it to attention here.
It fixes the same problem but uses peek
instead of ahead
which makes the implementation slightly less complex. I think my PR provides a bit more DX with the custom error message. Not sure if there is a drawback of using ahead
instead of peek
.
@hds Mind taking another look?