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.

Sign up for GitHub

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

Jump to bottom

Make tracing_subscriber::fmt::format::Writer::new public #2223

Closed
TmLev opened this issue Jul 16, 2022 · 2 comments
Closed

Make tracing_subscriber::fmt::format::Writer::new public #2223

TmLev opened this issue Jul 16, 2022 · 2 comments

Comments

@TmLev
Copy link
Sponsor

TmLev commented Jul 16, 2022

Feature Request

Crates

https://github.com/tokio-rs/tracing
https://github.com/davidbarsky/tracing-tree

Motivation

I'm using tracing-tree, but it lacks timestamps. I opened an issue in tracing-tree about it and @davidbarsky suggested that FormatTime can be used as part of the implementation.

When trying to add support for it, I stumbled upon Writer which is needed for calling FormatTime::format_time. However, Writer has only one private constructor and cannot be instantiated outside the tracing-subscriber crate, which is quite limiting.

Proposal

Simple solution would be to make constructor entirely public, as suggested in TODO by @hawkw.

Alternatives

Another solution is to (maybe) add custom constructor that accepts String, for instance. This eliminates the need for making existing constructor public, but Writer::new(&'writer mut impl fmt::Write) seems to be more flexible.
@TmLev TmLev mentioned this issue Jul 16, 2022
Closed
@Fuuzetsu
Copy link

Fuuzetsu commented Oct 3, 2022

I also wanted this when investigating #2334 , ended up with bunch of copy-paste...

@pigeonhands pigeonhands mentioned this issue Oct 6, 2022
Open
@TmLev TmLev mentioned this issue Mar 30, 2023
Closed
@kayabaNerve kayabaNerve mentioned this issue Apr 15, 2023
Open
@kaifastromai
Copy link
Contributor

I am somewhat confused as to why this issue has not been moved forward at all? The change itself is trivial (code wise, I am not sure what the wider reaching effects may be/other plans that might conflict with such a change), but there is already a PR. Are the maintainers backing off?

This was referenced Aug 10, 2023
Merged
Merged
@hawkw hawkw closed this as completed in 99e0377 Oct 11, 2023
hawkw added a commit that referenced this issue Oct 12, 2023
@kaifastromai @cephas-storm-bfx @hawkw
subscriber: make format::Writer::new() public (#2680)
71b5b2c 
## Motivation

As seen here #2512 and #2223. Previously pr'ed here #2525, but no progress has
been made there for quite some. I have applied the suggested changes. Not sure
the formatting of the doc is sufficient or otherwise correct

## Solution

Make the `format::Writer::new()` function public. I don't see any obvious
trade-offs, but I am not familiar with the larger direction of
tracing/subscriber, so I may be wrong.

Closes  #2512
Closes #2223

Co-authored-by: Cephas Storm <cephas.storm@borisfx.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
davidbarsky pushed a commit that referenced this issue Oct 12, 2023
@kaifastromai @cephas-storm-bfx
@hawkw @davidbarsky
subscriber: make format::Writer::new() public (#2680)
32884eb 
## Motivation

As seen here #2512 and #2223. Previously pr'ed here #2525, but no progress has
been made there for quite some. I have applied the suggested changes. Not sure
the formatting of the doc is sufficient or otherwise correct

## Solution

Make the `format::Writer::new()` function public. I don't see any obvious
trade-offs, but I am not familiar with the larger direction of
tracing/subscriber, so I may be wrong.

Closes  #2512
Closes #2223

Co-authored-by: Cephas Storm <cephas.storm@borisfx.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
kaffarell pushed a commit to kaffarell/tracing that referenced this issue Nov 21, 2023
@kaifastromai @cephas-storm-bfx
@hawkw @kaffarell
subscriber: make format::Writer::new() public (tokio-rs#2680)
9143b53 
## Motivation

As seen here tokio-rs#2512 and tokio-rs#2223. Previously pr'ed here tokio-rs#2525, but no progress has
been made there for quite some. I have applied the suggested changes. Not sure
the formatting of the doc is sufficient or otherwise correct

## Solution

Make the `format::Writer::new()` function public. I don't see any obvious
trade-offs, but I am not familiar with the larger direction of
tracing/subscriber, so I may be wrong.

Closes  tokio-rs#2512
Closes tokio-rs#2223

Co-authored-by: Cephas Storm <cephas.storm@borisfx.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Fuuzetsu @TmLev @kaifastromai