SERVER-13379 Canonicalize verbose and v options as systemLog.verbosity

2 files changed, 74 insertions, 48 deletions

diff --git a/src/mongo/db/server_options_helpers.cpp b/src/mongo/db/server_options_helpers.cpp
index c3285616a26..db9e31fe01b 100644
--- a/src/mongo/db/server_options_helpers.cpp
+++ b/src/mongo/db/server_options_helpers.cpp
@@ -140,13 +140,10 @@ namespace {
         // v=true              | 1
         // vv=true             | 2 (etc.)
         //
-        // JSON Config Option  | Resulting Verbosity
+        // YAML Config Option  | Resulting Verbosity
         // _________________________________________
-        // { "verbose" : "" }  | 0
-        // { "verbose" : "v" } | 1
-        // { "verbose" : "vv" }| 2 (etc.)
-        // { "v" : true }      | 1
-        // { "vv" : true }     | 2 (etc.)
+        // systemLog:          |
+        //    verbosity: 5     | 5
         options->addOptionChaining("verbose", "verbose,v", moe::String,
                 "be more verbose (include multiple times for more verbosity e.g. -vvvvv)")
                                   .setImplicit(moe::Value(std::string("v")))
@@ -385,8 +382,21 @@ namespace {
     }
 
     Status validateServerOptions(const moe::Environment& params) {
-        // This function should contain custom validation that cannot be expressed as simple
-        // constraints on the options.
+        if (params.count("verbose")) {
+            std::string verbosity = params["verbose"].as<std::string>();
+
+            // Skip this for backwards compatibility.  See SERVER-11471.
+            if (verbosity != "true") {
+                for (std::string::iterator iterator = verbosity.begin();
+                    iterator != verbosity.end(); iterator++) {
+                    if (*iterator != 'v') {
+                        return Status(ErrorCodes::BadValue,
+                                      "The \"verbose\" option string cannot contain any characters "
+                                      "other than \"v\"");
+                    }
+                }
+            }
+        }
 
         return Status::OK();
     }
@@ -453,6 +463,42 @@ namespace {
             }
         }
 
+        // Handle both the "--verbose" string argument and the "-vvvv" arguments at the same time so
+        // that we ensure that we set the log level to the maximum of the options provided
+        int logLevel = -1;
+        for (std::string s = ""; s.length() <= 14; s.append("v")) {
+            if (!s.empty() && params->count(s)) {
+                logLevel = s.length();
+            }
+
+            if (params->count("verbose")) {
+                std::string verbosity;
+                params->get("verbose", &verbosity);
+                if (s == verbosity ||
+                    // Treat a verbosity of "true" the same as a single "v".  See SERVER-11471.
+                    (s == "v" && verbosity == "true")) {
+                    logLevel = s.length();
+                }
+            }
+
+            // Remove all "v" options we have already handled
+            Status ret = params->remove(s);
+            if (!ret.isOK()) {
+                return ret;
+            }
+        }
+
+        if (logLevel != -1) {
+            Status ret = params->set("systemLog.verbosity", moe::Value(logLevel));
+            if (!ret.isOK()) {
+                return ret;
+            }
+            ret = params->remove("verbose");
+            if (!ret.isOK()) {
+                return ret;
+            }
+        }
+
         return Status::OK();
     }
 
@@ -485,53 +531,17 @@ namespace {
                           "The net.http.port option is not currently supported");
         }
 
-        if (params.count("verbose")) {
-            std::string verbosity = params["verbose"].as<std::string>();
-
-            // Skip this for backwards compatibility.  See SERVER-11471.
-            if (verbosity != "true") {
-                for (std::string::iterator iterator = verbosity.begin();
-                    iterator != verbosity.end(); iterator++) {
-                    if (*iterator != 'v') {
-                        return Status(ErrorCodes::BadValue,
-                                      "The \"verbose\" option string cannot contain any characters "
-                                      "other than \"v\"");
-                    }
-                }
-            }
-        }
-
-        // Handle the JSON config file verbosity setting first so that it gets overriden by the
-        // setting on the command line
         if (params.count("systemLog.verbosity")) {
             int verbosity = params["systemLog.verbosity"].as<int>();
             if (verbosity < 0) {
+                // This can only happen in YAML config
                 return Status(ErrorCodes::BadValue,
-                                "systemLog.verbosity in JSON Config cannot be negative");
+                              "systemLog.verbosity YAML Config cannot be negative");
             }
             logger::globalLogDomain()->setMinimumLoggedSeverity(
                     logger::LogSeverity::Debug(verbosity));
         }
 
