cli: set log levels earlier
authorSimon Marchi <simon.marchi@efficios.com>
Wed, 14 Aug 2019 20:47:26 +0000 (16:47 -0400)
committerPhilippe Proulx <eeppeliteloop@gmail.com>
Sat, 17 Aug 2019 00:12:13 +0000 (20:12 -0400)
The problem this patch is aiming to solve is that the effective log
level is not set when executing bt_config_convert_from_args.  For
example, bt_python_bindings_bt2_log_level is still NONE, only to be set
later.  So if Python plugin fails to be imported during the auto source
discovery phase, nothing will be logged, making debugging quite
difficult.  Any argument passed (top level --log-level, --debug or
--verbose) won't help because they will take effect later, when we call
set_auto_log_levels.

This patch moves the call to set_auto_log_levels earlier, as soon as we
know what the default log level will be.  For all commands but
`convert`, it is after we are done parsing the top-level arguments.  The
default log level will be the most verbose any --debug, --verbose or
--log-level argument passed.

With the `convert` command, it is possible to pass --debug and
--verbose, with the same effect as the top-level equivalents.  So for
this command, we call set_auto_log_levels just after parsing its
arguments.

The --debug argument sets the log level to TRACE and the --verbose
argument sets it to INFO.  If more than one of --debug, --verbose and
--log-level are specified, the most verbose level of all specified
arguments is chosen.

Note that the --log-level argument of the convert command is not the
same as the top-level's --log-level argument.  The argument of the
convert command sets the --log-level of a specific component or
auto-source-discovery input.

The behavior of the environment variables LIBBABELTRACE2_INIT_LOG_LEVEL,
BABELTRACE_PLUGIN_CTF_METADATA_LOG_LEVEL and
BABELTRACE_PYTHON_BT2_LOG_LEVEL is kept.  If they are specified, they
force the log level of the corresponding subsystem, overriding the
log level specified on the command line.

Change-Id: I9ac558737df4789fc8f5bd8d0240887b3b11b295
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1935
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
src/cli/Makefile.am
src/cli/babeltrace2-cfg-cli-args.c
src/cli/babeltrace2-cfg.h
src/cli/babeltrace2-log-level.c [new file with mode: 0644]
src/cli/babeltrace2-log-level.h [new file with mode: 0644]
src/cli/babeltrace2.c

index 56e2a899e7cf1260a1ef22821432bded3fb0fddb..bc7627e91808c74c2e2f1c90900e4199a3594348 100644 (file)
@@ -32,6 +32,8 @@ babeltrace2_bin_SOURCES = \
        babeltrace2-cfg-cli-args-default.c \
        babeltrace2-cfg-cli-params-arg.c \
        babeltrace2-cfg-cli-params-arg.h \
+       babeltrace2-log-level.c \
+       babeltrace2-log-level.h \
        babeltrace2-plugins.c \
        babeltrace2-plugins.h \
        babeltrace2-query.c \
index cb900f48b22e47248694593d9f82fb4191e7ac3a..1e2da506ad49722fe2a9535bc9ba041bed920e46 100644 (file)
 #include "babeltrace2-cfg-cli-args.h"
 #include "babeltrace2-cfg-cli-args-connect.h"
 #include "babeltrace2-cfg-cli-params-arg.h"
+#include "babeltrace2-log-level.h"
 #include "babeltrace2-plugins.h"
 #include "babeltrace2-query.h"
 #include "autodisc/autodisc.h"
 #include "common/version.h"
 
-static const int cli_default_log_level = BT_LOG_WARNING;
-
 /* INI-style parsing FSM states */
 enum ini_parsing_fsm_state {
        /* Expect a map key (identifier) */
@@ -3846,19 +3845,20 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
                        stream_intersection_mode = true;
                        break;
                case OPT_VERBOSE:
-                       if (*default_log_level != BT_LOG_TRACE &&
-                                       *default_log_level != BT_LOG_DEBUG) {
-                               *default_log_level = BT_LOG_INFO;
-                       }
+                       *default_log_level =
+                               logging_level_min(*default_log_level, BT_LOG_INFO);
                        break;
                case OPT_DEBUG:
-                       *default_log_level = BT_LOG_TRACE;
+                       *default_log_level =
+                               logging_level_min(*default_log_level, BT_LOG_TRACE);
                        break;
                default:
                        break;
                }
        }
 
