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

appender: add a builder for constructing RollingFileAppenders #2227

Merged
merged 6 commits into from
Jul 18, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jul 18, 2022

Motivation

Several currently open PRs, such as #2225 and #2221, add new
configuration parameters to the rolling file appender in
tracing-appender. The best way to add new optional configuration
settings without breaking existing APIs or creating a very large number
of new constructors is to add a builder interface.

Solution

Since a number of PRs would all need to add the builder API, introducing
potential conflicts, this branch just adds the builder interface
without adding any new configuration options. Once this merges, the
existing in-flight PRs can be rebased onto this branch to use the
builder interface without conflicting with each other.

Also, the Builder::build method is fallible and returns a Result,
rather than panicking. This is a relatively common pattern in Rust ---
for example, std::thread::Builder::spawn returns a Result if a new
thread cannot be spawned, while std::thread::spawn simply panics. This
allows users to handle appender initialization errors gracefully without
breaking the API of the existing new constructor.

Fixes #1953

Several currently open PRs, such as #2225 and #2221, add new
configuration parameters to the rolling file appender in
`tracing-appender`. The best way to add new optional configuration
settings without breaking existing APIs or creating a very large number
of new constructors is to add a builder interface.

Since a number of PRs would all need to add the builder API, introducing
potential conflicts, this branch _just_ adds the builder interface
without adding any new configuration options. Once this merges, the
existing in-flight PRs can be rebased onto this branch to use the
builder interface without conflicting with each other.

Also, the `Builder::build` method is fallible and returns a `Result`,
rather than panicking. This is a relatively common pattern in Rust ---
for example, `std::thread::Builder::spawn` returns a `Result` if a new
thread cannot be spawned, while `std::thread::spawn` simply panics. This
allows users to handle appender initialization errors gracefully without
breaking the API of the existing `new` constructor.

Fixes #1953
@hawkw hawkw requested a review from a team as a code owner July 18, 2022 17:27
@hawkw hawkw self-assigned this Jul 18, 2022
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw enabled auto-merge (squash) July 18, 2022 17:41
hawkw added 3 commits July 18, 2022 11:02
lol. lmao.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw merged commit ab36c17 into master Jul 18, 2022
@hawkw hawkw deleted the eliza/appender-builder branch July 18, 2022 19:21
hawkw added a commit that referenced this pull request Jul 20, 2022
## Motivation

Several currently open PRs, such as #2225 and #2221, add new
configuration parameters to the rolling file appender in
`tracing-appender`. The best way to add new optional configuration
settings without breaking existing APIs or creating a very large number
of new constructors is to add a builder interface.

## Solution

Since a number of PRs would all need to add the builder API, introducing
potential conflicts, this branch _just_ adds the builder interface
without adding any new configuration options. Once this merges, the
existing in-flight PRs can be rebased onto this branch to use the
builder interface without conflicting with each other.

Also, the `Builder::build` method is fallible and returns a `Result`,
rather than panicking. This is a relatively common pattern in Rust ---
for example, `std::thread::Builder::spawn` returns a `Result` if a new
thread cannot be spawned, while `std::thread::spawn` simply panics. This
allows users to handle appender initialization errors gracefully without
breaking the API of the existing `new` constructor.

Fixes #1953

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jul 20, 2022
## Motivation

Several currently open PRs, such as #2225 and #2221, add new
configuration parameters to the rolling file appender in
`tracing-appender`. The best way to add new optional configuration
settings without breaking existing APIs or creating a very large number
of new constructors is to add a builder interface.

## Solution

Since a number of PRs would all need to add the builder API, introducing
potential conflicts, this branch _just_ adds the builder interface
without adding any new configuration options. Once this merges, the
existing in-flight PRs can be rebased onto this branch to use the
builder interface without conflicting with each other.

Also, the `Builder::build` method is fallible and returns a `Result`,
rather than panicking. This is a relatively common pattern in Rust ---
for example, `std::thread::Builder::spawn` returns a `Result` if a new
thread cannot be spawned, while `std::thread::spawn` simply panics. This
allows users to handle appender initialization errors gracefully without
breaking the API of the existing `new` constructor.

Fixes #1953

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jul 20, 2022
## Motivation

