From 949f049bd7bd19725768d46e349178c7c0da12a7 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 14 Apr 2021 22:28:07 -0400 Subject: [PATCH] lttng add-trigger: replace log level options with --log-level MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Implement the proposal to replace the --loglevel and --loglevel-only options with a single --log-level option. The syntax of the argument to --log-level is: - `LEVEL`: to specify only this level - `LEVEL..`: to specify from this level up to the most critical level - `..`: to specify any level. Basically the same as no `--log-level` option. The intent is to keep the door open to fully support ranges with the `LEVEL1..LEVEL2` syntax. The loglevel_parse_range_string function and friends are implement this, but their caller in add_trigger.c, parse_log_level_string, artificially restricts it to the two cases shown above. Change-Id: Id8607b3dae2db5aace263f0c56ada2e822db907c Signed-off-by: Simon Marchi Signed-off-by: Jérémie Galarneau --- src/bin/lttng/commands/add_trigger.c | 159 ++++++++++++------ src/bin/lttng/loglevel.c | 129 ++++++++++++++ src/bin/lttng/loglevel.h | 20 +++ .../tools/trigger/test_add_trigger_cli | 18 +- .../tools/trigger/test_list_triggers_cli | 17 +- 5 files changed, 290 insertions(+), 53 deletions(-) diff --git a/src/bin/lttng/commands/add_trigger.c b/src/bin/lttng/commands/add_trigger.c index 1859a7fa8..934464349 100644 --- a/src/bin/lttng/commands/add_trigger.c +++ b/src/bin/lttng/commands/add_trigger.c @@ -47,8 +47,7 @@ enum { OPT_FILTER, OPT_EXCLUDE_NAMES, OPT_EVENT_NAME, - OPT_LOGLEVEL, - OPT_LOGLEVEL_ONLY, + OPT_LOG_LEVEL, OPT_DOMAIN, OPT_TYPE, @@ -67,8 +66,7 @@ static const struct argpar_opt_descr event_rule_opt_descrs[] = { { OPT_FILTER, 'f', "filter", true }, { OPT_NAME, 'n', "name", true }, { OPT_EXCLUDE_NAMES, 'x', "exclude-names", true }, - { OPT_LOGLEVEL, '\0', "loglevel", true }, - { OPT_LOGLEVEL_ONLY, '\0', "loglevel-only", true }, + { OPT_LOG_LEVEL, 'l', "log-level", true }, { OPT_EVENT_NAME, 'E', "event-name", true }, { OPT_DOMAIN, 'd', "domain", true }, @@ -184,45 +182,108 @@ int create_exclusion_list_and_validate(const char *event_name, char ***exclusion_list); /* - * Parse `str` as a log level in domain `domain_type`. Return -1 if the string - * is not recognized as a valid log level. + * Parse `str` as a log level in domain `domain_type`. + * + * Return the log level in `*log_level`. Return true in `*log_level_only` if + * the string specifies exactly this log level, false if it specifies at least + * this log level. + * + * Return true if the string was successfully parsed as a log level string. */ -static -int parse_loglevel_string(const char *str, enum lttng_domain_type domain_type) +static bool parse_log_level_string(const char *str, + enum lttng_domain_type domain_type, + int *log_level, + bool *log_level_only) { + bool ret; + switch (domain_type) { case LTTNG_DOMAIN_UST: { - enum lttng_loglevel loglevel; - const int ret = loglevel_name_to_value(str, &loglevel); + enum lttng_loglevel log_level_min, log_level_max; + if (!loglevel_parse_range_string( + str, &log_level_min, &log_level_max)) { + goto error; + } - return ret == -1 ? ret : (int) loglevel; + /* Only support VAL and VAL.. for now. */ + if (log_level_min != log_level_max && + log_level_max != LTTNG_LOGLEVEL_EMERG) { + goto error; + } + + *log_level = (int) log_level_min; + *log_level_only = log_level_min == log_level_max; + break; } case LTTNG_DOMAIN_LOG4J: { - enum lttng_loglevel_log4j loglevel; - const int ret = loglevel_log4j_name_to_value(str, &loglevel); + enum lttng_loglevel_log4j log_level_min, log_level_max; + if (!loglevel_log4j_parse_range_string( + str, &log_level_min, &log_level_max)) { + goto error; + } - return ret == -1 ? ret : (int) loglevel; + /* Only support VAL and VAL.. for now. */ + if (log_level_min != log_level_max && + log_level_max != LTTNG_LOGLEVEL_LOG4J_FATAL) { + goto error; + } + + *log_level = (int) log_level_min; + *log_level_only = log_level_min == log_level_max; + break; } case LTTNG_DOMAIN_JUL: { - enum lttng_loglevel_jul loglevel; - const int ret = loglevel_jul_name_to_value(str, &loglevel); + enum lttng_loglevel_jul log_level_min, log_level_max; + if (!loglevel_jul_parse_range_string( + str, &log_level_min, &log_level_max)) { + goto error; + } - return ret == -1 ? ret : (int) loglevel; + /* Only support VAL and VAL.. for now. */ + if (log_level_min != log_level_max && + log_level_max != LTTNG_LOGLEVEL_JUL_SEVERE) { + goto error; + } + + *log_level = (int) log_level_min; + *log_level_only = log_level_min == log_level_max; + break; } case LTTNG_DOMAIN_PYTHON: { - enum lttng_loglevel_python loglevel; - const int ret = loglevel_python_name_to_value(str, &loglevel); + enum lttng_loglevel_python log_level_min, log_level_max; + if (!loglevel_python_parse_range_string( + str, &log_level_min, &log_level_max)) { + goto error; + } - return ret == -1 ? ret : (int) loglevel; + /* Only support VAL and VAL.. for now. */ + if (log_level_min != log_level_max && + log_level_max != + LTTNG_LOGLEVEL_PYTHON_CRITICAL) { + goto error; + } + + *log_level = (int) log_level_min; + *log_level_only = log_level_min == log_level_max; + break; } default: /* Invalid domain type. */ abort(); } + + ret = true; + goto end; + +error: + ret = false; + +end: + return ret; } static int parse_kernel_probe_opts(const char *source, @@ -574,8 +635,7 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) char *filter = NULL; /* Log level. */ - char *loglevel_str = NULL; - bool loglevel_only = false; + char *log_level_str = NULL; lttng_dynamic_pointer_array_init(&res.capture_descriptors, destroy_event_expr); @@ -660,15 +720,12 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) } break; - case OPT_LOGLEVEL: - case OPT_LOGLEVEL_ONLY: - if (!assign_string(&loglevel_str, item_opt->arg, - "--loglevel/--loglevel-only")) { + case OPT_LOG_LEVEL: + if (!assign_string(&log_level_str, + item_opt->arg, "--log-level/-l")) { goto error; } - loglevel_only = item_opt->descr->id == - OPT_LOGLEVEL_ONLY; break; case OPT_CAPTURE: { @@ -859,9 +916,16 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) } } - if (loglevel_str && event_rule_type != LTTNG_EVENT_RULE_TYPE_TRACEPOINT) { - ERR("Log levels are only applicable to tracepoint event rules."); - goto error; + if (log_level_str) { + if (event_rule_type != LTTNG_EVENT_RULE_TYPE_TRACEPOINT) { + ERR("Log levels are only applicable to tracepoint event rules."); + goto error; + } + + if (domain_type == LTTNG_DOMAIN_KERNEL) { + ERR("Log levels are not supported by the kernel tracer."); + goto error; + } } /* Finally, create the event rule object. */ @@ -913,26 +977,25 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) } } - if (loglevel_str) { - int loglevel; - - if (domain_type == LTTNG_DOMAIN_KERNEL) { - ERR("Log levels are not supported by the kernel tracer."); - goto error; - } - - loglevel = parse_loglevel_string( - loglevel_str, domain_type); - if (loglevel < 0) { - ERR("Failed to parse `%s` as a log level.", - loglevel_str); + /* + * ".." is the same as passing no log level option and + * correspond the the "ANY" case. + */ + if (log_level_str && strcmp(log_level_str, "..") != 0) { + int log_level; + bool log_level_only; + + if (!parse_log_level_string(log_level_str, domain_type, + &log_level, &log_level_only)) { + ERR("Failed to parse log level string `%s`.", + log_level_str); goto error; } - if (loglevel_only) { - log_level_rule = lttng_log_level_rule_exactly_create(loglevel); + if (log_level_only) { + log_level_rule = lttng_log_level_rule_exactly_create(log_level); } else { - log_level_rule = lttng_log_level_rule_at_least_as_severe_as_create(loglevel); + log_level_rule = lttng_log_level_rule_at_least_as_severe_as_create(log_level); } if (log_level_rule == NULL) { @@ -1065,7 +1128,7 @@ end: free(filter); free(name); free(exclude_names); - free(loglevel_str); + free(log_level_str); free(location); free(event_name); diff --git a/src/bin/lttng/loglevel.c b/src/bin/lttng/loglevel.c index c18bf381c..4ac795f5b 100644 --- a/src/bin/lttng/loglevel.c +++ b/src/bin/lttng/loglevel.c @@ -144,6 +144,73 @@ end: return ret; } +static bool loglevel_parse_range_string_common(const char *str, + const struct loglevel_name_value *nvs, + size_t nvs_count, + int *min, + int *max) +{ + bool ret; + int i; + const struct loglevel_name_value *nv; + + for (i = 0; i < nvs_count; i++) { + nv = &nvs[i]; + + if (strncmp(str, nv->name, strlen(nv->name)) == 0) { + break; + } + } + + if (i == nvs_count) { + goto error; + } + + *min = nv->value; + str += strlen(nv->name); + + if (*str == '\0') { + *max = nv->value; + ret = true; + goto end; + } + + if (strncmp(str, "..", strlen("..")) != 0) { + goto error; + } + + str += strlen(".."); + + if (*str == '\0') { + *max = LTTNG_LOGLEVEL_EMERG; + ret = true; + goto end; + } + + for (i = 0; i < nvs_count; i++) { + nv = &nvs[i]; + + if (strcmp(str, nv->name) == 0) { + break; + } + } + + if (i == nvs_count) { + goto error; + } + + *max = nv->value; + + ret = true; + goto end; + +error: + ret = false; + +end: + return ret; +} + LTTNG_HIDDEN int loglevel_name_to_value(const char *name, enum lttng_loglevel *loglevel) { @@ -158,6 +225,21 @@ int loglevel_name_to_value(const char *name, enum lttng_loglevel *loglevel) return ret; } +LTTNG_HIDDEN +bool loglevel_parse_range_string(const char *str, + enum lttng_loglevel *min, + enum lttng_loglevel *max) +{ + int min_int, max_int; + bool ret = loglevel_parse_range_string_common(str, loglevel_values, + ARRAY_SIZE(loglevel_values), &min_int, &max_int); + + *min = min_int; + *max = max_int; + + return ret; +} + LTTNG_HIDDEN int loglevel_log4j_name_to_value( const char *name, enum lttng_loglevel_log4j *loglevel) @@ -174,6 +256,22 @@ int loglevel_log4j_name_to_value( return ret; } +LTTNG_HIDDEN +bool loglevel_log4j_parse_range_string(const char *str, + enum lttng_loglevel_log4j *min, + enum lttng_loglevel_log4j *max) +{ + int min_int, max_int; + bool ret = loglevel_parse_range_string_common(str, + loglevel_log4j_values, + ARRAY_SIZE(loglevel_log4j_values), &min_int, &max_int); + + *min = min_int; + *max = max_int; + + return ret; +} + LTTNG_HIDDEN int loglevel_jul_name_to_value( const char *name, enum lttng_loglevel_jul *loglevel) @@ -190,6 +288,21 @@ int loglevel_jul_name_to_value( return ret; } +LTTNG_HIDDEN +bool loglevel_jul_parse_range_string(const char *str, + enum lttng_loglevel_jul *min, + enum lttng_loglevel_jul *max) +{ + int min_int, max_int; + bool ret = loglevel_parse_range_string_common(str, loglevel_jul_values, + ARRAY_SIZE(loglevel_jul_values), &min_int, &max_int); + + *min = min_int; + *max = max_int; + + return ret; +} + LTTNG_HIDDEN int loglevel_python_name_to_value( const char *name, enum lttng_loglevel_python *loglevel) @@ -206,6 +319,22 @@ int loglevel_python_name_to_value( return ret; } +LTTNG_HIDDEN +bool loglevel_python_parse_range_string(const char *str, + enum lttng_loglevel_python *min, + enum lttng_loglevel_python *max) +{ + int min_int, max_int; + bool ret = loglevel_parse_range_string_common(str, + loglevel_python_values, + ARRAY_SIZE(loglevel_python_values), &min_int, &max_int); + + *min = min_int; + *max = max_int; + + return ret; +} + static const char *lookup_name_from_value(const struct loglevel_name_value values[], size_t values_count, int loglevel) diff --git a/src/bin/lttng/loglevel.h b/src/bin/lttng/loglevel.h index 78989504e..8724ab16c 100644 --- a/src/bin/lttng/loglevel.h +++ b/src/bin/lttng/loglevel.h @@ -14,18 +14,38 @@ LTTNG_HIDDEN int loglevel_name_to_value(const char *name, enum lttng_loglevel *loglevel); +LTTNG_HIDDEN +bool loglevel_parse_range_string(const char *str, + enum lttng_loglevel *min, + enum lttng_loglevel *max); + LTTNG_HIDDEN int loglevel_log4j_name_to_value( const char *name, enum lttng_loglevel_log4j *loglevel); +LTTNG_HIDDEN +bool loglevel_log4j_parse_range_string(const char *str, + enum lttng_loglevel_log4j *min, + enum lttng_loglevel_log4j *max); + LTTNG_HIDDEN int loglevel_jul_name_to_value( const char *name, enum lttng_loglevel_jul *loglevel); +LTTNG_HIDDEN +bool loglevel_jul_parse_range_string(const char *str, + enum lttng_loglevel_jul *min, + enum lttng_loglevel_jul *max); + LTTNG_HIDDEN int loglevel_python_name_to_value( const char *name, enum lttng_loglevel_python *loglevel); +LTTNG_HIDDEN +bool loglevel_python_parse_range_string(const char *str, + enum lttng_loglevel_python *min, + enum lttng_loglevel_python *max); + LTTNG_HIDDEN const char *loglevel_value_to_name(int loglevel); diff --git a/tests/regression/tools/trigger/test_add_trigger_cli b/tests/regression/tools/trigger/test_add_trigger_cli index 207689477..06bd2d3de 100755 --- a/tests/regression/tools/trigger/test_add_trigger_cli +++ b/tests/regression/tools/trigger/test_add_trigger_cli @@ -23,7 +23,7 @@ TESTDIR="$CURDIR/../../.." # shellcheck source=../../../utils/utils.sh source "$TESTDIR/utils/utils.sh" -plan_tests 252 +plan_tests 264 FULL_LTTNG_BIN="${TESTDIR}/../src/bin/lttng/${LTTNG_BIN}" @@ -126,6 +126,18 @@ test_success "rotate session action polices" \ --action rotate-session my_session \ --rate-policy=once-after:55 +test_success "--log-level single level" \ + --condition event-rule-matches --domain=user --log-level=INFO \ + --action notify + +test_success "--log-level range open max" \ + --condition event-rule-matches --domain=user --log-level=INFO.. \ + --action notify + +test_success "--log-level range any" \ + --condition event-rule-matches --domain=user --log-level=.. \ + --action notify + skip $ist_root "non-root user: skipping kprobe tests" 18 || { for type in kprobe kernel-probe; do test_success "--condition event-rule-matches probe by symbol" \ @@ -309,6 +321,10 @@ test_failure "extra args after --condition event-rule-matches" \ "Error: Unexpected argument 'bozo'" \ --condition event-rule-matches --domain=user bozo +test_failure "--log-level unknown level" \ + "Error: Failed to parse log level string \`FOO\`." \ + --condition event-rule-matches --domain=user --log-level=FOO + test_failure "two same --domain" \ "Error: More than one \`--domain\` was specified." \ --condition event-rule-matches --domain=user --domain=user diff --git a/tests/regression/tools/trigger/test_list_triggers_cli b/tests/regression/tools/trigger/test_list_triggers_cli index 30b636969..f5d34f967 100755 --- a/tests/regression/tools/trigger/test_list_triggers_cli +++ b/tests/regression/tools/trigger/test_list_triggers_cli @@ -23,8 +23,7 @@ TESTDIR="$CURDIR/../../.." # shellcheck source=../../../utils/utils.sh source "$TESTDIR/utils/utils.sh" - -NUM_TESTS=82 +NUM_TESTS=84 FULL_LTTNG_BIN="${TESTDIR}/../src/bin/lttng/${LTTNG_BIN}" @@ -95,8 +94,9 @@ test_on_event_tracepoint () lttng_add_trigger_ok "C" --condition event-rule-matches --domain=user --action notify lttng_add_trigger_ok "A" --condition event-rule-matches --name=aaa --domain=user --filter 'p == 2' --action notify lttng_add_trigger_ok "D" --condition event-rule-matches --name='hello*' --domain=user -x 'hello2,hello3,hello4' --action notify - lttng_add_trigger_ok "B" --condition event-rule-matches --domain=user --name=gerboise --loglevel INFO --action notify - lttng_add_trigger_ok "E" --condition event-rule-matches --domain=user --name=lemming --loglevel-only WARNING --action notify + lttng_add_trigger_ok "B" --condition event-rule-matches --domain=user --name=gerboise --log-level INFO.. --action notify + lttng_add_trigger_ok "E" --condition event-rule-matches --domain=user --name=lemming --log-level WARNING --action notify + lttng_add_trigger_ok "J" --condition event-rule-matches --domain=user --name=lemming --log-level .. --action notify lttng_add_trigger_ok "F" --condition event-rule-matches --domain=user --name=capture-payload-field --capture a --action notify lttng_add_trigger_ok "G" --condition event-rule-matches --domain=user --name=capture-array --capture 'a[2]' --capture '$ctx.tourlou[18]' --action notify lttng_add_trigger_ok "H" --condition event-rule-matches --domain=user --name=capture-chan-ctx --capture '$ctx.vpid' --action notify @@ -184,6 +184,14 @@ test_on_event_tracepoint () notify errors: none errors: none + - name: J + user id: ${uid} + condition: event rule hit + rule: lemming (type: tracepoint, domain: ust) + actions: + notify + errors: none + errors: none EOF list_triggers_matches_ok "event-rule-matches, tracepoint event rule" "${tmp_expected_stdout}" @@ -197,6 +205,7 @@ test_on_event_tracepoint () lttng_remove_trigger_ok "G" lttng_remove_trigger_ok "H" lttng_remove_trigger_ok "I" + lttng_remove_trigger_ok "J" } test_on_event_probe () -- 2.34.1