+       set_auto_log_levels(default_log_level);
+
        /*
         * Legacy behaviour: --verbose used to make the `text` output
         * format print more information. --verbose is now equivalent to
@@ -4031,8 +4031,7 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
 
                        status = auto_discover_source_components(non_opts, plugins, plugin_count,
                                auto_source_discovery_restrict_component_class_name,
-                               *default_log_level >= 0 ? *default_log_level : cli_default_log_level,
-                               &auto_disc);
+                               *default_log_level, &auto_disc);
 
                        if (status != 0) {
                                goto error;
@@ -4312,14 +4311,6 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
                goto end;
        }
 
-       /*
-        * If the log level is still unset at this point, set it to
-        * the program's default.
-        */
-       if (*default_log_level < 0) {
-               *default_log_level = cli_default_log_level;
-       }
-
        cfg = bt_config_run_from_args_array(run_args, retcode,
                plugin_paths, *default_log_level);
        if (!cfg) {
@@ -4334,14 +4325,6 @@ error:
        BT_OBJECT_PUT_REF_AND_RESET(cfg);
 
 end:
-       /*
-        * If the log level is still unset at this point, set it to
-        * the program's default.
-        */
-       if (*default_log_level < 0) {
-               *default_log_level = cli_default_log_level;
-       }
-
        bt_argpar_parse_ret_fini(&argpar_parse_ret);
 
        free(output);
@@ -4514,28 +4497,28 @@ struct bt_config *bt_config_cli_args_create(int argc, const char *argv[],
 
                        switch (item_opt->descr->id) {
                                case OPT_DEBUG:
-                                       default_log_level = BT_LOG_TRACE;
+                                       default_log_level =
+                                               logging_level_min(default_log_level, BT_LOG_TRACE);
                                        break;
                                case OPT_VERBOSE:
-                                       /*
-                                        * Legacy: do not override a previous
-                                        * --debug because --verbose and --debug
-                                        * can be specified together (in this
-                                        * case we want the lowest log level to
-                                        * apply, TRACE).
-                                        */
-                                       default_log_level = BT_LOG_INFO;
+                                       default_log_level =
+                                               logging_level_min(default_log_level, BT_LOG_INFO);
                                        break;
                                case OPT_LOG_LEVEL:
-                                       default_log_level =
-                                               bt_log_get_level_from_string(item_opt->arg);
-                                       if (default_log_level < 0) {
+                               {
+                                       int level = bt_log_get_level_from_string(item_opt->arg);
+
+                                       if (level < 0) {
                                                BT_CLI_LOGE_APPEND_CAUSE(
                                                        "Invalid argument for --log-level option:\n    %s",
                                                        item_opt->arg);
                                                goto error;
                                        }
+
+                                       default_log_level =
+                                               logging_level_min(default_log_level, level);
                                        break;
+                               }
                                case OPT_PLUGIN_PATH:
                                        if (bt_config_append_plugin_paths_check_setuid_setgid(
                                                        plugin_paths, item_opt->arg)) {
@@ -4620,13 +4603,16 @@ struct bt_config *bt_config_cli_args_create(int argc, const char *argv[],
        BT_ASSERT(command_argc >= 0);
 
        /*
-        * The convert command can set its own default log level for
-        * backward compatibility reasons. It only does so if there's no
-        * log level yet, so do not force one for this command.
+        * For all commands other than `convert`, we now know the log level to
+        * use, so we can apply it with `set_auto_log_levels`.
+        *
+        * The convert command has `--debug` and `--verbose` arguments that are
+        * equivalent to the top-level arguments of the same name.  So after it
+        * has parsed its arguments, `bt_config_convert_from_args` calls
+        * `set_auto_log_levels` itself.
         */
-       if (command_type != COMMAND_TYPE_CONVERT && default_log_level < 0) {
-               /* Default log level */
-               default_log_level = cli_default_log_level;
+       if (command_type != COMMAND_TYPE_CONVERT) {
+               set_auto_log_levels(&default_log_level);
        }
 
        /*
index 58bc9d50962133f40d64143062fdaaa70c21ba9b..fc566d79ab2802e11b9b39f18395ec5ad4ac8f2b 100644 (file)
@@ -63,8 +63,6 @@ struct bt_config_connection {
 
 struct bt_config {
        bt_object base;
-       bool debug;
-       bool verbose;
        bt_value *plugin_paths;
        bool omit_system_plugin_path;
        bool omit_home_plugin_path;
diff --git a/src/cli/babeltrace2-log-level.c b/src/cli/babeltrace2-log-level.c
new file mode 100644 (file)
index 0000000..da29051
--- /dev/null
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2019 Philippe Proulx <pproulx@efficios.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#define BT_LOG_TAG "CLI"
+#include "logging.h"
+
+#include "babeltrace2-log-level.h"
+
+#include <stdlib.h>
+
+#include "common/assert.h"
+
+/*
+ * Known environment variable names for the log levels of the project's
+ * modules.
+ */
+static const char* log_level_env_var_names[] = {
+       "BABELTRACE_PLUGIN_CTF_METADATA_LOG_LEVEL",
+       "BABELTRACE_PYTHON_BT2_LOG_LEVEL",
+       NULL,
+};
+
+static const int babeltrace2_default_log_level = BT_LOG_WARNING;
+
+void set_auto_log_levels(int *logging_level)
+{
+       const char **env_var_name;
+
+       /* Setting this is equivalent to passing --debug. */
+       if (getenv("BABELTRACE_DEBUG") &&
+                       strcmp(getenv("BABELTRACE_DEBUG"), "1") == 0) {
+               *logging_level = logging_level_min(*logging_level, BT_LOG_TRACE);
+       }
+
+       /* Setting this is equivalent to passing --verbose. */
+       if (getenv("BABELTRACE_VERBOSE") &&
+                       strcmp(getenv("BABELTRACE_VERBOSE"), "1") == 0) {
+               *logging_level = logging_level_min(*logging_level, BT_LOG_INFO);
+       }
+
+       /*
+        * logging_level is BT_LOG_NONE at this point if no log level was
+        * specified at all by the user.
+        */
+       if (*logging_level == -1) {
+               *logging_level = babeltrace2_default_log_level;
+       }
+
+       /*
+        * If the user hasn't requested a specific log level for the lib
+        * (through LIBBABELTRACE2_INIT_LOG_LEVEL), set it.
+        */
+       if (!getenv("LIBBABELTRACE2_INIT_LOG_LEVEL")) {
+               bt_logging_set_global_level(*logging_level);
+       }
+
+       /*
+        * If the user hasn't requested a specific log level for the CLI,
+        * (through BABELTRACE_CLI_LOG_LEVEL), set it.
+        */
+       if (!getenv(ENV_BABELTRACE_CLI_LOG_LEVEL)) {
+               bt_cli_log_level = *logging_level;
+       }
+
+       for (env_var_name = log_level_env_var_names; *env_var_name; env_var_name++) {
+               if (!getenv(*env_var_name)) {
+                       char val[2] = { 0 };
+
+                       /*
+                        * Set module's default log level if not
+                        * explicitly specified.
+                        */
+                       val[0] = bt_log_get_letter_from_level(*logging_level);
+                       g_setenv(*env_var_name, val, 1);
+               }
+       }
+}
diff --git a/src/cli/babeltrace2-log-level.h b/src/cli/babeltrace2-log-level.h
new file mode 100644 (file)
index 0000000..af06cd7
--- /dev/null
@@ -0,0 +1,50 @@
+#ifndef CLI_BABELTRACE_LOG_LEVEL_H
+#define CLI_BABELTRACE_LOG_LEVEL_H
+
+/*
+ * Copyright 2019 Philippe Proulx <pproulx@efficios.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <babeltrace2/babeltrace.h>
+
+#define ENV_BABELTRACE_CLI_LOG_LEVEL "BABELTRACE_CLI_LOG_LEVEL"
+
+/*
+ * Return the minimal (most verbose) log level between `a` and `b`.
+ *
+ * If one of the parameter has the value -1, it is ignored in the comparison
+ * and the other value is returned.  If both parameters are -1, -1 is returned.
+ */
+static inline
+int logging_level_min(int a, int b)
+{
+       if (a == -1) {
+               return b;
+       } else if (b == -1) {
+               return a;
+       } else {
+               return MIN(a, b);
+       }
+}
+
+void set_auto_log_levels(int *logging_level);
+
+#endif /* CLI_BABELTRACE_LOG_LEVEL_H */
index b42246b4f3fda238cf093476d155320e4a8d445c..87d812fd178ca8b36b56dea287caba4368c2d8d0 100644 (file)
 #include "babeltrace2-cfg.h"
 #include "babeltrace2-cfg-cli-args.h"
 #include "babeltrace2-cfg-cli-args-default.h"
+#include "babeltrace2-log-level.h"
 #include "babeltrace2-plugins.h"
 #include "babeltrace2-query.h"
 
 #define ENV_BABELTRACE_WARN_COMMAND_NAME_DIRECTORY_CLASH "BABELTRACE_CLI_WARN_COMMAND_NAME_DIRECTORY_CLASH"
-#define ENV_BABELTRACE_CLI_LOG_LEVEL "BABELTRACE_CLI_LOG_LEVEL"
 #define NSEC_PER_SEC   1000000000LL
 
-/*
- * Known environment variable names for the log levels of the project's
- * modules.
- */
-static const char* log_level_env_var_names[] = {
-       "BABELTRACE_PLUGIN_CTF_METADATA_LOG_LEVEL",
-       "BABELTRACE_PYTHON_BT2_LOG_LEVEL",
-       NULL,
-};
-
 /* Application's interrupter (owned by this) */
 static bt_interrupter *the_interrupter;
 
@@ -569,8 +559,6 @@ void print_cfg(struct bt_config *cfg)
        }
 
        BT_LOGI_STR("CLI configuration:");
-       BT_LOGI("  Debug mode: %s\n", cfg->debug ? "yes" : "no");
-       BT_LOGI("  Verbose mode: %s\n", cfg->verbose ? "yes" : "no");
 
        switch (cfg->command) {
        case BT_CONFIG_COMMAND_RUN:
@@ -2494,87 +2482,6 @@ void init_log_level(void)
        bt_cli_log_level = bt_log_get_level_from_env(ENV_BABELTRACE_CLI_LOG_LEVEL);
 }
 
-static
-void set_auto_log_levels(struct bt_config *cfg)
-{
-       const char **env_var_name;
-
-       /*
-        * Override the configuration's default log level if
-        * BABELTRACE_VERBOSE or BABELTRACE_DEBUG environment variables
-        * are found for backward compatibility with legacy Babetrace 1.
-        */
-       if (getenv("BABELTRACE_DEBUG") &&
-                       strcmp(getenv("BABELTRACE_DEBUG"), "1") == 0) {
-               cfg->log_level = BT_LOG_TRACE;
-       } else if (getenv("BABELTRACE_VERBOSE") &&
-                       strcmp(getenv("BABELTRACE_VERBOSE"), "1") == 0) {
-               cfg->log_level = BT_LOG_INFO;
-       }
-
-       /*
-        * Set log levels according to --debug or --verbose. For
-        * backward compatibility, --debug is more verbose than
-        * --verbose. So:
-        *
-        *     --verbose: INFO log level
-        *     --debug:   TRACE log level (includes DEBUG, which is
-        *                is less verbose than TRACE in the internal
-        *                logging framework)
-        */
-       if (!getenv("LIBBABELTRACE2_INIT_LOG_LEVEL")) {
-               if (cfg->verbose) {
-                       bt_logging_set_global_level(BT_LOG_INFO);
-               } else if (cfg->debug) {
-                       bt_logging_set_global_level(BT_LOG_TRACE);
-               } else {
-                       /*
-                        * Set library's default log level if not
-                        * explicitly specified.
-                        */
-                       bt_logging_set_global_level(cfg->log_level);
-               }
-       }
-
-       if (!getenv(ENV_BABELTRACE_CLI_LOG_LEVEL)) {
-               if (cfg->verbose) {
-                       bt_cli_log_level = BT_LOG_INFO;
-               } else if (cfg->debug) {
-                       bt_cli_log_level = BT_LOG_TRACE;
-               } else {
-                       /*
-                        * Set CLI's default log level if not explicitly
-                        * specified.
-                        */
-                       bt_cli_log_level = cfg->log_level;
-               }
-       }
-
-       env_var_name = log_level_env_var_names;
-
-       while (*env_var_name) {
-               if (!getenv(*env_var_name)) {
-                       if (cfg->verbose) {
-                               g_setenv(*env_var_name, "INFO", 1);
-                       } else if (cfg->debug) {
-                               g_setenv(*env_var_name, "TRACE", 1);
-                       } else {
-                               char val[2] = { 0 };
-
-                               /*
-                                * Set module's default log level if not
-                                * explicitly specified.
-                                */
-                               val[0] = bt_log_get_letter_from_level(
-                                       cfg->log_level);
-                               g_setenv(*env_var_name, val, 1);
-                       }
-               }
-
-               env_var_name++;
-       }
-}
-
 static
 void print_error_causes(void)
 {
@@ -2721,7 +2628,6 @@ int main(int argc, const char **argv)
                goto end;
        }
 
-       set_auto_log_levels(cfg);
        print_cfg(cfg);
 
        if (cfg->command_needs_plugins) {
This page took 0.033347 seconds and 4 git commands to generate.