Add chip_logging_backend option and 'none' and 'syslog' backends (#34830

)

* Tidy up stdio logging target dependencies

Also rename from Logging.cpp to Stdio.cpp and add license header.

* Add chip_logging_backend option and 'none' backend

chip_logging_backend controls which backend is pulled in by the
src/platform/logging:default target. The default is 'platform', retaining the
current behavior.

On Darwin, remove the no-op LoggingImpl and make stdio the default backend when
compiling tools or example apps. Use the new 'none' backend when building
Matter.framework, retaining current behavior.

Depend on logging:default instead of logging:stdio for linux:app-main examples.

* Add a syslog logging backend

* Fix STM32 build

* Use stdio logging backend for cirque tests

The stdio and linux backends use slightly different output formats and the
cirque tests currently only parse the stdio format correctly.

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

* Fix stray space in GN string comparison

---------

Co-authored-by: Boris Zbarsky <[email protected]>

examples/platform/linux/BUILD.gn

@@ -93,7 +93,7 @@ source_set("app-main") {
     "${chip_root}/src/controller:controller",
     "${chip_root}/src/controller:gen_check_chip_controller_headers",
     "${chip_root}/src/lib",
-    "${chip_root}/src/platform/logging:stdio",
+    "${chip_root}/src/platform/logging:default",
   ]
   deps = [
     ":ota-test-event-trigger",

scripts/build/gn_gen_cirque.sh

@@ -36,7 +36,7 @@ echo "Setup build environment"
 source "./scripts/activate.sh"
 
 echo "Build: GN configure"
-gn --root="$CHIP_ROOT" gen --check --fail-on-unused-args out/debug --args='target_os="all"'"chip_build_tests=false chip_enable_wifi=false chip_im_force_fabric_quota_check=true enable_default_builds=false enable_host_gcc_build=true enable_standalone_chip_tool_build=true enable_linux_all_clusters_app_build=true enable_linux_lighting_app_build=true enable_linux_lit_icd_app_build=true"
+gn --root="$CHIP_ROOT" gen --check --fail-on-unused-args out/debug --args='target_os="all" chip_logging_backend="stdio" chip_build_tests=false chip_enable_wifi=false chip_im_force_fabric_quota_check=true enable_default_builds=false enable_host_gcc_build=true enable_standalone_chip_tool_build=true enable_linux_all_clusters_app_build=true enable_linux_lighting_app_build=true enable_linux_lit_icd_app_build=true'
 
 echo "Build: Ninja build"
 time ninja -C out/debug all check

src/darwin/Framework/chip_xcode_build_connector.sh

@@ -103,6 +103,7 @@ declare -a args=(
     'chip_enable_python_modules=false'
     'chip_device_config_enable_dynamic_mrp_config=true'
     'chip_log_message_max_size=4096' # might as well allow nice long log messages
+    'chip_logging_backend="none"'    # os_log() is integrated via CHIP_SYSTEM_CONFIG_PLATFORM_LOG
     'chip_disable_platform_kvs=true'
     'enable_fuzz_test_targets=false'
     "target_cpu=\"$target_cpu\""

src/lib/core/core.gni

@@ -54,7 +54,23 @@ declare_args() {
   # Configure chip logging to output through external logging implementation.
   # External code will need to provide implementation for CHIP log output
   # function (LogV), which is defined in "src/platform/logging/LogV.h".
+  # Same as setting chip_logging_backend = "external"
   chip_use_external_logging = false
+}
+
+declare_args() {
+  # Logging backend to use for targets that don't link a specific log
+  # backend (e.g. command line utilites usually use 'stdio'). Options:
+  #   'platform' - The default log backend of the device platform
+  #   'external' - External LogV implementation (src/platform/logging/LogV.h)
+  #   'none'     - Discard all log output
+  #   'stdio'    - Print to stdout
+  #   'syslog'   - POSIX syslog()
+  if (chip_use_external_logging) {
+    chip_logging_backend = "external"
+  } else {
+    chip_logging_backend = "platform"
+  }
 
   # Enable short error strings.
   chip_config_short_error_str = false
@@ -117,6 +133,15 @@ if (chip_target_style == "") {
   }
 }
 
+assert(
+    chip_logging_backend == "platform" || chip_logging_backend == "external" ||
+        chip_logging_backend == "none" || chip_logging_backend == "stdio" ||
+        chip_logging_backend == "syslog",
+    "Please select a valid logging backend: platform, external, none, stdio, syslog")
+assert(
+    !chip_use_external_logging || chip_logging_backend == "external",
+    "Setting chip_use_external_logging = true conflicts with selected chip_logging_backend")
+
 assert(chip_target_style == "unix" || chip_target_style == "embedded",
        "Please select a valid target style: unix, embedded")
 

src/platform/Darwin/BUILD.gn

@@ -151,7 +151,6 @@ static_library("logging") {
   sources = [
     "Logging.h",
     "Logging.mm",
-    "LoggingImpl.cpp",
   ]
 
   deps = [

src/platform/logging/BUILD.gn

@@ -8,8 +8,8 @@ import("${chip_root}/src/lib/core/core.gni")
 import("${chip_root}/src/lib/shell/shell_device.gni")
 import("${chip_root}/src/platform/device.gni")
 
-source_set("default") {
-  if (!chip_use_external_logging) {
+group("default") {
+  if (chip_logging_backend == "platform") {
     deps = []
 
     if (chip_use_pw_logging) {
@@ -63,7 +63,7 @@ source_set("default") {
     } else if (chip_device_platform == "qpg") {
       deps += [ "${chip_root}/src/platform/qpg:logging" ]
     } else if (chip_device_platform == "darwin") {
-      deps += [ "${chip_root}/src/platform/Darwin:logging" ]
+      deps += [ ":stdio" ]  # For tools / examples. The framework uses "none".
     } else if (chip_device_platform == "mw320") {
       deps += [ "${chip_root}/src/platform/nxp/mw320:logging" ]
     } else if (chip_device_platform == "k32w0" ||
@@ -82,22 +82,43 @@ source_set("default") {
       assert(chip_device_platform == "fake" ||
              chip_device_platform == "external" || chip_device_platform == "none")
     }
+  } else if (chip_logging_backend == "none" ||
+             chip_logging_backend == "stdio" ||
+             chip_logging_backend == "syslog") {
+    deps = [ ":${chip_logging_backend}" ]
+  } else {
+    assert(chip_logging_backend == "external")
   }
 }
 
 source_set("headers") {
   public = [ "LogV.h" ]
+  public_deps = [
+    "${chip_root}/src/lib/support:attributes",
+    "${chip_root}/src/lib/support:logging_constants",
+  ]
+}
+
+source_set("none") {
+  sources = [ "impl/None.cpp" ]
+  deps = [
+    ":headers",
+    "${chip_root}/src/platform:platform_base",
+  ]
 }
 
 source_set("stdio") {
-  sources = [ "impl/stdio/Logging.cpp" ]
+  sources = [ "impl/Stdio.cpp" ]
+  deps = [
+    ":headers",
+    "${chip_root}/src/platform:platform_base",
+  ]
+}
 
+source_set("syslog") {
+  sources = [ "impl/Syslog.cpp" ]
   deps = [
     ":headers",
-    "${chip_root}/src/lib/core:chip_config_header",
-    "${chip_root}/src/lib/support:attributes",
-    "${chip_root}/src/lib/support:logging_constants",
-    "${chip_root}/src/platform:platform_config_header",
-    "${chip_root}/src/platform/logging:headers",
+    "${chip_root}/src/platform:platform_base",
   ]
 }

src/platform/Darwin/LoggingImpl.cpp → src/platform/logging/impl/None.cpp

@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2021 Project CHIP Authors
+ *    Copyright (c) 2024 Project CHIP Authors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -21,12 +21,11 @@ namespace chip {
 namespace Logging {
 namespace Platform {
 
-void LogV(const char * module, uint8_t category, const char * msg, va_list v)
+void LogV(const char *, uint8_t, const char *, va_list)
 {
-    // ChipPlatformLog expands to an os_log call directly (see Logging.h), so
-    // we don't need to do anything further here. However his function and the
-    // call to it still exist because of scenarios where a different logging
-    // backend (usually stdio) is swapped in at link time, e.g. for unit tests.
+    // This backend discards all log messages. This is useful when all log output
+    // is routed via `SetLogRedirectCallback()` and/or platform logging
+    // integration at the log macro level (`CHIP_SYSTEM_CONFIG_PLATFORM_LOG`).
 }
 
 } // namespace Platform

src/platform/logging/impl/stdio/Logging.cpp → src/platform/logging/impl/Stdio.cpp

@@ -1,4 +1,19 @@
-/* See Project CHIP LICENSE file for licensing information. */
+/*
+ *
+ *    Copyright (c) 2021-2024 Project CHIP Authors
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
 
 #include <platform/logging/LogV.h>
 

src/platform/logging/impl/Syslog.cpp

@@ -0,0 +1,65 @@
+/*
+ *
+ *    Copyright (c) 2024 Project CHIP Authors
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#include <platform/logging/LogV.h>
+
+#include <lib/core/CHIPConfig.h>
+#include <lib/support/logging/Constants.h>
+
+#include <mutex>
+#include <syslog.h>
+
+namespace chip {
+namespace Logging {
+namespace Platform {
+
+namespace {
+int LogPriority(uint8_t category)
+{
+    switch (category)
+    {
+    case kLogCategory_Error:
+        return LOG_ERR;
+    case kLogCategory_Progress:
+        return LOG_NOTICE;
+    default:
+        return LOG_DEBUG;
+    }
+}
+} // namespace
+
+void LogV(const char * module, uint8_t category, const char * msg, va_list v)
+{
+    static std::mutex sMutex;
+    static bool sInitialized = false;
+    static char sBuffer[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
+    std::lock_guard guard(sMutex);
+
+    if (!sInitialized)
+    {
+        openlog(nullptr, 0, LOG_DAEMON);
+        sInitialized = true;
+    }
+
+    // Pre-format the message so we can include the module name
+    vsnprintf(sBuffer, sizeof(sBuffer), msg, v);
+    syslog(LogPriority(category), "%s: %s", module, sBuffer);
+}
+
+} // namespace Platform
+} // namespace Logging
+} // namespace chip

src/platform/stm32/BUILD.gn

@@ -39,7 +39,6 @@ static_library("stm32") {
     sources = [
       "../FreeRTOS/SystemTimeSupport.cpp",
       "../SingletonConfigurationManager.cpp",
-      "../logging/impl/stdio/Logging.cpp",
       "BLEManagerImpl.cpp",
       "BLEManagerImpl.h",
       "BlePlatformConfig.h",
@@ -69,7 +68,7 @@ static_library("stm32") {
       "SystemPlatformConfig.h",
     ]
 
-    deps += [ "${chip_root}/src/platform/logging:headers" ]
+    deps += [ "${chip_root}/src/platform/logging:stdio" ]
   }
 
   public = [

src/system/SystemConfig.h

@@ -558,7 +558,9 @@ struct LwIPEvent;
  *  @def CHIP_SYSTEM_CONFIG_PLATFORM_LOG
  *
  *  @brief
- *      Defines whether (1) or not (0) the system uses a platform-specific logging implementation.
+ *      Defines whether (1) or not (0) the system uses a platform-specific implementation of
+ *      ChipLog* macros. Most platforms do not use this option and simply provide a logging
+ *      backend that implements LogV.
  *
  *  See CHIPLogging.h for details.
  */