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

backport recent changes from master #2324

Merged
merged 6 commits into from
Sep 30, 2022
Merged

backport recent changes from master #2324

merged 6 commits into from
Sep 30, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Sep 24, 2022

This backports the following PRs from master:

The error messages for these tests appear to have changed a bit with
Rust 1.64. This commit includes changes to the `.stderr` file after
running the tests with `TRYBUILD=overwrite`, in order to update the
expected output for the latest Rust.
The `async_fn` test file in `tracing-attributes` contains several
functions that exist just to test whether they _compile_, rather than
make assertions about their behavior. Because these functions are never
called, they (naturally) emit dead code warnings.

This commit adds `#[allow(dead_code)]` to the compilation tests, plus a
comment explaining why we do this.
@hawkw hawkw requested a review from a team as a code owner September 24, 2022 21:34
@hawkw hawkw force-pushed the eliza/backport-2323 branch from 341d88f to 7d5c10a Compare September 24, 2022 23:07
@guswynn
Copy link
Contributor

guswynn commented Sep 29, 2022

@hawkw I can backport #2321 if you dont want to!

@hawkw
Copy link
Member Author

hawkw commented Sep 29, 2022

@guswynn I can take care of it, thanks!

jswrenn and others added 4 commits September 30, 2022 11:59
Allows `Subscriber`s and `Layer`s to stash their own `Dispatch` without
causing a memory leak.

## Motivation

Resolves a shortcoming of #2269: that it's impossible for `Subscriber`s
or `Layer`s to stash a copy of their own `Dispatch` without creating a
reference cycle that would prevent them from being dropped.

## Solution

Introduces `WeakDispatch` (analogous to `std::sync::Weak`) that holds a
weak pointer to a `Subscriber`. `WeakDispatch` can be created via
`Dispatch::downgrade`, and can be transformed into a `Dispatch` via
`WeakDispatch::upgrade`.

Co-authored-by: Eliza Weisman <[email protected]>
## Motivation

`tracing-appender` does not have `Rotation` based on size yet. Also, it
doesn't have the feature of keeping the most recent `N` log files

I believe the second feature is more easy to implement, and also will
partially solve the `Rotation` based on size problem. Because people may
choose `hourly` or `daily` rotation based on their needs, and put an
extra boundary of `keep the last 5 files` for example. Of course it
won't handle all the edge cases for `Rotation` based on size. But it
will cover most of the scenarios. And also, it is a good feature to have
on its own :)

## Solution

Introduce another field called `max_files: Option<usize>` to the `Inner`
of `RollingFileAppender` struct. I managed to did not touch any of the
existing functions, so it **WON'T BE A BREAKING CHANGE**. Yay :)

The solution is, whenever the rotation should happen, the
`refresh_writer()` is called. So I embed the following logic into that
function:

1- check the log folder and detect the log files 2- if there are more
log files than the `max_files` amount 3- store the filenames in a
vector, and sort them by their dates (dates are already present in the
filename) 4- keep deleting the oldest ones, till we have desired amount
of log files in the log folder

P.S. this PR was opened before, but got closed since it would be easier
for the maintainers to target `master` branch instead of `v0.1.x` Also,
@CBenoit contributed to this PR, it would be great to give credit to him
:)

Co-authored-by: Benoît Cortier <[email protected]>
Co-authored-by: Eliza Weisman <[email protected]>
## Motivation

Currently, when using the `Layer` impl for `Option<S: Layer<...>>`, the
`Layer::max_level_hint` returns  `Some(LevelFilter::OFF)`. This was
intended to allow totally disabling output in the case where a
`Subscriber` is composed entirely of `None` `Layer`s. However, when
other `Layer`s *do* exist but return `None` from their `max_level_hint`
implementations to indicate that they don't care what the max level is,
the presence of a single `None` layer will globally disable everything,
which is not the wanted behavior.

Fixes #2265

## Solution

This branch introduces a special downcast marker that can be used to
detect when a `Layer` in a `Layered` is `None`. This allows the
`pick_level_hint` method to short-circuit when a `Layer` implementation
which is `None` returns `Some(LevelFilter::OFF)` from its
`max_level_hint` if the other half of the `Layered` is not `None`. The
tests I added should be pretty thorough!

Additionally, the downcast marker is special-cased in the `reload`
`Layer`. Normally, this `Layer` doesn't support downcasting, but it can
in the case of the special marker value.

Co-authored-by: Eliza Weisman <[email protected]>
#2326)

## Motivation

This fixes a discrepancy in the `tracing-opentelemetry` docs for the new
MetricsLayer. The docs refer to `value.` as a way to collect discrete
data point, but this doesn't match any of the prefix constant mentioned
in the same file.

```rust
const METRIC_PREFIX_MONOTONIC_COUNTER: &str = "monotonic_counter.";
const METRIC_PREFIX_COUNTER: &str = "counter.";
const METRIC_PREFIX_HISTOGRAM: &str = "histogram.";
```

## Solution

This fixes the documentation and test by referring to `histogram.`
instead of `value.`.
@hawkw hawkw force-pushed the eliza/backport-2323 branch from 7d5c10a to 257f86f Compare September 30, 2022 19:15
@hawkw hawkw requested a review from jtescher as a code owner September 30, 2022 19:15
@hawkw hawkw merged commit 92e2454 into v0.1.x Sep 30, 2022
@hawkw hawkw deleted the eliza/backport-2323 branch September 30, 2022 20:22
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.

6 participants