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

diff --git a/folly/experimental/logging/Init.cpp b/folly/experimental/logging/Init.cpp
index 24600510da59d04813160cc498f95032f660b58c..73542a8e0500c2c35cf6c78f911ea40c58d6aa0f 100644 (file)
--- a/folly/experimental/logging/Init.cpp
+++ b/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 =
  * 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
 
 void initLogging(StringPiece configString) {
   // Register the StreamHandlerFactory

diff --git a/folly/experimental/logging/LogConfig.cpp b/folly/experimental/logging/LogConfig.cpp
index a622f2813cd5d1e93acb9dad7080a9bf5104f636..7a35c9ada58b7b1f29a92cfc4abec8c66f7c9d98 100644 (file)
--- a/folly/experimental/logging/LogConfig.cpp
+++ b/folly/experimental/logging/LogConfig.cpp
@@ -15,6 +15,8 @@
  */
 #include <folly/experimental/logging/LogConfig.h>
 
  */
 #include <folly/experimental/logging/LogConfig.h>
 

+#include <folly/Conv.h>
+

 namespace folly {
 
 bool LogConfig::operator==(const LogConfig& other) const {
 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_) {
   // 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);

     }
   }
 
     }
   }
 

diff --git a/folly/experimental/logging/LogConfigParser.cpp b/folly/experimental/logging/LogConfigParser.cpp
index 5eb805ac23610b0eb66d32dadf8c9309973955c9..ac9f5bdcbf314c9be766a840cdc04e6c6a75e543 100644 (file)
--- a/folly/experimental/logging/LogConfigParser.cpp
+++ b/folly/experimental/logging/LogConfigParser.cpp
@@ -357,49 +357,84 @@ bool splitNameValue(
 }
 
 std::pair<std::string, LogHandlerConfig> parseHandlerConfig(StringPiece value) {
 }
 
 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 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 (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,
     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,
       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,
       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);
   }
   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) {
 }
 
 dynamic logConfigToDynamic(const LogCategoryConfig& config) {

diff --git a/folly/experimental/logging/LogHandlerConfig.cpp b/folly/experimental/logging/LogHandlerConfig.cpp
index 009f82077db65ada5ae32802ff1f3cc38376e89e..74fe05d9c40bc8ac5309f6177202a1476ff56385 100644 (file)
--- a/folly/experimental/logging/LogHandlerConfig.cpp
+++ b/folly/experimental/logging/LogHandlerConfig.cpp
@@ -15,13 +15,34 @@
  */
 #include <folly/experimental/logging/LogHandlerConfig.h>
 
  */
 #include <folly/experimental/logging/LogHandlerConfig.h>
 

+#include <folly/lang/SafeAssert.h>
+
+using std::string;
+

 namespace folly {
 
 namespace folly {
 

+LogHandlerConfig::LogHandlerConfig() {}
+

 LogHandlerConfig::LogHandlerConfig(StringPiece t) : type{t.str()} {}
 
 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(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;
 }
 bool LogHandlerConfig::operator==(const LogHandlerConfig& other) const {
   return type == other.type && options == other.options;
 }

diff --git a/folly/experimental/logging/LogHandlerConfig.h b/folly/experimental/logging/LogHandlerConfig.h
index 4fe3a3fe823239b003ff6df1234dd9f15df31186..698d981bb8bdb8b5eb1e26f48985278ab4b6da08 100644 (file)
--- a/folly/experimental/logging/LogHandlerConfig.h
+++ b/folly/experimental/logging/LogHandlerConfig.h
@@ -18,6 +18,7 @@
 #include <string>
 #include <unordered_map>
 
 #include <string>
 #include <unordered_map>
 

+#include <folly/Optional.h>

 #include <folly/Range.h>
 
 namespace folly {
 #include <folly/Range.h>
 
 namespace folly {


@@ -29,13 +30,32 @@ class LogHandlerConfig {
  public:
   using Options = std::unordered_map<std::string, std::string>;
 
  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;
 
 
   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;
 };
 
   Options options;
 };
 

diff --git a/folly/experimental/logging/LoggerDB.cpp b/folly/experimental/logging/LoggerDB.cpp
index 1f58c047e8d2ef53fe4751f6b254cbad7ab43d6a..bd64d257b68c19c2db9dabdfda94238d788701f3 100644 (file)
--- a/folly/experimental/logging/LoggerDB.cpp
+++ b/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()) {
 
   // 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);
     // 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;
     }
 
       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) {
     // 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 {
         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
       }
     } catch (const std::exception& ex) {
       // Errors creating or updating the the log handler are generally due to

diff --git a/folly/experimental/logging/docs/Config.md b/folly/experimental/logging/docs/Config.md
index 482a9890876448bf4d2b1333d4e5fb1772911f4a..1816f3ad5c92aa85a7effad039d485f5351dac35 100644 (file)
--- a/folly/experimental/logging/docs/Config.md
+++ b/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_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>
 
 <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
 
 
 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.
 
 
 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
 
 
 ### Examples
 


@@ -143,13 +153,13 @@ Example log configuration strings:
   therefore be discarded, even though they are enabled for one of its parent
   categories.
 
   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.
 
 
   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
 
   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.
 
   "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
 
   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.
 
   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.
 
 
   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
 -------------------------
 
 JSON Configuration Syntax
 -------------------------


@@ -232,9 +250,14 @@ following fields:
 
 * `type`
 
 
 * `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`
 
 
 * `options`
 

diff --git a/folly/experimental/logging/test/ConfigParserTest.cpp b/folly/experimental/logging/test/ConfigParserTest.cpp
index 6eef6bcdba8c0f2b7e4f9a3efac617005b57cccc..6ef2d1bca702f55cc23810ded085c0742d836baf 100644 (file)
--- a/folly/experimental/logging/test/ConfigParserTest.cpp
+++ b/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) {
 }
 
 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) {
   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());
 
           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(
   EXPECT_THAT(
       config.getCategoryConfigs(),
       UnorderedElementsAre(


@@ -114,12 +114,12 @@ TEST(LogConfig, parseBasic) {
   EXPECT_THAT(
       config.getHandlerConfigs(),
       UnorderedElementsAre(
   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;"
 
   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(),
       "other=custom2");
   EXPECT_THAT(
       config.getCategoryConfigs(),


@@ -141,8 +141,36 @@ TEST(LogConfig, parseBasic) {
                   {{"foo", "bar"}, {"hello", "world"}, {"a", "b = c"}}}),
           Pair("other", LogHandlerConfig{"custom2"})));
 
                   {{"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
   // 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(),
   EXPECT_THAT(config.getCategoryConfigs(), UnorderedElementsAre());
   EXPECT_THAT(
       config.getHandlerConfigs(),


@@ -219,13 +247,7 @@ TEST(LogConfig, parseBasicErrors) {
   EXPECT_THROW_RE(
       parseLogConfig("ERR;"),
       LogConfigParseError,
   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,
   EXPECT_THROW_RE(
       parseLogConfig("ERR;foo="),
       LogConfigParseError,


@@ -238,8 +260,18 @@ TEST(LogConfig, parseBasicErrors) {
   EXPECT_THROW_RE(
       parseLogConfig("ERR;handler1=file;"),
       LogConfigParseError,
   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) {
 }
 
 TEST(LogConfig, parseJson) {


@@ -545,7 +577,7 @@ TEST(LogConfig, toJson) {
 
   config = parseLogConfig(
       "ERROR:h1,foo.bar:=FATAL,folly=INFO:; "
 
   config = parseLogConfig(
       "ERROR:h1,foo.bar:=FATAL,folly=INFO:; "

-      "h1=custom,foo=bar");
+      "h1=custom:foo=bar");

   expectedJson = folly::parseJson(R"JSON({
   "categories" : {
     "" : {
   expectedJson = folly::parseJson(R"JSON({
   "categories" : {
     "" : {


@@ -584,7 +616,7 @@ TEST(LogConfig, mergeConfigs) {
   EXPECT_THAT(config.getHandlerConfigs(), UnorderedElementsAre());
 
   config =
   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(),
   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 =
   // 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(),
   config.update(parseLogConfig("ERR"));
   EXPECT_THAT(
       config.getCategoryConfigs(),


@@ -616,7 +648,7 @@ TEST(LogConfig, mergeConfigs) {
               "custom", {{"opt1", "value1"}, {"opt2", "value2"}}))));
 
   config =
               "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(),
   config.update(parseLogConfig(".:=ERR"));
   EXPECT_THAT(
       config.getCategoryConfigs(),


@@ -631,7 +663,7 @@ TEST(LogConfig, mergeConfigs) {
 
   // Test clearing the root category's log handlers
   config =
 
   // 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(),
   config.update(parseLogConfig("FATAL:"));
   EXPECT_THAT(
       config.getCategoryConfigs(),


@@ -643,4 +675,28 @@ TEST(LogConfig, mergeConfigs) {
           "default",
           LogHandlerConfig(
               "custom", {{"opt1", "value1"}, {"opt2", "value2"}}))));
           "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\"");

 }
 }

diff --git a/folly/experimental/logging/test/ConfigUpdateTest.cpp b/folly/experimental/logging/test/ConfigUpdateTest.cpp
index d0743ced71247c06170769140c5b4cde141ca198..ad48453d139844f3210b5205c7d3323ca5af99da 100644 (file)
--- a/folly/experimental/logging/test/ConfigUpdateTest.cpp
+++ b/folly/experimental/logging/test/ConfigUpdateTest.cpp
@@ -64,7 +64,7 @@ std::ostream& operator<<(
   }
 
   auto config = configHandler->getConfig();
   }
 
   auto config = configHandler->getConfig();

-  os << "ConfigHandler(" << config.type;
+  os << "ConfigHandler(" << (config.type ? config.type.value() : "[no type]");

   for (const auto& entry : config.options) {
     os << ", " << entry.first << "=" << entry.second;
   }
   for (const auto& entry : config.options) {
     os << ", " << entry.first << "=" << entry.second;
   }


@@ -125,14 +125,14 @@ TEST(ConfigUpdate, updateConfig) {
   EXPECT_EQ(parseLogConfig(".:=ERROR:"), db.getConfig());
 
   // Apply an update
   EXPECT_EQ(parseLogConfig(".:=ERROR:"), db.getConfig());
 
   // Apply an update

-  db.updateConfig(parseLogConfig("INFO:stderr; stderr=handlerA,stream=stderr"));
+  db.updateConfig(parseLogConfig("INFO:stderr; stderr=handlerA:stream=stderr"));

   EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
   EXPECT_THAT(
       db.getCategory("")->getHandlers(),
       UnorderedElementsAre(
           MatchLogHandler("handlerA", {{"stream", "stderr"}})));
   EXPECT_EQ(
   EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
   EXPECT_THAT(
       db.getCategory("")->getHandlers(),
       UnorderedElementsAre(
           MatchLogHandler("handlerA", {{"stream", "stderr"}})));
   EXPECT_EQ(

-      parseLogConfig(".:=INFO:stderr; stderr=handlerA,stream=stderr"),
+      parseLogConfig(".:=INFO:stderr; stderr=handlerA:stream=stderr"),

       db.getConfig());
 
   // Update the log level for category "foo"
       db.getConfig());
 
   // Update the log level for category "foo"


@@ -146,14 +146,14 @@ TEST(ConfigUpdate, updateConfig) {
   EXPECT_EQ(1, db.getCategory("")->getHandlers().size());
   EXPECT_EQ(
       parseLogConfig(
   EXPECT_EQ(1, db.getCategory("")->getHandlers().size());
   EXPECT_EQ(
       parseLogConfig(

-          ".:=INFO:stderr, foo:=DBG2:; stderr=handlerA,stream=stderr"),
+          ".:=INFO:stderr, foo:=DBG2:; stderr=handlerA:stream=stderr"),

       db.getConfig());
 
   // Add 2 log handlers to the "bar" log category.
   db.updateConfig(
       parseLogConfig("bar=ERROR:new:h2; "
       db.getConfig());
 
   // Add 2 log handlers to the "bar" log category.
   db.updateConfig(
       parseLogConfig("bar=ERROR:new:h2; "

-                     "new=handlerB,key=value; "
-                     "h2=handlerA,foo=bar"));
+                     "new=handlerB:key=value; "
+                     "h2=handlerA:foo=bar"));

   EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
   EXPECT_THAT(
       db.getCategory("")->getHandlers(),
   EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
   EXPECT_THAT(
       db.getCategory("")->getHandlers(),


@@ -167,15 +167,15 @@ TEST(ConfigUpdate, updateConfig) {
           MatchLogHandler("handlerA", {{"foo", "bar"}})));
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=ERROR:new:h2; "
           MatchLogHandler("handlerA", {{"foo", "bar"}})));
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=ERROR:new:h2; "

-                     "stderr=handlerA,stream=stderr; "
-                     "new=handlerB,key=value; "
-                     "h2=handlerA,foo=bar"),
+                     "stderr=handlerA: stream=stderr; "
+                     "new=handlerB: key=value; "
+                     "h2=handlerA: foo=bar"),

       db.getConfig());
 
   // Updating the "new" log handler settings should automatically update
   // the settings we see on the "bar" category, even if we don't explicitly
   // list "bar" in the config update
       db.getConfig());
 
   // Updating the "new" log handler settings should automatically update
   // the settings we see on the "bar" category, even if we don't explicitly
   // list "bar" in the config update

-  db.updateConfig(parseLogConfig("; new=handlerB,newkey=newvalue"));
+  db.updateConfig(parseLogConfig("; new=handlerB:newkey=newvalue"));

   EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
   EXPECT_THAT(
       db.getCategory("")->getHandlers(),
   EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
   EXPECT_THAT(
       db.getCategory("")->getHandlers(),


@@ -189,9 +189,9 @@ TEST(ConfigUpdate, updateConfig) {
           MatchLogHandler("handlerA", {{"foo", "bar"}})));
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=ERROR:new:h2; "
           MatchLogHandler("handlerA", {{"foo", "bar"}})));
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=ERROR:new:h2; "

-                     "stderr=handlerA,stream=stderr; "
-                     "new=handlerB,newkey=newvalue; "
-                     "h2=handlerA,foo=bar"),
+                     "stderr=handlerA: stream=stderr; "
+                     "new=handlerB: newkey=newvalue; "
+                     "h2=handlerA: foo=bar"),

       db.getConfig());
 
   // Updating the level settings for the "bar" handler should leave its
       db.getConfig());
 
   // Updating the level settings for the "bar" handler should leave its


@@ -210,16 +210,16 @@ TEST(ConfigUpdate, updateConfig) {
           MatchLogHandler("handlerA", {{"foo", "bar"}})));
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=WARN:new:h2; "
           MatchLogHandler("handlerA", {{"foo", "bar"}})));
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=WARN:new:h2; "

-                     "stderr=handlerA,stream=stderr; "
-                     "new=handlerB,newkey=newvalue; "
-                     "h2=handlerA,foo=bar"),
+                     "stderr=handlerA: stream=stderr; "
+                     "new=handlerB: newkey=newvalue; "
+                     "h2=handlerA: foo=bar"),

       db.getConfig());
 
   // Update the options for the h2 handler in place, and also add it to the
   // "test.foo" category.  The changes should also be reflected on the "bar"
   // category.
   db.updateConfig(
       db.getConfig());
 
   // Update the options for the h2 handler in place, and also add it to the
   // "test.foo" category.  The changes should also be reflected on the "bar"
   // category.
   db.updateConfig(

-      parseLogConfig("test.foo=INFO:h2; h2=handlerA,reuse_handler=1,foo=xyz"));
+      parseLogConfig("test.foo=INFO:h2; h2=handlerA:reuse_handler=1,foo=xyz"));

   EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
   EXPECT_THAT(
       db.getCategory("")->getHandlers(),
   EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
   EXPECT_THAT(
       db.getCategory("")->getHandlers(),


@@ -240,11 +240,71 @@ TEST(ConfigUpdate, updateConfig) {
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=WARN:new:h2, "
                      "test.foo=INFO:h2; "
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=WARN:new:h2, "
                      "test.foo=INFO:h2; "

-                     "stderr=handlerA,stream=stderr; "
-                     "new=handlerB,newkey=newvalue; "
-                     "h2=handlerA,reuse_handler=1,foo=xyz"),
+                     "stderr=handlerA: stream=stderr; "
+                     "new=handlerB: newkey=newvalue; "
+                     "h2=handlerA: reuse_handler=1,foo=xyz"),

       db.getConfig());
 
       db.getConfig());
 

