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

Add the logtostdout and colorlogtostdout flag to allow logging to stdout #790

Merged
merged 1 commit into from
Feb 20, 2022

Conversation

git-hulk
Copy link
Contributor

Currently, glog allowed to use logtostderr to send error logs to stderr,
but many log tailors would regard logs from stderr as error logs. So we
want to send non-error logs to stdout and only send error logs to stderr
according to the stderrthreshold.

Resolved #701

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2022

Codecov Report

Merging #790 (a5f29ef) into master (aa94e6b) will increase coverage by 0.15%.
The diff coverage is 73.80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #790      +/-   ##
==========================================
+ Coverage   72.78%   72.93%   +0.15%     
==========================================
  Files          17       17              
  Lines        3259     3281      +22     
==========================================
+ Hits         2372     2393      +21     
- Misses        887      888       +1     
Impacted Files Coverage Δ
src/glog/logging.h.in 80.00% <ø> (ø)
src/logging.cc 74.03% <71.79%> (+0.32%) ⬆️
src/raw_logging.cc 100.00% <100.00%> (ø)
src/vlog_is_on.cc 70.22% <0.00%> (ø)
src/windows/port.h 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa94e6b...a5f29ef. Read the comment docs.

@git-hulk
Copy link
Contributor Author

@sergiud I have add a new file(logging_unittet.out) at src directory, but it seems can't be read at Window. Did I miss anything?

@sergiud
Copy link
Collaborator

sergiud commented Feb 13, 2022

The tests of MSVC builds are failing. Could you please rebase onto current master and see if this helps?

@git-hulk git-hulk force-pushed the feature/logs-to-stdout branch from 664e178 to 0df1696 Compare February 13, 2022 12:40
@sergiud sergiud force-pushed the feature/logs-to-stdout branch from 0df1696 to 2f0c27a Compare February 13, 2022 12:56
@sergiud
Copy link
Collaborator

sergiud commented Feb 13, 2022

I fixed the formatting and revised the commit message. Some tests seem to be flaky though which is likely not your fault.

@sergiud sergiud force-pushed the feature/logs-to-stdout branch from 2f0c27a to 2f9a5dd Compare February 13, 2022 13:01
@git-hulk
Copy link
Contributor Author

@sergiud cool, thanks for your great help!

@git-hulk
Copy link
Contributor Author

@sergiud How do you think about this PR? Does this implementation make sense for you?

@sergiud
Copy link
Collaborator

sergiud commented Feb 16, 2022

I already skipped through the PR and it does make sense to me. However, I would like to wait for additional comments from users (if any). So please hold tight.

@git-hulk
Copy link
Contributor Author

ok, cool. Sorry for pushing you.

Copy link
Collaborator

@sergiud sergiud left a comment

Choose a reason for hiding this comment

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

I have a few suggestions. Thanks for looking into the issue!

src/googletest.h Outdated Show resolved Hide resolved
src/logging.cc Outdated Show resolved Hide resolved
src/logging.cc Outdated Show resolved Hide resolved
src/logging.cc Show resolved Hide resolved
@git-hulk
Copy link
Contributor Author

Thanks for your suggestions, has resolved those issues.

@git-hulk git-hulk requested a review from sergiud February 18, 2022 08:22
@sergiud
Copy link
Collaborator

sergiud commented Feb 19, 2022

Looks great!

@sergiud sergiud added this to the 0.6 milestone Feb 19, 2022
Currently, glog allows to use of logtostderr to send error logs to
stderr, but many log tailers would regard logs from stderr as error
logs. So we want to send non-error logs to stdout and only send error
logs to stderr according to the stderrthreshold.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console logging: separation of stdout and stderr
3 participants