logging: allow partial updates to log handler settings

Summary:
This updates the LogHandlerConfig code to allow changing the settings on an
existing log handler without listing all of its existing options from scratch.

This also changes the syntax of the basic log handler configuration string to
use a colon to separate the log handler name+type from the options list.  In
other words, rather than specifying `default=stream,stream=stderr,async=true`
you now say `default=stream:stream=stderr,async=true`.

The primary motivation for this change is to make it easy for users to switch
the async setting for the default log handler on or off.  Callers can now
specify `default:async=true` to easily enable async mode on the default log
handler without having to completely re-list the full settings for the default
handler.

Reviewed By: yfeldblum

Differential Revision: D6494228

fbshipit-source-id: 52a296f800a5456f0c3aa10546298139c8db52fc

folly/experimental/logging/Init.cpp

@@ -44,7 +44,7 @@ namespace folly {
  * handler, the handler will be automatically forgotten by the LoggerDB code.
  */
 constexpr StringPiece kDefaultLoggingConfig =
-    ".=WARN:default; default=stream,stream=stderr,async=false";
+    ".=WARN:default; default=stream:stream=stderr,async=false";
 
 void initLogging(StringPiece configString) {
   // Register the StreamHandlerFactory

folly/experimental/logging/LogConfig.cpp

@@ -15,6 +15,8 @@
  */
 #include <folly/experimental/logging/LogConfig.h>
 
+#include <folly/Conv.h>
+
 namespace folly {
 
 bool LogConfig::operator==(const LogConfig& other) const {
@@ -30,9 +32,24 @@ void LogConfig::update(const LogConfig& other) {
   // Update handlerConfigs_ with all of the entries from the other LogConfig.
   // Any entries already present in our handlerConfigs_ are replaced wholesale.
   for (const auto& entry : other.handlerConfigs_) {
-    auto result = handlerConfigs_.insert(entry);
-    if (!result.second) {
-      result.first->second = entry.second;
+    if (entry.second.type.hasValue()) {
+      // This is a complete LogHandlerConfig that should be inserted
+      // or completely replace an existing handler config with this name.
+      auto result = handlerConfigs_.insert(entry);
+      if (!result.second) {
+        result.first->second = entry.second;
+      }
+    } else {
+      // This config is updating an existing LogHandlerConfig rather than
+      // completely replacing it.
+      auto iter = handlerConfigs_.find(entry.first);
+      if (iter == handlerConfigs_.end()) {
+        throw std::invalid_argument(to<std::string>(
+            "cannot update configuration for unknown log handler \"",
+            entry.first,
+            "\""));
+      }
+      iter->second.update(entry.second);
     }
   }
 

folly/experimental/logging/LogConfigParser.cpp

@@ -357,49 +357,84 @@ bool splitNameValue(
 }
 
 std::pair<std::string, LogHandlerConfig> parseHandlerConfig(StringPiece value) {
-  std::vector<StringPiece> pieces;
-  folly::split(",", value, pieces);
-  FOLLY_SAFE_DCHECK(
-      pieces.size() >= 1, "folly::split() always returns a list of length 1");
+  // Parse the handler name and optional type
+  auto colonIndex = value.find(':');
+  StringPiece namePortion;
+  StringPiece optionsStr;
+  if (colonIndex == StringPiece::npos) {
+    namePortion = value;
+  } else {
+    namePortion = StringPiece{value.begin(), value.begin() + colonIndex};
+    optionsStr = StringPiece{value.begin() + colonIndex + 1, value.end()};
+  }
 
   StringPiece handlerName;
-  StringPiece handlerType;
-  if (!splitNameValue(pieces[0], &handlerName, &handlerType)) {
-    throw LogConfigParseError{to<string>(
-        "error parsing log handler configuration \"",
-        pieces[0],
-        "\": expected data in the form NAME=TYPE")};
+  Optional<StringPiece> handlerType(in_place);
+  if (!splitNameValue(namePortion, &handlerName, &handlerType.value())) {
+    handlerName = trimWhitespace(namePortion);
+    handlerType = folly::none;
   }
+
+  // Make sure the handler name and type are not empty.
+  // Also disallow commas in the name: this helps catch accidental errors where
+  // the user left out the ':' and intended to be specifying options instead of
+  // part of the name or type.
   if (handlerName.empty()) {
     throw LogConfigParseError{
         "error parsing log handler configuration: empty log handler name"};
   }
-  if (handlerType.empty()) {
+  if (handlerName.contains(',')) {
     throw LogConfigParseError{to<string>(
         "error parsing configuration for log handler \"",
         handlerName,
-        "\": empty log handler type")};
+        "\": name cannot contain a comma when using the basic config format")};
   }
-
-  LogHandlerConfig config{handlerType};
-  for (size_t n = 1; n < pieces.size(); ++n) {
-    StringPiece optionName;
-    StringPiece optionValue;
-    if (!splitNameValue(pieces[n], &optionName, &optionValue)) {
+  if (handlerType.hasValue()) {
+    if (handlerType->empty()) {
       throw LogConfigParseError{to<string>(
           "error parsing configuration for log handler \"",
           handlerName,
-          "\": options must be of the form NAME=VALUE")};
+          "\": empty log handler type")};
     }
-
-    auto ret = config.options.emplace(optionName.str(), optionValue.str());
-    if (!ret.second) {
+    if (handlerType->contains(',')) {
       throw LogConfigParseError{to<string>(
           "error parsing configuration for log handler \"",
           handlerName,
-          "\": duplicate configuration for option \"",
-          optionName,
-          "\"")};
+          "\": invalid type \"",
+          handlerType.value(),
+          "\": type name cannot contain a comma when using "
+          "the basic config format")};
+    }
+  }
+
+  // Parse the options
+  LogHandlerConfig config{handlerType};
+  optionsStr = trimWhitespace(optionsStr);
+  if (!optionsStr.empty()) {
+    std::vector<StringPiece> pieces;
+    folly::split(",", optionsStr, pieces);
+    FOLLY_SAFE_DCHECK(
+        pieces.size() >= 1, "folly::split() always returns a list of length 1");
+
+    for (const auto& piece : pieces) {
+      StringPiece optionName;
+      StringPiece optionValue;
+      if (!splitNameValue(piece, &optionName, &optionValue)) {
+        throw LogConfigParseError{to<string>(
+            "error parsing configuration for log handler \"",
+            handlerName,
+            "\": options must be of the form NAME=VALUE")};
+      }
+
+      auto ret = config.options.emplace(optionName.str(), optionValue.str());
+      if (!ret.second) {
+        throw LogConfigParseError{to<string>(
+            "error parsing configuration for log handler \"",
+            handlerName,
+            "\": duplicate configuration for option \"",
+            optionName,
+            "\"")};
+      }
     }
   }
 
@@ -535,7 +570,11 @@ dynamic logConfigToDynamic(const LogHandlerConfig& config) {
   for (const auto& opt : config.options) {
     options.insert(opt.first, opt.second);
   }
-  return dynamic::object("type", config.type)("options", options);
+  auto result = dynamic::object("options", options);
+  if (config.type.hasValue()) {
+    result("type", config.type.value());
+  }
+  return std::move(result);
 }
 
 dynamic logConfigToDynamic(const LogCategoryConfig& config) {

folly/experimental/logging/LogHandlerConfig.cpp

@@ -15,13 +15,34 @@
  */
 #include <folly/experimental/logging/LogHandlerConfig.h>
 
+#include <folly/lang/SafeAssert.h>
+
+using std::string;
+
 namespace folly {
 
+LogHandlerConfig::LogHandlerConfig() {}
+
 LogHandlerConfig::LogHandlerConfig(StringPiece t) : type{t.str()} {}
 
+LogHandlerConfig::LogHandlerConfig(Optional<StringPiece> t)
+    : type{t.hasValue() ? Optional<string>{t->str()} : Optional<string>{}} {}
+
 LogHandlerConfig::LogHandlerConfig(StringPiece t, Options opts)
     : type{t.str()}, options{std::move(opts)} {}
 
+LogHandlerConfig::LogHandlerConfig(Optional<StringPiece> t, Options opts)
+    : type{t.hasValue() ? Optional<string>{t->str()} : Optional<string>{}},
+      options{std::move(opts)} {}
+
+void LogHandlerConfig::update(const LogHandlerConfig& other) {
+  FOLLY_SAFE_DCHECK(
+      !other.type.hasValue(), "LogHandlerConfig type cannot be updated");
+  for (const auto& option : other.options) {
+    options[option.first] = option.second;
+  }
+}
+
 bool LogHandlerConfig::operator==(const LogHandlerConfig& other) const {
   return type == other.type && options == other.options;
 }

folly/experimental/logging/LogHandlerConfig.h

@@ -18,6 +18,7 @@
 #include <string>
 #include <unordered_map>
 
+#include <folly/Optional.h>
 #include <folly/Range.h>
 
 namespace folly {
@@ -29,13 +30,32 @@ class LogHandlerConfig {
  public:
   using Options = std::unordered_map<std::string, std::string>;
 
-  explicit LogHandlerConfig(folly::StringPiece type);
-  LogHandlerConfig(folly::StringPiece type, Options options);
+  LogHandlerConfig();
+  explicit LogHandlerConfig(StringPiece type);
+  explicit LogHandlerConfig(Optional<StringPiece> type);
+  LogHandlerConfig(StringPiece type, Options options);
+  LogHandlerConfig(Optional<StringPiece> type, Options options);
+
+  /**
+   * Update this LogHandlerConfig object by merging in settings from another
+   * LogConfig.
+   *
+   * The other LogHandlerConfig must not have a type set.
+   */
+  void update(const LogHandlerConfig& other);
 
   bool operator==(const LogHandlerConfig& other) const;
   bool operator!=(const LogHandlerConfig& other) const;
 
-  std::string type;
+  /**
+   * The handler type name.
+   *
+   * If this field is unset than this configuration object is intended to be
+   * used to update an existing LogHandler object.  This field must always
+   * be set in the configuration for all existing LogHandler objects.
+   */
+  Optional<std::string> type;
+
   Options options;
 };
 

folly/experimental/logging/LoggerDB.cpp

@@ -195,13 +195,6 @@ void LoggerDB::startConfigUpdate(
 
   // Create all of the new LogHandlers needed from this configuration
   for (const auto& entry : config.getHandlerConfigs()) {
-    // Look up the LogHandlerFactory
-    auto factoryIter = handlerInfo->factories.find(entry.second.type);
-    if (factoryIter == handlerInfo->factories.end()) {
-      throw std::invalid_argument(to<std::string>(
-          "unknown log handler type \"", entry.second.type, "\""));
-    }
-
     // Check to see if there is an existing LogHandler with this name
     std::shared_ptr<LogHandler> oldHandler;
     auto iter = handlers->find(entry.first);
@@ -209,17 +202,49 @@ void LoggerDB::startConfigUpdate(
       oldHandler = iter->second;
     }
 
+    LogHandlerConfig updatedConfig;
+    const LogHandlerConfig* handlerConfig;
+    if (entry.second.type.hasValue()) {
+      handlerConfig = &entry.second;
+    } else {
+      // This configuration is intended to update an existing LogHandler
+      if (!oldHandler) {
+        throw std::invalid_argument(to<std::string>(
+            "cannot update unknown log handler \"", entry.first, "\""));
+      }
+
+      updatedConfig = oldHandler->getConfig();
+      if (!updatedConfig.type.hasValue()) {
+        // This normally should not happen unless someone improperly manually
+        // constructed a LogHandler object.  All existing LogHandler objects
+        // should indicate their type.
+        throw std::invalid_argument(to<std::string>(
+            "existing log handler \"",
+            entry.first,
+            "\" is missing type information"));
+      }
+      updatedConfig.update(entry.second);
+      handlerConfig = &updatedConfig;
+    }
+
+    // Look up the LogHandlerFactory
+    auto factoryIter = handlerInfo->factories.find(handlerConfig->type.value());
+    if (factoryIter == handlerInfo->factories.end()) {
+      throw std::invalid_argument(to<std::string>(
+          "unknown log handler type \"", handlerConfig->type.value(), "\""));
+    }
+
     // Create the new log handler
     const auto& factory = factoryIter->second;
     std::shared_ptr<LogHandler> handler;
     try {
       if (oldHandler) {
-        handler = factory->updateHandler(oldHandler, entry.second.options);
+        handler = factory->updateHandler(oldHandler, handlerConfig->options);
         if (handler != oldHandler) {
           oldToNewHandlerMap->emplace(oldHandler, handler);
         }
       } else {
-        handler = factory->createHandler(entry.second.options);
+        handler = factory->createHandler(handlerConfig->options);
       }
     } catch (const std::exception& ex) {
       // Errors creating or updating the the log handler are generally due to

folly/experimental/logging/docs/Config.md

@@ -70,7 +70,8 @@ special character like a comma or semicolon use the JSON format instead.
 <handler_list> ::= ":" <handler_name> <handler_list>
                  | <empty_string>
 
-<handler_config> ::= <handler_name> "=" <handler_type> <handler_options>
+<handler_config> ::= <handler_name> "=" <handler_type> ":" <handler_options>
+                   | <handler_name> ":" <handler_options>
 <handler_options> ::= "," <option_name> "=" <option_value> <handler_options>
                     | <empty_string>
 
@@ -113,13 +114,22 @@ for this category to be cleared instead.
 
 Each log handler configuration section takes the form
 
-  NAME=TYPE,OPTION1=VALUE1,OPTION2=VALUE2
+  NAME=TYPE:OPTION1=VALUE1,OPTION2=VALUE2
 
 NAME specifies the log handler name, and TYPE specifies the log handler
 type.  A comma separated list of name=value options may follow the log
 handler name and type.  The option list will be passed to the
 LogHandlerFactory for the specified handler type.
 
+The log handler type may be omitted to update the settings of an existing log
+handler object:
+
+  NAME:OPTION1=VALUE1
+
+A log handler with this name must already exist.  Options specified in the
+configuration will be updated with their new values, and any option names not
+mentioned will be left unchanged.
+
 
 ### Examples
 
@@ -143,13 +153,13 @@ Example log configuration strings:
   therefore be discarded, even though they are enabled for one of its parent
   categories.
 
-* `ERROR:stderr, folly=INFO; stderr=stream,stream=stderr`
+* `ERROR:stderr, folly=INFO; stderr=stream:stream=stderr`
 
   Sets the root log category level to ERROR, and sets its handler list to
   use the "stderr" handler.  Sets the folly log level to INFO.  Defines
   a log handler named "stderr" which writes to stderr.
 
-* `ERROR:x,folly=INFO:y;x=stream,stream=stderr;y=file,path=/tmp/y.log`
+* `ERROR:x,folly=INFO:y;x=stream:stream=stderr;y=file:path=/tmp/y.log`
 
   Defines two log handlers: "x" which writes to stderr and "y" which
   writes to the file /tmp/y.log
@@ -157,7 +167,7 @@ Example log configuration strings:
   "x" handler.  Sets the log level for the "folly" category to INFO and
   configures it to use the "y" handler.
 
-* `ERROR:default:x; default=stream,stream=stderr; x=file,path=/tmp/x.log`
+* `ERROR:default:x; default=stream:stream=stderr; x=file:path=/tmp/x.log`
 
   Defines two log handlers: "default" which writes to stderr and "x" which
   writes to the file /tmp/x.log
@@ -172,13 +182,21 @@ Example log configuration strings:
   to the empty list.  Not specifying handler information at all (no ':')
   will leave any pre-existing handlers as-is.
 
-* `;default=stream,stream=stdout`
+* `;default=stream:stream=stdout`
 
   Does not change any log category settings, and defines a "default" handler
   that writes to stdout.  This format is useful to update log handler settings
   if the "default" handler already exists and is attached to existing log
   categories.
 
+* `ERROR; stderr:async=true`
+
+  Sets the root log category level to ERR, and sets the "async" property to
+  true on the "stderr" handler.  A log handler named "stderr" must already
+  exist.  Therefore this configuration string is only valid to use with
+  `LoggerDB::updateConfig()`, and cannot be used with
+  `LoggerDB::resetConfig()`.
+
 
 JSON Configuration Syntax
 -------------------------
@@ -232,9 +250,14 @@ following fields:
 
 * `type`
 
-  This field is required.  It should be a string containing the name of the log
-  handler type.  This type name must correspond to `LogHandlerFactory` type
-  registered with the `LoggerDB`.
+  This field should be a string containing the name of the log handler type.
+  This type name must correspond to `LogHandlerFactory` type registered with
+  the `LoggerDB`.
+
+  If this field is not present then this configuration will be used to update
+  an existing log handler.  A log handler with this name must already exist.
+  The values from the `options` field will be merged into the existing log
+  handler options.
 
 * `options`
 

folly/experimental/logging/test/ConfigParserTest.cpp

@@ -41,7 +41,7 @@ std::ostream& operator<<(std::ostream& os, const LogCategoryConfig& config) {
 }
 
 std::ostream& operator<<(std::ostream& os, const LogHandlerConfig& config) {
-  os << config.type;
+  os << (config.type ? config.type.value() : "[no type]");
   bool first = true;
   for (const auto& opt : config.options) {
     if (!first) {
@@ -106,7 +106,7 @@ TEST(LogConfig, parseBasic) {
           Pair("", LogCategoryConfig{LogLevel::ERR, true, {}})));
   EXPECT_THAT(config.getHandlerConfigs(), UnorderedElementsAre());
 
-  config = parseLogConfig(" ERR:stderr; stderr=file,stream=stderr ");
+  config = parseLogConfig(" ERR:stderr; stderr=stream:stream=stderr ");
   EXPECT_THAT(
       config.getCategoryConfigs(),
       UnorderedElementsAre(
@@ -114,12 +114,12 @@ TEST(LogConfig, parseBasic) {
   EXPECT_THAT(
       config.getHandlerConfigs(),
       UnorderedElementsAre(
-          Pair("stderr", LogHandlerConfig{"file", {{"stream", "stderr"}}})));
+          Pair("stderr", LogHandlerConfig{"stream", {{"stream", "stderr"}}})));
 
   config = parseLogConfig(
       "ERR:myfile:custom, folly=DBG2, folly.io:=WARN:other;"
-      "myfile=file,path=/tmp/x.log; "
-      "custom=custom,foo=bar,hello=world,a = b = c; "
+      "myfile=file:path=/tmp/x.log; "
+      "custom=custom:foo=bar,hello=world,a = b = c; "
       "other=custom2");
   EXPECT_THAT(
       config.getCategoryConfigs(),
@@ -141,8 +141,36 @@ TEST(LogConfig, parseBasic) {
                   {{"foo", "bar"}, {"hello", "world"}, {"a", "b = c"}}}),
           Pair("other", LogHandlerConfig{"custom2"})));
 
+  // Test updating existing handler configs, with no handler type
+  config = parseLogConfig("ERR;foo");
+  EXPECT_THAT(
+      config.getCategoryConfigs(),
+      UnorderedElementsAre(Pair("", LogCategoryConfig{LogLevel::ERR, true})));
+  EXPECT_THAT(
+      config.getHandlerConfigs(),
+      UnorderedElementsAre(Pair("foo", LogHandlerConfig{})));
+
+  config = parseLogConfig("ERR;foo:a=b,c=d");
+  EXPECT_THAT(
+      config.getCategoryConfigs(),
+      UnorderedElementsAre(Pair("", LogCategoryConfig{LogLevel::ERR, true})));
+  EXPECT_THAT(
+      config.getHandlerConfigs(),
+      UnorderedElementsAre(Pair(
+          "foo", LogHandlerConfig{folly::none, {{"a", "b"}, {"c", "d"}}})));
+
+  config = parseLogConfig("ERR;test=file:path=/tmp/test.log;foo:a=b,c=d");
+  EXPECT_THAT(
+      config.getCategoryConfigs(),
+      UnorderedElementsAre(Pair("", LogCategoryConfig{LogLevel::ERR, true})));
+  EXPECT_THAT(
+      config.getHandlerConfigs(),
+      UnorderedElementsAre(
+          Pair("foo", LogHandlerConfig{folly::none, {{"a", "b"}, {"c", "d"}}}),
+          Pair("test", LogHandlerConfig{"file", {{"path", "/tmp/test.log"}}})));
+
   // Log handler changes with no category changes
-  config = parseLogConfig("; myhandler=custom,foo=bar");
+  config = parseLogConfig("; myhandler=custom:foo=bar");
   EXPECT_THAT(config.getCategoryConfigs(), UnorderedElementsAre());
   EXPECT_THAT(
       config.getHandlerConfigs(),
@@ -219,13 +247,7 @@ TEST(LogConfig, parseBasicErrors) {
   EXPECT_THROW_RE(
       parseLogConfig("ERR;"),
       LogConfigParseError,
-      "error parsing log handler configuration \"\": "
-      "expected data in the form NAME=TYPE");
-  EXPECT_THROW_RE(
-      parseLogConfig("ERR;foo"),
-      LogConfigParseError,
-      "error parsing log handler configuration \"foo\": "
-      "expected data in the form NAME=TYPE");
+      "error parsing log handler configuration: empty log handler name");
   EXPECT_THROW_RE(
       parseLogConfig("ERR;foo="),
       LogConfigParseError,
@@ -238,8 +260,18 @@ TEST(LogConfig, parseBasicErrors) {
   EXPECT_THROW_RE(
       parseLogConfig("ERR;handler1=file;"),
       LogConfigParseError,
-      "error parsing log handler configuration \"\": "
-      "expected data in the form NAME=TYPE");
+      "error parsing log handler configuration: empty log handler name");
+  EXPECT_THROW_RE(
+      parseLogConfig("ERR;test=file,path=/tmp/test.log;foo:a=b,c=d"),
+      LogConfigParseError,
+      "error parsing configuration for log handler \"test\": "
+      "invalid type \"file,path=/tmp/test.log\": type name cannot contain "
+      "a comma when using the basic config format");
+  EXPECT_THROW_RE(
+      parseLogConfig("ERR;test,path=/tmp/test.log;foo:a=b,c=d"),
+      LogConfigParseError,
+      "error parsing configuration for log handler \"test,path\": "
+      "name cannot contain a comma when using the basic config format");
 }
 
 TEST(LogConfig, parseJson) {
@@ -545,7 +577,7 @@ TEST(LogConfig, toJson) {
 
   config = parseLogConfig(
       "ERROR:h1,foo.bar:=FATAL,folly=INFO:; "
-      "h1=custom,foo=bar");
+      "h1=custom:foo=bar");
   expectedJson = folly::parseJson(R"JSON({
   "categories" : {
     "" : {
@@ -584,7 +616,7 @@ TEST(LogConfig, mergeConfigs) {
   EXPECT_THAT(config.getHandlerConfigs(), UnorderedElementsAre());
 
   config =
-      parseLogConfig("WARN:default; default=custom,opt1=value1,opt2=value2");
+      parseLogConfig("WARN:default; default=custom:opt1=value1,opt2=value2");
   config.update(parseLogConfig("folly.io=DBG2,foo=INFO"));
   EXPECT_THAT(
       config.getCategoryConfigs(),
@@ -602,7 +634,7 @@ TEST(LogConfig, mergeConfigs) {
   // Updating the root category's log level without specifying
   // handlers should leave its current handler list intact
   config =
-      parseLogConfig("WARN:default; default=custom,opt1=value1,opt2=value2");
+      parseLogConfig("WARN:default; default=custom:opt1=value1,opt2=value2");
   config.update(parseLogConfig("ERR"));
   EXPECT_THAT(
       config.getCategoryConfigs(),
@@ -616,7 +648,7 @@ TEST(LogConfig, mergeConfigs) {
               "custom", {{"opt1", "value1"}, {"opt2", "value2"}}))));
 
   config =
-      parseLogConfig("WARN:default; default=custom,opt1=value1,opt2=value2");
+      parseLogConfig("WARN:default; default=custom:opt1=value1,opt2=value2");
   config.update(parseLogConfig(".:=ERR"));
   EXPECT_THAT(
       config.getCategoryConfigs(),
@@ -631,7 +663,7 @@ TEST(LogConfig, mergeConfigs) {
 
   // Test clearing the root category's log handlers
   config =
-      parseLogConfig("WARN:default; default=custom,opt1=value1,opt2=value2");
+      parseLogConfig("WARN:default; default=custom:opt1=value1,opt2=value2");
   config.update(parseLogConfig("FATAL:"));
   EXPECT_THAT(
       config.getCategoryConfigs(),
@@ -643,4 +675,28 @@ TEST(LogConfig, mergeConfigs) {
           "default",
           LogHandlerConfig(
               "custom", {{"opt1", "value1"}, {"opt2", "value2"}}))));
+
+  // Test updating the settings on a log handler
+  config =
+      parseLogConfig("WARN:default; default=stream:stream=stderr,async=false");
+  config.update(parseLogConfig("INFO; default:async=true"));
+  EXPECT_THAT(
+      config.getCategoryConfigs(),
+      UnorderedElementsAre(
+          Pair("", LogCategoryConfig{LogLevel::INFO, true, {"default"}})));
+  EXPECT_THAT(
+      config.getHandlerConfigs(),
+      UnorderedElementsAre(Pair(
+          "default",
+          LogHandlerConfig(
+              "stream", {{"stream", "stderr"}, {"async", "true"}}))));
+
+  // Updating the settings for a non-existent log handler should fail
+  config =
+      parseLogConfig("WARN:default; default=stream:stream=stderr,async=false");
+  EXPECT_THROW_RE(
+      config.update(parseLogConfig("INFO; other:async=true")),
+      std::invalid_argument,
+      "cannot update configuration for "
+      "unknown log handler \"other\"");
 }

folly/experimental/logging/test/fatal_test.py

@@ -69,12 +69,12 @@ class FatalTests(unittest.TestCase):
         subprocess.check_output([self.helper, '--crash=no'])
 
     def test_async(self):
-        handler_setings = 'default=stream,stream=stderr,async=true'
+        handler_setings = 'default=stream:stream=stderr,async=true'
         err = self.run_helper('--logging=;' + handler_setings)
         self.assertRegex(err, self.get_crash_regex())
 
     def test_immediate(self):
-        handler_setings = 'default=stream,stream=stderr,async=false'
+        handler_setings = 'default=stream:stream=stderr,async=false'
         err = self.run_helper('--logging=;' + handler_setings)
         self.assertRegex(err, self.get_crash_regex())