+  // Update the options for the h2 handler using partial options
+  db.updateConfig(parseLogConfig("; h2:abc=def"));
+  EXPECT_EQ(LogLevel::INFO, db.getCategory("")->getLevel());
+  EXPECT_THAT(
+      db.getCategory("")->getHandlers(),
+      UnorderedElementsAre(
+          MatchLogHandler("handlerA", {{"stream", "stderr"}})));
+  EXPECT_EQ(LogLevel::WARN, db.getCategory("bar")->getLevel());
+  EXPECT_THAT(
+      db.getCategory("bar")->getHandlers(),
+      UnorderedElementsAre(
+          MatchLogHandler("handlerB", {{"newkey", "newvalue"}}),
+          MatchLogHandler(
+              "handlerA",
+              {{"abc", "def"}, {"foo", "xyz"}, {"reuse_handler", "1"}})));
+  EXPECT_EQ(LogLevel::INFO, db.getCategory("test.foo")->getLevel());
+  EXPECT_THAT(
+      db.getCategory("test.foo")->getHandlers(),
+      UnorderedElementsAre(MatchLogHandler(
+          "handlerA",
+          {{"abc", "def"}, {"foo", "xyz"}, {"reuse_handler", "1"}})));
+  EXPECT_EQ(
+      parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=WARN:new:h2, "
+                     "test.foo=INFO:h2; "
+                     "stderr=handlerA: stream=stderr; "
+                     "new=handlerB: newkey=newvalue; "
+                     "h2=handlerA: reuse_handler=1,abc=def,foo=xyz"),
+      db.getConfig());
+
+  // Update the options for the "new" handler using partial options
+  db.updateConfig(parseLogConfig("; new:opt1=value1"));
+  EXPECT_EQ(LogLevel::WARN, db.getCategory("bar")->getLevel());
+  EXPECT_THAT(
+      db.getCategory("bar")->getHandlers(),
+      UnorderedElementsAre(
+          MatchLogHandler(
+              "handlerB", {{"opt1", "value1"}, {"newkey", "newvalue"}}),
+          MatchLogHandler(
+              "handlerA",
+              {{"abc", "def"}, {"foo", "xyz"}, {"reuse_handler", "1"}})));
+  EXPECT_EQ(LogLevel::INFO, db.getCategory("test.foo")->getLevel());
+  EXPECT_THAT(
+      db.getCategory("test.foo")->getHandlers(),
+      UnorderedElementsAre(MatchLogHandler(
+          "handlerA",
+          {{"abc", "def"}, {"foo", "xyz"}, {"reuse_handler", "1"}})));
+  EXPECT_EQ(
+      parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=WARN:new:h2, "
+                     "test.foo=INFO:h2; "
+                     "stderr=handlerA: stream=stderr; "
+                     "new=handlerB: newkey=newvalue,opt1=value1; "
+                     "h2=handlerA: reuse_handler=1,abc=def,foo=xyz"),
+      db.getConfig());
+
+  // Supplying partial options for a non-existent log handler should fail
+  EXPECT_THROW_RE(
+      db.updateConfig(parseLogConfig("; no_such_handler:foo=bar")),
+      std::invalid_argument,
+      "cannot update unknown log handler \"no_such_handler\"");
+

   // Explicitly clear the handlers for the "bar" category
   // This should remove the "new" handler from the LoggerDB since bar was the
   // only category referring to it.
   // Explicitly clear the handlers for the "bar" category
   // This should remove the "new" handler from the LoggerDB since bar was the
   // only category referring to it.