-        // Handle both the "--verbose" string argument and the "-vvvv" arguments at the same time so
-        // that we ensure that we set the log level to the maximum of the options provided
-        for (string s = ""; s.length() <= 14; s.append("v")) {
-            if (!s.empty() && params.count(s)) {
-                logger::globalLogDomain()->setMinimumLoggedSeverity(
-                        logger::LogSeverity::Debug(s.length()));
-            }
-
-            if (params.count("verbose")) {
-                std::string verbosity = params["verbose"].as<std::string>();
-                if (s == verbosity ||
-                    // Treat a verbosity of "true" the same as a single "v".  See SERVER-11471.
-                    (s == "v" && verbosity == "true")) {
-                    logger::globalLogDomain()->setMinimumLoggedSeverity(
-                            logger::LogSeverity::Debug(s.length()));
-                }
-            }
-        }
-
         if (params.count("enableExperimentalIndexStatsCmd")) {
             serverGlobalParams.experimental.indexStatsCmdEnabled = true;
         }
diff --git a/src/mongo/db/server_options_test.cpp b/src/mongo/db/server_options_test.cpp
index c12d47a0080..df84f80360b 100644
--- a/src/mongo/db/server_options_test.cpp
+++ b/src/mongo/db/server_options_test.cpp
@@ -67,6 +67,8 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
+        ASSERT_OK(::mongo::validateServerOptions(environment));
+        ASSERT_OK(::mongo::canonicalizeServerOptions(&environment));
         ASSERT_OK(::mongo::storeServerOptions(environment, argv));
 
         // Make sure the log level didn't change since we didn't specify any verbose options
@@ -92,6 +94,8 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
+        ASSERT_OK(::mongo::validateServerOptions(environment));
+        ASSERT_OK(::mongo::canonicalizeServerOptions(&environment));
         ASSERT_OK(::mongo::storeServerOptions(environment, argv));
 
         int verbosity = 1;
@@ -118,6 +122,8 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
+        ASSERT_OK(::mongo::validateServerOptions(environment));
+        ASSERT_OK(::mongo::canonicalizeServerOptions(&environment));
         ASSERT_OK(::mongo::storeServerOptions(environment, argv));
 
         int verbosity = 4;
@@ -144,6 +150,8 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
+        ASSERT_OK(::mongo::validateServerOptions(environment));
+        ASSERT_OK(::mongo::canonicalizeServerOptions(&environment));
         ASSERT_OK(::mongo::storeServerOptions(environment, argv));
 
         int verbosity = 0;
@@ -170,7 +178,7 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
-        ASSERT_NOT_OK(::mongo::storeServerOptions(environment, argv));
+        ASSERT_NOT_OK(::mongo::validateServerOptions(environment));
     }
 
     TEST(Verbosity, INIConfigString) {
@@ -194,6 +202,8 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
+        ASSERT_OK(::mongo::validateServerOptions(environment));
+        ASSERT_OK(::mongo::canonicalizeServerOptions(&environment));
         ASSERT_OK(::mongo::storeServerOptions(environment, argv));
 
         int verbosity = 4;
@@ -222,7 +232,7 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
-        ASSERT_NOT_OK(::mongo::storeServerOptions(environment, argv));
+        ASSERT_NOT_OK(::mongo::validateServerOptions(environment));
     }
 
     TEST(Verbosity, INIConfigEmptyString) {
@@ -246,6 +256,8 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
+        ASSERT_OK(::mongo::validateServerOptions(environment));
+        ASSERT_OK(::mongo::canonicalizeServerOptions(&environment));
         ASSERT_OK(::mongo::storeServerOptions(environment, argv));
 
         int verbosity = 0;
@@ -274,6 +286,8 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
+        ASSERT_OK(::mongo::validateServerOptions(environment));
+        ASSERT_OK(::mongo::canonicalizeServerOptions(&environment));
         ASSERT_OK(::mongo::storeServerOptions(environment, argv));
 
         int verbosity = 4;
@@ -304,6 +318,8 @@ namespace {
 
         ASSERT_OK(parser.run(options, argv, env_map, &environment));
 
+        ASSERT_OK(::mongo::validateServerOptions(environment));
+        ASSERT_OK(::mongo::canonicalizeServerOptions(&environment));
         ASSERT_OK(::mongo::storeServerOptions(environment, argv));
 
         int verbosity = 3;