Skip to content

Commit

Permalink
Reland "[remoting] Add GN switch for WebRTC debug logs."
Browse files Browse the repository at this point in the history
This is a reland of f790c4c

Original change's description:
> [remoting] Add GN switch for WebRTC debug logs.
>
> In the host process, the WebRTC "VERBOSE1" logs are extremely spammy,
> and they cannot be filtered separately from "INFO" messages using --v or
> --vmodule switches (the third_party/webrtc_overrides implementation sets
> them to the same log verbosity level).
>
> To make these logs optional and easy to enable, this CL provides a
> GN argument "remoting_webrtc_verbose_logging" with a default value of
> false. Because of the huge amount of logs generated, this is set to
> false even for Debug builds.
>
> This CL also replaces AppendSwitch(..) with the corrected call to
> AppendSwitchASCII(). The former, when passed "key=value", would set a
> command-line switch literally named "key=value", whose value was an
> empty string. So the WebRTC debug logs were never enabled even in Debug
> builds.
>
> Change-Id: I2a3b5c0130f4bb004b1ebf7433fc5097513b6559
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2725024
> Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
> Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
> Reviewed-by: Joe Downing <joedow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#858408}

Change-Id: Id3f54cc9cb6a24bc2214e6d64e3ef485ac591516
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2726819
Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858477}
  • Loading branch information
Lambros Lambrou authored and Chromium LUCI CQ committed Mar 1, 2021
1 parent a390476 commit 895fc6d
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 21 deletions.
14 changes: 14 additions & 0 deletions remoting/build/config/remoting_logging.gni
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Copyright 2021 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

declare_args() {
# Enabling this will cause WebRTC's LS_INFO and LS_VERBOSE (level 1)
# messages to appear in the debug log output. Note that WebRTC's
# "VERBOSE1" messages are very spammy and it is impossible to
# separately filter LS_INFO and LS_VERBOSE messages (see
# third_party/webrtc_overrides/rtc_base/). So a separate GN option is
# provided here, to allow this logging to be quickly enabled for Debug
# or Release builds.
remoting_webrtc_verbose_logging = false
}
37 changes: 26 additions & 11 deletions remoting/host/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import("//build/config/chromeos/ui_mode.gni")
import("//build/config/python.gni")
import("//build/util/process_version.gni")
import("//remoting/build/config/remoting_build.gni")
import("//remoting/build/config/remoting_logging.gni")

group("all_tests") {
testonly = true
Expand Down Expand Up @@ -46,19 +47,10 @@ source_set("host") {
deps = [ "//remoting/host/file_transfer" ]
}

source_set("base") {
source_set("logging") {
sources = [
"host_exit_codes.cc",
"host_exit_codes.h",
"logging.cc",
"logging.h",
"switches.cc",
"switches.h",
"username.cc",
"username.h",
]
deps = [
"//base",
"//remoting/base",
]

if (is_linux || is_chromeos) {
Expand All @@ -72,6 +64,29 @@ source_set("base") {
if (is_win) {
sources += [ "logging_win.cc" ]
}

if (remoting_webrtc_verbose_logging) {
defines = [ "WEBRTC_VERBOSE_LOGGING" ]
}

deps = [ "//base" ]
}

source_set("base") {
sources = [
"host_exit_codes.cc",
"host_exit_codes.h",
"switches.cc",
"switches.h",
"username.cc",
"username.h",
]
deps = [
":logging",
"//base",
"//remoting/base",
]
public_deps = [ ":logging" ]
}

# Split up from common to avoid circular dependency.
Expand Down
9 changes: 0 additions & 9 deletions remoting/host/host_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,6 @@ int HostMain(int argc, char** argv) {

base::CommandLine::Init(argc, argv);

#if !defined(NDEBUG)
// Always enable Webrtc logging for debug builds.
// Without this switch, Webrtc errors will still be logged but
// RTC_LOG(LS_INFO) lines will not.
// See https://webrtc.org/native-code/logging
auto* cl = base::CommandLine::ForCurrentProcess();
cl->AppendSwitch("vmodule=*/webrtc/*=1");
#endif

// Parse the command line.
const base::CommandLine* command_line =
base::CommandLine::ForCurrentProcess();
Expand Down
18 changes: 18 additions & 0 deletions remoting/host/logging.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "remoting/host/logging.h"

#include "base/command_line.h"

namespace remoting {

void InitHostLoggingCommon() {
#ifdef WEBRTC_VERBOSE_LOGGING
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII("vmodule",
"*/webrtc/*=1");
#endif
}

} // namespace remoting
5 changes: 4 additions & 1 deletion remoting/host/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ constexpr GUID kRemotingHostLogProviderGuid = {
#endif

// Initializes host logging.
extern void InitHostLogging();
void InitHostLogging();

// Common initialization for all platforms, called by InitHostLogging().
void InitHostLoggingCommon();

} // namespace remoting

Expand Down
2 changes: 2 additions & 0 deletions remoting/host/logging_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
namespace remoting {

void InitHostLogging() {
InitHostLoggingCommon();

// Write logs to the system debug log.
logging::LoggingSettings settings;
settings.logging_dest =
Expand Down
2 changes: 2 additions & 0 deletions remoting/host/logging_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ bool LogMessageToAsl(
} // namespace

void InitHostLogging() {
InitHostLoggingCommon();

// Write logs to the system debug log.
logging::LoggingSettings settings;
settings.logging_dest =
Expand Down
2 changes: 2 additions & 0 deletions remoting/host/logging_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
namespace remoting {

void InitHostLogging() {
InitHostLoggingCommon();

// Write logs to the system debug log.
logging::LoggingSettings settings;
settings.logging_dest =
Expand Down

0 comments on commit 895fc6d

Please sign in to comment.