@@ -259,17 +319,17 @@ TEST(ConfigUpdate, updateConfig) {
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=WARN:, "
                      "test.foo=INFO:h2; "
   EXPECT_EQ(
       parseLogConfig(".:=INFO:stderr, foo:=DBG2:, bar=WARN:, "
                      "test.foo=INFO:h2; "

-                     "stderr=handlerA,stream=stderr; "
-                     "h2=handlerA,reuse_handler=1,foo=xyz"),
+                     "stderr=handlerA: stream=stderr; "
+                     "h2=handlerA: reuse_handler=1,abc=def,foo=xyz"),

       db.getConfig());
 
   // Now test resetConfig()
   db.resetConfig(
       parseLogConfig("bar=INFO:h2, test.abc=DBG3; "
       db.getConfig());
 
   // Now test resetConfig()
   db.resetConfig(
       parseLogConfig("bar=INFO:h2, test.abc=DBG3; "

-                     "h2=handlerB,abc=xyz"));
+                     "h2=handlerB: abc=xyz"));

   EXPECT_EQ(
       parseLogConfig(".:=ERR:, bar=INFO:h2, test.abc=DBG3:; "
   EXPECT_EQ(
       parseLogConfig(".:=ERR:, bar=INFO:h2, test.abc=DBG3:; "

-                     "h2=handlerB,abc=xyz"),
+                     "h2=handlerB: abc=xyz"),

       db.getConfig());
 }
 
       db.getConfig());
 }
 


