New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 760431 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Support --log-file flag to override logging location

Project Member Reported by michae...@chromium.org, Aug 30 2017

Issue description

Different binaries, like chrome, content_shell and app_shell, log to different locations, sometimes depending on runtime factors. This default behavior is WAI, but it's often necessary to control exactly where the main log file ends up. The current state is problematic because:

* Each binary has its own logic for the default logging path.
* chrome allows its logging path to be overridden, but only from the environmental variable CHROME_LOG_FILE.
* app_shell and content_shell do not allow their logging paths to be overridden.

We should provide a switch in //content that overrides the log file path for non-test binaries. Following the existing log switches there, the name should begin with "log-", so I propose "log-file".

I raised this at https://groups.google.com/a/chromium.org/d/topic/chromium-dev/mo7ID94la8A/discussion but there's no discussion there yet.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66dbc95a09d9647bcecc2ab49b5f3ba39ba572e5

commit 66dbc95a09d9647bcecc2ab49b5f3ba39ba572e5
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Fri Sep 22 03:50:06 2017

Add --log-file=PATH switch to app_shell

Let users specify the logging location. The switch name is intentionally
left generic to allow other binaries to use the same flag.

Bug:  760431 
Change-Id: I5f03f146b090fa5ba0e21718aeb27f5780981dde
Reviewed-on: https://chromium-review.googlesource.com/647314
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503645}
[modify] https://crrev.com/66dbc95a09d9647bcecc2ab49b5f3ba39ba572e5/content/public/common/content_switches.cc
[modify] https://crrev.com/66dbc95a09d9647bcecc2ab49b5f3ba39ba572e5/content/public/common/content_switches.h
[modify] https://crrev.com/66dbc95a09d9647bcecc2ab49b5f3ba39ba572e5/extensions/shell/app/shell_main_delegate.cc

Owner: michae...@chromium.org
Status: Started (was: Available)
Will follow up by adding --log-file to chrome and content_shell, and some other places we use similar logic.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6802ffe8616e2846744c2132f18200653f07aed0

commit 6802ffe8616e2846744c2132f18200653f07aed0
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Fri Sep 29 03:40:25 2017

Add --log-file=PATH switch to more executables

The --log-file switch lets users specify the logging location. It was
added as a generic flag and implemented in app_shell in
crrev.com/c/647314.

This CL expands the switch to other logging configurers, notably
including chrome and content_shell.

In Chrome, --log-file=<PATH> will override the $CHROME_LOG_FILE
environmental variable if both are present.

Bug:  760431 
Change-Id: I72448b1a8cdea5b5818badd340594a2f9d6d8cd2
Reviewed-on: https://chromium-review.googlesource.com/685997
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505287}
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/chrome/browser/logging_chrome_unittest.cc
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/chrome/chrome_watcher/chrome_watcher_main.cc
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/chrome/common/logging_chrome.cc
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/chrome/common/logging_chrome.h
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/content/browser/sandbox_parameters_mac.mm
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/content/public/browser/content_browser_client.h
[modify] https://crrev.com/6802ffe8616e2846744c2132f18200653f07aed0/content/shell/app/shell_main_delegate.cc

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c807c323abc0e8f8aa396b305ce7d142eb4c8941

commit c807c323abc0e8f8aa396b305ce7d142eb4c8941
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Fri Oct 27 01:04:15 2017

Remove obsolete TODO for kLogFile

Bug:  760431 
Change-Id: I743bf1cf5bc191769fcd4808a79670c9ebd99910
Reviewed-on: https://chromium-review.googlesource.com/724665
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512047}
[modify] https://crrev.com/c807c323abc0e8f8aa396b305ce7d142eb4c8941/content/public/common/content_switches.cc

Sign in to add a comment