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

fmt::Subscriber NO_COLOR support #2647

Merged
merged 9 commits into from Aug 17, 2023
Merged

fmt::Subscriber NO_COLOR support #2647

merged 9 commits into from Aug 17, 2023

Conversation

dmlary
Copy link
Contributor

@dmlary dmlary commented Jul 13, 2023

Motivation

It's necessary at times to be able to disable ANSI color output for rust utilities using tracing. The informal standard for this is the NO_COLOR environment variable described here: https://no-color.org/

Further details/discussion in #2388

Solution

This commit updates fmt::Subscriber to check the NO_COLOR environment variable before enabling ANSI color output. As described in the spec, any non-empty value set for NO_COLOR will prevent is_ansi from being set to true.

fixes #2388

@dmlary
fmt::Subscriber NO_COLOR support
8fa10c9 
The `NO_COLOR` environment variable should disable all ANSI color
output (https:://no-color.org).

This commit updates `fmt::Subscriber` to check the `NO_COLOR`
environment variable before enabling ANSI color output.  As described in
the spec, any non-empty value set for `NO_COLOR` will prevent `is_ansi`
from being set to `true`.

fixes tokio-rs#2388
@dmlary dmlary requested review from hawkw, davidbarsky and a team as code owners July 13, 2023 00:37
@dmlary
update to allow NO_COLOR to be overridden
f9f3f8c 
Re-reading https://no-color.org showed that they do allow overriding of
the setting with command-line flags.  Similarly, I've removed the
changes to `fmt::subscriber::with_ansi` and `fmt::subscriber::set_ansi`;
they will once again set `is_ansi` regardless of `NO_COLOR`.
hawkw
hawkw requested changes Aug 14, 2023
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, I think your approach looks good overall! I had some smallish suggestions that we might want to address before we merge this PR.

Additionally, I think the approach proposed in #2388 (comment) seems like it might be a good idea for a follow-up, where the "automatically determine whether or not to use ANSI colors based on (potentially, multiple) env vars" is a mode which can be explicitly requested. We might want to deprecate with_ansi/set_ansi in favor of a new with_color_mode method taking an enum of "Always", "Never", or "Auto". But, it would be fine to make that change in a second branch after this is merged.

tracing-subscriber/src/fmt/fmt_subscriber.rs Show resolved Hide resolved
tracing-subscriber/src/fmt/fmt_subscriber.rs Outdated Show resolved Hide resolved
@dmlary dmlary requested a review from hawkw August 16, 2023 16:03
@dmlary
Copy link
Contributor Author

dmlary commented Aug 16, 2023

I'm not sure what's causing the wasm CI failures here.

@hawkw
Copy link
Member

hawkw commented Aug 16, 2023

I'm not sure what's causing the wasm CI failures here.

Those aren't your fault --- see #2675

hawkw
hawkw approved these changes Aug 16, 2023
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one CI failure that is related to this change --- the docs links are broken. Once that's fixed, i'll be happy to merge this!

tracing-subscriber/src/fmt/fmt_subscriber.rs Show resolved Hide resolved
tracing-subscriber/src/fmt/fmt_subscriber.rs Show resolved Hide resolved
hawkw
hawkw approved these changes Aug 17, 2023
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, thank you!

tracing-subscriber/src/fmt/fmt_subscriber.rs Outdated Show resolved Hide resolved
@hawkw hawkw enabled auto-merge (squash) August 17, 2023 16:45
@hawkw hawkw merged commit 81ab9d6 into tokio-rs:master Aug 17, 2023
56 checks passed
davidbarsky pushed a commit that referenced this pull request Sep 26, 2023
@dmlary @davidbarsky
subscriber: support NO_COLOR in fmt::Subscriber (#2647)
b83a228 
It's necessary at times to be able to disable ANSI color output for
rust utilities using `tracing`.  The informal standard for this is the
`NO_COLOR` environment variable described here: https://no-color.org/

Further details/discussion in #2388

This commit updates `fmt::Subscriber` to check the `NO_COLOR`
environment variable when determining whether ANSI color output is
enabled by default. As described in the spec, any non-empty value set
for `NO_COLOR` will cause ANSI color support to be disabled by default.

If the user manually overrides ANSI color support, such as by calling
`with_ansi(true)`, this will still enable ANSI colors, even if
`NO_COLOR` is set. The `NO_COLOR` env var only effects the default
behavior.

Fixes #2388
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@dmlary @davidbarsky
subscriber: support NO_COLOR in fmt::Subscriber (#2647)
824d70e 
It's necessary at times to be able to disable ANSI color output for
rust utilities using `tracing`.  The informal standard for this is the
`NO_COLOR` environment variable described here: https://no-color.org/

Further details/discussion in #2388

This commit updates `fmt::Subscriber` to check the `NO_COLOR`
environment variable when determining whether ANSI color output is
enabled by default. As described in the spec, any non-empty value set
for `NO_COLOR` will cause ANSI color support to be disabled by default.

If the user manually overrides ANSI color support, such as by calling
`with_ansi(true)`, this will still enable ANSI colors, even if
`NO_COLOR` is set. The `NO_COLOR` env var only effects the default
behavior.

Fixes #2388
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@dmlary @davidbarsky
subscriber: support NO_COLOR in fmt::Subscriber (#2647)
d66f129 
It's necessary at times to be able to disable ANSI color output for
rust utilities using `tracing`.  The informal standard for this is the
`NO_COLOR` environment variable described here: https://no-color.org/

Further details/discussion in #2388

This commit updates `fmt::Subscriber` to check the `NO_COLOR`
environment variable when determining whether ANSI color output is
enabled by default. As described in the spec, any non-empty value set
for `NO_COLOR` will cause ANSI color support to be disabled by default.

If the user manually overrides ANSI color support, such as by calling
`with_ansi(true)`, this will still enable ANSI colors, even if
`NO_COLOR` is set. The `NO_COLOR` env var only effects the default
behavior.

Fixes #2388
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@dmlary @davidbarsky
subscriber: support NO_COLOR in fmt::Layer (#2647)
8372ba3 
It's necessary at times to be able to disable ANSI color output for
rust utilities using `tracing`.  The informal standard for this is the
`NO_COLOR` environment variable described here: https://no-color.org/

Further details/discussion in #2388

This commit updates `fmt::Layer` to check the `NO_COLOR`
environment variable when determining whether ANSI color output is
enabled by default. As described in the spec, any non-empty value set
for `NO_COLOR` will cause ANSI color support to be disabled by default.

If the user manually overrides ANSI color support, such as by calling
`with_ansi(true)`, this will still enable ANSI colors, even if
`NO_COLOR` is set. The `NO_COLOR` env var only effects the default
behavior.

Fixes #2388
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@dmlary @davidbarsky
subscriber: support NO_COLOR in fmt::Layer (#2647)
da44a0d 
It's necessary at times to be able to disable ANSI color output for
rust utilities using `tracing`.  The informal standard for this is the
`NO_COLOR` environment variable described here: https://no-color.org/

Further details/discussion in #2388

This commit updates `fmt::Layer` to check the `NO_COLOR`
environment variable when determining whether ANSI color output is
enabled by default. As described in the spec, any non-empty value set
for `NO_COLOR` will cause ANSI color support to be disabled by default.

If the user manually overrides ANSI color support, such as by calling
`with_ansi(true)`, this will still enable ANSI colors, even if
`NO_COLOR` is set. The `NO_COLOR` env var only effects the default
behavior.

Fixes #2388
davidbarsky pushed a commit that referenced this pull request Sep 29, 2023
@dmlary @davidbarsky
subscriber: support NO_COLOR in fmt::Layer (#2647)
bbc87fe 
It's necessary at times to be able to disable ANSI color output for
rust utilities using `tracing`.  The informal standard for this is the
`NO_COLOR` environment variable described here: https://no-color.org/

Further details/discussion in #2388

This commit updates `fmt::Layer` to check the `NO_COLOR`
environment variable when determining whether ANSI color output is
enabled by default. As described in the spec, any non-empty value set
for `NO_COLOR` will cause ANSI color support to be disabled by default.

If the user manually overrides ANSI color support, such as by calling
`with_ansi(true)`, this will still enable ANSI colors, even if
`NO_COLOR` is set. The `NO_COLOR` env var only effects the default
behavior.

Fixes #2388
hawkw pushed a commit that referenced this pull request Oct 1, 2023
@dmlary @hawkw
subscriber: support NO_COLOR in fmt::Layer (#2647)
72bbf0f 
It's necessary at times to be able to disable ANSI color output for
rust utilities using `tracing`.  The informal standard for this is the
`NO_COLOR` environment variable described here: https://no-color.org/

Further details/discussion in #2388

This commit updates `fmt::Layer` to check the `NO_COLOR`
environment variable when determining whether ANSI color output is
enabled by default. As described in the spec, any non-empty value set
for `NO_COLOR` will cause ANSI color support to be disabled by default.

If the user manually overrides ANSI color support, such as by calling
`with_ansi(true)`, this will still enable ANSI colors, even if
`NO_COLOR` is set. The `NO_COLOR` env var only effects the default
behavior.

Fixes #2388
hawkw pushed a commit that referenced this pull request Nov 13, 2023
@davidbarsky
chore: prepare tracing-subscriber 0.3.18 release (#2789)
8b7a1dd 
# 0.3.18 (November 13, 2023)

This release of `tracing-subscriber` adds support for the [`NO_COLOR`] environment
variable (an informal standard to disable emitting ANSI color escape codes) in
`fmt::Layer`, reintroduces support for the [`chrono`] crate, and increases the
minimum supported Rust version (MSRV) to Rust 1.63.0.

It also introduces several minor API improvements.

### Added

- **chrono**: Add [`chrono`] implementations of `FormatTime` ([#2690])
- **subscriber**: Add support for the [`NO_COLOR`] environment variable in
`fmt::Layer` ([#2647])
- **fmt**: make `format::Writer::new()` public ([#2680])
- **filter**: Implement `layer::Filter` for `Option<Filter>` ([#2407])

### Changed

- **log**: bump version of `tracing-log` to 0.2 ([#2772])
- Increased minimum supported Rust version (MSRV) to 1.63.0+.

[`chrono`]: https://github.com/chronotope/chrono
[`NO_COLOR`]: https://no-color.org/
[#2690]: #2690
[#2647]: #2647
[#2680]: #2680
[#2407]: #2407
[#2772]: #2772

Thanks to @shayne-fletcher, @dmlary, @kaifastromai, and @jsgf for contributing!
@Turbo87 Turbo87 mentioned this pull request Nov 14, 2023
Merged
@dmlary dmlary mentioned this pull request Jan 18, 2024
Closed
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request Feb 14, 2024
@davidbarsky @kaffarell
chore: prepare tracing-subscriber 0.3.18 release (tokio-rs#2789)
a654936 
# 0.3.18 (November 13, 2023)

This release of `tracing-subscriber` adds support for the [`NO_COLOR`] environment
variable (an informal standard to disable emitting ANSI color escape codes) in
`fmt::Layer`, reintroduces support for the [`chrono`] crate, and increases the
minimum supported Rust version (MSRV) to Rust 1.63.0.

It also introduces several minor API improvements.

### Added

- **chrono**: Add [`chrono`] implementations of `FormatTime` ([tokio-rs#2690])
- **subscriber**: Add support for the [`NO_COLOR`] environment variable in
`fmt::Layer` ([tokio-rs#2647])
- **fmt**: make `format::Writer::new()` public ([tokio-rs#2680])
- **filter**: Implement `layer::Filter` for `Option<Filter>` ([tokio-rs#2407])

### Changed

- **log**: bump version of `tracing-log` to 0.2 ([tokio-rs#2772])
- Increased minimum supported Rust version (MSRV) to 1.63.0+.

[`chrono`]: https://github.com/chronotope/chrono
[`NO_COLOR`]: https://no-color.org/
[tokio-rs#2690]: tokio-rs#2690
[tokio-rs#2647]: tokio-rs#2647
[tokio-rs#2680]: tokio-rs#2680
[tokio-rs#2407]: tokio-rs#2407
[tokio-rs#2772]: tokio-rs#2772

Thanks to @shayne-fletcher, @dmlary, @kaifastromai, and @jsgf for contributing!
@aaronriekenberg aaronriekenberg mentioned this pull request Mar 3, 2024
Closed
m-wynn added a commit to m-wynn/terraform-coralogix-aws that referenced this pull request May 10, 2024
@m-wynn
Add NO_COLOR env var to coralogix-aws-shipper
29d823d 
The tracing library that coralogix-aws-shipper supports NO_COLOR, which
avoids sending ansi control characters to cloudwatch logs.

* [tracing library pr](tokio-rs/tracing#2647)

* [NO_COLOR](https://no-color.org/)
@m-wynn m-wynn mentioned this pull request May 10, 2024
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkw hawkw hawkw approved these changes

@davidbarsky davidbarsky Awaiting requested review from davidbarsky davidbarsky is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fmt subscriber should honor NO_COLOR by default
2 participants
@dmlary @hawkw