@@ -289,7 +349,7 @@ TEST(ConfigUpdate, getConfigAnonymousHandlers) {
   db.getCategory("x.y.z")->addHandler(handlerFoo);
   EXPECT_EQ(
       parseLogConfig(".:=ERR:, x.y.z=DBG2:anonymousHandler1; "
   db.getCategory("x.y.z")->addHandler(handlerFoo);
   EXPECT_EQ(
       parseLogConfig(".:=ERR:, x.y.z=DBG2:anonymousHandler1; "

-                     "anonymousHandler1=foo,abc=xyz"),
+                     "anonymousHandler1=foo:abc=xyz"),

       db.getConfig());
 
   // If we attach the same handler to another category it should still only be
       db.getConfig());
 
   // If we attach the same handler to another category it should still only be


@@ -300,20 +360,20 @@ TEST(ConfigUpdate, getConfigAnonymousHandlers) {
       parseLogConfig(".:=ERR:, "
                      "x.y.z=DBG2:anonymousHandler1, "
                      "test.category=DBG1:anonymousHandler1; "
       parseLogConfig(".:=ERR:, "
                      "x.y.z=DBG2:anonymousHandler1, "
                      "test.category=DBG1:anonymousHandler1; "

-                     "anonymousHandler1=foo,abc=xyz"),
+                     "anonymousHandler1=foo:abc=xyz"),

       db.getConfig());
 
   // If we use updateConfig() to explicitly define a handler named
   // "anonymousHandler1", the unnamed handler will be reported as
   // "anonymousHandler2" instead now.
   db.updateConfig(parseLogConfig(
       db.getConfig());
 
   // If we use updateConfig() to explicitly define a handler named
   // "anonymousHandler1", the unnamed handler will be reported as
   // "anonymousHandler2" instead now.
   db.updateConfig(parseLogConfig(

-      "a.b.c=INFO:anonymousHandler1; anonymousHandler1=handlerA,key=value"));
+      "a.b.c=INFO:anonymousHandler1; anonymousHandler1=handlerA:key=value"));

   EXPECT_EQ(
       parseLogConfig(".:=ERR:, "
                      "a.b.c=INFO:anonymousHandler1, "
                      "x.y.z=DBG2:anonymousHandler2, "
                      "test.category=DBG1:anonymousHandler2; "
   EXPECT_EQ(
       parseLogConfig(".:=ERR:, "
                      "a.b.c=INFO:anonymousHandler1, "
                      "x.y.z=DBG2:anonymousHandler2, "
                      "test.category=DBG1:anonymousHandler2; "

-                     "anonymousHandler1=handlerA,key=value; "
-                     "anonymousHandler2=foo,abc=xyz"),
+                     "anonymousHandler1=handlerA: key=value; "
+                     "anonymousHandler2=foo: abc=xyz"),

       db.getConfig());
 }
       db.getConfig());
 }

diff --git a/folly/experimental/logging/test/fatal_test.py b/folly/experimental/logging/test/fatal_test.py
index fa7a2a9ba6419c6efd27fc7a9542b713f18e5e57..bca4fbd6db59942f9444d0834f150d65b06c1723 100644 (file)
--- a/folly/experimental/logging/test/fatal_test.py
+++ b/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):
         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):
         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())
 
         err = self.run_helper('--logging=;' + handler_setings)
         self.assertRegex(err, self.get_crash_regex())