Skip to content
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
wants to merge 20 commits into from

Conversation

shayne-fletcher
Copy link
Owner

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 types LocalTime and Utc with associated time::FormatTime trait implementations that call chrono::Local::now().to_rfc3339() and chrono::Utc::now().to_rfc3339() respectively. Simple unit-tests of the new functionality accompany the additions.

@shayne-fletcher shayne-fletcher force-pushed the chrono-subscriber branch 2 times, most recently from dbb35e0 to 5576e08 Compare August 21, 2023 20:28
shayne-fletcher and others added 15 commits August 23, 2023 12:01
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.