Several currently open PRs, such as #2225 and #2221, add new
configuration parameters to the rolling file appender in
`tracing-appender`. The best way to add new optional configuration
settings without breaking existing APIs or creating a very large number
of new constructors is to add a builder interface.

## Solution

Since a number of PRs would all need to add the builder API, introducing
potential conflicts, this branch _just_ adds the builder interface
without adding any new configuration options. Once this merges, the
existing in-flight PRs can be rebased onto this branch to use the
builder interface without conflicting with each other.

Also, the `Builder::build` method is fallible and returns a `Result`,
rather than panicking. This is a relatively common pattern in Rust ---
for example, `std::thread::Builder::spawn` returns a `Result` if a new
thread cannot be spawned, while `std::thread::spawn` simply panics. This
allows users to handle appender initialization errors gracefully without
breaking the API of the existing `new` constructor.

Fixes #1953

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw pushed a commit that referenced this pull request Nov 13, 2023
# 0.2.3 (November 13, 2023)

This release contains several new features. It also increases the
minimum supported Rust version (MSRV) to Rust 1.63.0.

## Added

- **rolling**: add option to automatically delete old log files (#2323)
- **non_blocking**: allow worker thread name to be configured (#2365)
- **rolling**: add a builder for constructing `RollingFileAppender`s
  (#2227)
- **rolling**: add `Builder::filename_suffix` parameter (#2225)
- **non_blocking**: remove `Sync` bound from writer for `NonBlocking`
  (#2607)
- **non_blocking**: name spawned threads (#2219)

## Fixed

- Fixed several documentation typos and issues (#2689, #2375)

## Changed

- Increased minimum supported Rust version (MSRV) to 1.63.0+ (#2793)
- Updated minimum `tracing-subscriber` version to
  [0.3.18][subscriber-v0.3.18] (#2790)

[subscriber-v0.3.18]:
    https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.18
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request Feb 14, 2024
# 0.2.3 (November 13, 2023)

This release contains several new features. It also increases the
minimum supported Rust version (MSRV) to Rust 1.63.0.

## Added

- **rolling**: add option to automatically delete old log files (tokio-rs#2323)
- **non_blocking**: allow worker thread name to be configured (tokio-rs#2365)
- **rolling**: add a builder for constructing `RollingFileAppender`s
  (tokio-rs#2227)
- **rolling**: add `Builder::filename_suffix` parameter (tokio-rs#2225)
- **non_blocking**: remove `Sync` bound from writer for `NonBlocking`
  (tokio-rs#2607)
- **non_blocking**: name spawned threads (tokio-rs#2219)

## Fixed

- Fixed several documentation typos and issues (tokio-rs#2689, tokio-rs#2375)

## Changed

- Increased minimum supported Rust version (MSRV) to 1.63.0+ (tokio-rs#2793)
- Updated minimum `tracing-subscriber` version to
  [0.3.18][subscriber-v0.3.18] (tokio-rs#2790)

[subscriber-v0.3.18]:
    https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.18
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.2.3 (November 13, 2023)

This release contains several new features. It also increases the
minimum supported Rust version (MSRV) to Rust 1.63.0.

## Added

- **rolling**: add option to automatically delete old log files (tokio-rs#2323)
- **non_blocking**: allow worker thread name to be configured (tokio-rs#2365)
- **rolling**: add a builder for constructing `RollingFileAppender`s
  (tokio-rs#2227)
- **rolling**: add `Builder::filename_suffix` parameter (tokio-rs#2225)
- **non_blocking**: remove `Sync` bound from writer for `NonBlocking`
  (tokio-rs#2607)
- **non_blocking**: name spawned threads (tokio-rs#2219)

## Fixed

- Fixed several documentation typos and issues (tokio-rs#2689, tokio-rs#2375)

## Changed

- Increased minimum supported Rust version (MSRV) to 1.63.0+ (tokio-rs#2793)
- Updated minimum `tracing-subscriber` version to
  [0.3.18][subscriber-v0.3.18] (tokio-rs#2790)

[subscriber-v0.3.18]:
    https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.18
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.

RollingFileAppender::new() panics if the file can't be opened
2 participants