forked from tokio-rs/tracing
-
Notifications
You must be signed in to change notification settings - Fork 0
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
[tracing-subscriber]: add chrono crate implementations of FormatTime #1
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shayne-fletcher
force-pushed
the
chrono-subscriber
branch
from
August 18, 2023 19:45
f70dbde
to
db86c5a
Compare
…(local time & UTC)
shayne-fletcher
force-pushed
the
chrono-subscriber
branch
2 times, most recently
from
August 21, 2023 20:28
dbb35e0
to
5576e08
Compare
shayne-fletcher
force-pushed
the
chrono-subscriber
branch
from
August 21, 2023 21:10
5576e08
to
f4817a5
Compare
This branch fixes a handful of new warnings which have shown up after updating to Rust 1.72.0. This includes: * `clippy::redundant_closure_call` in macros --- allowed because the macro sometimes calls a function that isn't a closure, and the closure is just used in the case where it's not a function. * Unnecessary uses of `#` in raw string literals that don't contain `"` characters. * Dead code warnings with specific feature flag combinations in `tracing-subscriber`. In addition, I've fixed a broken RustDoc link that was making the Netlify build sad.
## Motivation The instrument macro currently doesn't work with the "log" crate feature: tokio-rs#2585 ## Solution Change the generated code to create a span if either `tracing::if_log_enabled!` or `tracing::level_enabled!`. I'm not sure how to add a test for this or if this is the best solution. Fixes tokio-rs#2585
## Motivation There are a few errors in the file appender docs - this fixes them. It also wasn't clear/apparent to me that you can create a non-rolling file appender with the `rolling` module - this calls that out more clearly. ## Solution Updates to docs.
## Motivation The `.folded` format expects a `;`-separated list of the stack function, optionally followed up by a sample count. The implementation before this commit added a blank space after each `;` which made parsers, such as `inferno-flamegraph` mis-interpret the data. Furthermore, normally one wouldn't expect the filename and line-number in such stack trace. ## Solution Remove white-space between `;` for the generated file and remove filename and line-number by default.
…okio-rs#2709) ## Motivation `#[tracing::instrument]` uses `unreachable!()` macro which needlessly expands to panicking and formatting code. It only needs any `!` type. ## Solution `loop {}` works just as well for a `!` type, and it crates less work for the compiler. The code is truly unreachable, so the message would never be useful. Rust used to be concerned about semantics of empty loops in LLVM, but this [has been solved](https://reviews.llvm.org/D85393).
## Motivation Fixes: tokio-rs#1566 ## Solution This change removes the maximum of 32 fields limitation using const generics. Having this arbitrary restriction in place to prevent stack overflows feels a little misplaced to me since stack size varies between environments.
## Motivation The motivation is tokio-rs#1426. Currently, event names are set to a default value of `event file:line`, which (1) doesn't allow for customization, and (2) prevents events from working for some opentelemetry endpoints (they often use the event name `exception` to designate something like a panic). ## Solution Went through the event macros, and added new parameterization that allows for setting the `name`. In addition, added some comments, and reordering, to make life a bit better for the next person that comes along to edit those macros. Finally, added tests for the macro expansion alongside the existing tests with a reasonable amount of coverage (though, admittedly, more could be added for all of the macro invocation types Fixes tokio-rs#1426
## Motivation It's currently not possible to customize how messages will get send to journald. This became apparent in tokio-rs#2425, where first a specific API got designed, but then it was decided that users should not get restricted in only a subset of fields, but should be able to simply choose by themselves what fields get set with what values. So in a sense, this is the successor/rework of tokio-rs#2425. ## Solution Allow custom fields to be set in tracing-journald. ## Open Questions - [x] How should we deal with fields that also get supplied by other options? For example, setting `SYSLOG_IDENTIFIER` here and also setting `.with_syslog_identifier()` will send said field twice, potentially with differing values. Is that a problem? - Answer: No, this is not a problem. Closes tokio-rs#2425
## Motivation I've found myself in the case where I wanted to have customized event field name for different trait implementations. In fact, these implementations are completely unrelated (in separate applications), so, in this use case, I find more readable to have `foo="some_id"` and `bar=16` instead of `resource="foo" value="some_id"` and `resource=bar value=16` Because events only accept identifier or literal as field name, this is quite cumbersome/impossible to do. A simple solution could be to make events accept constant expression too; in my use case, I could then add a associated constant to my trait. ## Solution This PR proposes a new syntax for using constant field names: ```rust tracing::debug!({ CONSTANT_EXPR } = "foo"); ``` This is the same syntax than constant expression, so it should be quite intuitive. To avoid constant expression names conflict, internal variables of macro expansion have been prefixed with `__`, e.g. `__CALLSITE`. Co-authored-by: Joseph Perez <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
## Motivation Adding a relevant library to the list of `tracing`-enabled crates. ## Solution Added to READMEs and documentation.
davidbarsky
force-pushed
the
chrono-subscriber
branch
from
September 21, 2023 16:13
a42b9de
to
ae007f9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Issue tokio-rs#2080 explains that it's not possible to soundly use
tracing_subscriber::fmt::time::LocalTime
in a multithreaded context. It proposes adding alternative time formatters that use the chrono crate to workaround which is what this PR offers.Solution
A new source file 'chrono_crate.rs' is added to the 'tracing-subscriber' package implementing
mod chrono_crate
providing two new tag typesLocalTime
andUtc
with associatedtime::FormatTime
trait implementations that callchrono::Local::now().to_rfc3339()
andchrono::Utc::now().to_rfc3339()
respectively. Simple unit-tests of the new functionality accompany the additions.