From: Jonathan Rajotte Date: Tue, 30 Mar 2021 01:43:12 +0000 (-0400) Subject: Move firing policy from lttng_trigger to lttng_action X-Git-Tag: v2.13.0-rc1~88 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=e45dd625d3e802d8e6e2ec3de180c73546e8f9fe Move firing policy from lttng_trigger to lttng_action After some reflection on the future of the importance of the trigger feature and the centralization we wish to carry around it, it is required that the notion of firing policy be moved from the trigger object to each action object of a trigger. This is necessary since we plan on introducing tracer side actions, such as increment value of map. Controlling the firing policy on the tracer side is not an easy thing to do since for UST tracing a lot a synchronizations must take place and also we must define the behaviour when multiple apps are present. Hence, we need a way to ensure that we are not painting ourself in a corner. The middle ground that was chosen was to move the notion of firing policy to the action object level. This allows us to scope the concept to the action and decide for each type if firing policy can be supported and, as needed, define the behaviour per action type. Essentially this patch perform the complete transition. It removes the notion of firing policy at the trigger level and exposes the firing policy of each action type if applicable. CLI ====== For the `add-trigger` command the change essentially boils down to moving the `--fire-every` and `--fire-once-after` from a top-level parsing to the parsing of each actions. Yes, for now all actions supports the `--fire-*` options but it will not be the case in the future. A side effect of this is that a user could decide to have different firing policy for each actions, but this also mean that if a user want to apply the same firing policy across actions, the user needs to specify it for each actions. This could be solved in the future as the trigger feature matures and that common ground are found in the behaviour or implementation of the actions (tracer side action, async action, sync actions etc.) such that "syntactic sugar" options emerge. As for the `list-trigger`, we move the firing policy printing to each actions. Tests have been updated to reflect the changes. Signed-off-by: Jonathan Rajotte Signed-off-by: Jérémie Galarneau Change-Id: Ib29d4319922096c0c4b3f00782f3bbefb17e2f40 --- diff --git a/include/lttng/trigger/trigger-internal.h b/include/lttng/trigger/trigger-internal.h index 59fe771aa..c5ee9c64b 100644 --- a/include/lttng/trigger/trigger-internal.h +++ b/include/lttng/trigger/trigger-internal.h @@ -30,11 +30,6 @@ struct lttng_trigger { char *name; /* For now only the uid portion of the credentials is used. */ struct lttng_credentials creds; - struct { - enum lttng_trigger_firing_policy type; - uint64_t threshold; - uint64_t current_count; - } firing_policy; /* * Internal use only. * The unique token passed to the tracer to identify an event-rule @@ -60,10 +55,6 @@ struct lttng_trigger_comm { uint32_t length; /* Includes '\0' terminator. */ uint32_t name_length; - /* Firing policy. */ - /* Maps to enum lttng_trigger_firing_policy. */ - uint8_t firing_policy_type; - uint64_t firing_policy_threshold; /* A null-terminated name, a condition, and an action follow. */ char payload[]; } LTTNG_PACKED; @@ -162,20 +153,6 @@ LTTNG_HIDDEN void lttng_trigger_set_credentials(struct lttng_trigger *trigger, const struct lttng_credentials *creds); - -/* - * Fire the trigger. - * Increments the occurrence count. - */ -LTTNG_HIDDEN -void lttng_trigger_fire(struct lttng_trigger *trigger); - -/* - * Check if the trigger would fire. - */ -LTTNG_HIDDEN -bool lttng_trigger_should_fire(const struct lttng_trigger *trigger); - /* * Return the type of any underlying domain restriction. If no particular * requirement is present, returns LTTNG_DOMAIN_NONE. diff --git a/include/lttng/trigger/trigger.h b/include/lttng/trigger/trigger.h index 3b1cedda1..6490af118 100644 --- a/include/lttng/trigger/trigger.h +++ b/include/lttng/trigger/trigger.h @@ -36,11 +36,6 @@ enum lttng_trigger_status { LTTNG_TRIGGER_STATUS_PERMISSION_DENIED = -6, }; -enum lttng_trigger_firing_policy { - LTTNG_TRIGGER_FIRING_POLICY_EVERY_N = 0, - LTTNG_TRIGGER_FIRING_POLICY_ONCE_AFTER_N = 1, -}; - /* * Create a trigger object associating a condition and an action. * @@ -144,37 +139,6 @@ extern enum lttng_trigger_status lttng_trigger_get_name( extern enum lttng_trigger_status lttng_trigger_set_name( struct lttng_trigger *trigger, const char *name); -/* - * Set the trigger firing policy. - * - * This is optional. By default a trigger is set to fire each time the - * associated condition occurs. - * - * Threshold is the number of times the condition must be hit before the policy - * is enacted. - * - * Return LTTNG_TRIGGER_STATUS_OK on success, LTTNG_TRIGGER_STATUS_INVALID - * if invalid parameters are passed. - */ -extern enum lttng_trigger_status lttng_trigger_set_firing_policy( - struct lttng_trigger *trigger, - enum lttng_trigger_firing_policy policy_type, - uint64_t threshold); - -/* - * Get the trigger firing policy. - * - * Threshold is the number of time the condition must be hit before the policy is - * enacted. - * - * Return LTTNG_TRIGGER_STATUS_OK on success, LTTNG_TRIGGER_STATUS_INVALID - * if invalid parameters are passed. - */ -extern enum lttng_trigger_status lttng_trigger_get_firing_policy( - const struct lttng_trigger *trigger, - enum lttng_trigger_firing_policy *policy_type, - uint64_t *threshold); - /* * Destroy (frees) a trigger object. */ diff --git a/src/bin/lttng-sessiond/notification-thread-events.c b/src/bin/lttng-sessiond/notification-thread-events.c index 827cf2c14..29318432f 100644 --- a/src/bin/lttng-sessiond/notification-thread-events.c +++ b/src/bin/lttng-sessiond/notification-thread-events.c @@ -4523,13 +4523,6 @@ int dispatch_one_event_notifier_notification(struct notification_thread_state *s struct notification_trigger_tokens_ht_element, node); - if (!lttng_trigger_should_fire(element->trigger)) { - ret = 0; - goto end_unlock; - } - - lttng_trigger_fire(element->trigger); - trigger_status = lttng_trigger_get_name(element->trigger, &trigger_name); assert(trigger_status == LTTNG_TRIGGER_STATUS_OK); @@ -4841,12 +4834,6 @@ int handle_notification_thread_channel_sample( goto put_list; } - if (!lttng_trigger_should_fire(trigger)) { - goto put_list; - } - - lttng_trigger_fire(trigger); - /* * Ownership of `evaluation` transferred to the action executor * no matter the result. diff --git a/src/bin/lttng/commands/add_trigger.c b/src/bin/lttng/commands/add_trigger.c index ef5b167c5..17e06445b 100644 --- a/src/bin/lttng/commands/add_trigger.c +++ b/src/bin/lttng/commands/add_trigger.c @@ -1335,38 +1335,209 @@ error: end: return cond; } +static const struct argpar_opt_descr notify_action_opt_descrs[] = { + { OPT_FIRE_ONCE_AFTER, '\0', "fire-once-after", true }, + { OPT_FIRE_EVERY, '\0', "fire-every", true }, + ARGPAR_OPT_DESCR_SENTINEL +}; static struct lttng_action *handle_action_notify(int *argc, const char ***argv) { - return lttng_action_notify_create(); -} + struct lttng_action *action = NULL; + struct argpar_state *state = NULL; + struct argpar_item *item = NULL; + char *error = NULL; + char *fire_once_after_str = NULL; + char *fire_every_str = NULL; + struct lttng_firing_policy *policy = NULL; -static const struct argpar_opt_descr no_opt_descrs[] = { - ARGPAR_OPT_DESCR_SENTINEL -}; + state = argpar_state_create(*argc, *argv, notify_action_opt_descrs); + if (!state) { + ERR("Failed to allocate an argpar state."); + goto error; + } + + while (true) { + enum argpar_state_parse_next_status status; + + ARGPAR_ITEM_DESTROY_AND_RESET(item); + status = argpar_state_parse_next(state, &item, &error); + if (status == ARGPAR_STATE_PARSE_NEXT_STATUS_ERROR) { + ERR("%s", error); + goto error; + } else if (status == ARGPAR_STATE_PARSE_NEXT_STATUS_ERROR_UNKNOWN_OPT) { + /* Just stop parsing here. */ + break; + } else if (status == ARGPAR_STATE_PARSE_NEXT_STATUS_END) { + break; + } + + assert(status == ARGPAR_STATE_PARSE_NEXT_STATUS_OK); + + if (item->type == ARGPAR_ITEM_TYPE_OPT) { + const struct argpar_item_opt *item_opt = + (const struct argpar_item_opt *) item; + + switch (item_opt->descr->id) { + case OPT_FIRE_ONCE_AFTER: + { + if (!assign_string(&fire_once_after_str, + item_opt->arg, + "--fire-once-after")) { + goto error; + } + + break; + } + case OPT_FIRE_EVERY: + { + if (!assign_string(&fire_every_str, + item_opt->arg, + "--fire-every")) { + goto error; + } + + break; + } + + default: + abort(); + } + } else { + const struct argpar_item_non_opt *item_non_opt; + + assert(item->type == ARGPAR_ITEM_TYPE_NON_OPT); + + item_non_opt = (const struct argpar_item_non_opt *) item; + + switch (item_non_opt->non_opt_index) { + default: + ERR("Unexpected argument `%s`.", + item_non_opt->arg); + goto error; + } + } + } + + *argc -= argpar_state_get_ingested_orig_args(state); + *argv += argpar_state_get_ingested_orig_args(state); + + if (fire_once_after_str && fire_every_str) { + ERR("--fire-once and --fire-every are mutually exclusive."); + goto error; + } + + if (fire_once_after_str) { + unsigned long long threshold; + + if (utils_parse_unsigned_long_long( + fire_once_after_str, &threshold) != 0) { + ERR("Failed to parse `%s` as an integer.", + fire_once_after_str); + goto error; + } + + if (threshold == 0) { + ERR("Once after N policy threshold cannot be `0`."); + goto error; + } + + policy = lttng_firing_policy_once_after_n_create(threshold); + if (!policy) { + ERR("Failed to create policy once after `%s`.", + fire_once_after_str); + goto error; + } + } + + if (fire_every_str) { + unsigned long long interval; + if (utils_parse_unsigned_long_long(fire_every_str, &interval) != + 0) { + ERR("Failed to parse `%s` as an integer.", + fire_every_str); + goto error; + } + if (interval == 0) { + ERR("Every N policy interval cannot be `0`."); + goto error; + } + + policy = lttng_firing_policy_every_n_create(interval); + if (!policy) { + ERR("Failed to create policy every `%s`.", + fire_every_str); + goto error; + } + } + + action = lttng_action_notify_create(); + if (!action) { + ERR("Failed to create notify action"); + goto error; + } + + if (policy) { + enum lttng_action_status status; + status = lttng_action_notify_set_firing_policy(action, policy); + if (status != LTTNG_ACTION_STATUS_OK) { + ERR("Failed to set firing policy"); + goto error; + } + } + + goto end; + +error: + lttng_action_destroy(action); + action = NULL; + free(error); +end: + free(fire_once_after_str); + free(fire_every_str); + lttng_firing_policy_destroy(policy); + argpar_state_destroy(state); + argpar_item_destroy(item); + return action; +} /* - * Generic handler for a kind of action that takes a session name as its sole - * argument. + * Generic handler for a kind of action that takes a session name and an + * optional firing policy. */ -static -struct lttng_action *handle_action_simple_session( - int *argc, const char ***argv, +static struct lttng_action *handle_action_simple_session_with_policy(int *argc, + const char ***argv, struct lttng_action *(*create_action_cb)(void), - enum lttng_action_status (*set_session_name_cb)(struct lttng_action *, const char *), + enum lttng_action_status (*set_session_name_cb)( + struct lttng_action *, const char *), + enum lttng_action_status (*set_firing_policy_cb)( + struct lttng_action *, + const struct lttng_firing_policy *), const char *action_name) { struct lttng_action *action = NULL; struct argpar_state *state = NULL; struct argpar_item *item = NULL; const char *session_name_arg = NULL; + char *fire_once_after_str = NULL; + char *fire_every_str = NULL; char *error = NULL; enum lttng_action_status action_status; - - state = argpar_state_create(*argc, *argv, no_opt_descrs); + struct lttng_firing_policy *policy = NULL; + + assert(set_session_name_cb); + assert(set_firing_policy_cb); + + const struct argpar_opt_descr firing_policy_opt_descrs[] = { + { OPT_FIRE_ONCE_AFTER, '\0', "fire-once-after", true }, + { OPT_FIRE_EVERY, '\0', "fire-every", true }, + ARGPAR_OPT_DESCR_SENTINEL + }; + + state = argpar_state_create(*argc, *argv, firing_policy_opt_descrs); if (!state) { ERR("Failed to allocate an argpar state."); goto error; @@ -1374,14 +1545,14 @@ struct lttng_action *handle_action_simple_session( while (true) { enum argpar_state_parse_next_status status; - const struct argpar_item_non_opt *item_non_opt; ARGPAR_ITEM_DESTROY_AND_RESET(item); status = argpar_state_parse_next(state, &item, &error); if (status == ARGPAR_STATE_PARSE_NEXT_STATUS_ERROR) { ERR("%s", error); goto error; - } else if (status == ARGPAR_STATE_PARSE_NEXT_STATUS_ERROR_UNKNOWN_OPT) { + } else if (status == + ARGPAR_STATE_PARSE_NEXT_STATUS_ERROR_UNKNOWN_OPT) { /* Just stop parsing here. */ break; } else if (status == ARGPAR_STATE_PARSE_NEXT_STATUS_END) { @@ -1389,17 +1560,48 @@ struct lttng_action *handle_action_simple_session( } assert(status == ARGPAR_STATE_PARSE_NEXT_STATUS_OK); - assert(item->type == ARGPAR_ITEM_TYPE_NON_OPT); + if (item->type == ARGPAR_ITEM_TYPE_OPT) { + const struct argpar_item_opt *item_opt = + (const struct argpar_item_opt *) item; - item_non_opt = (const struct argpar_item_non_opt *) item; + switch (item_opt->descr->id) { + case OPT_FIRE_ONCE_AFTER: + { + if (!assign_string(&fire_once_after_str, + item_opt->arg, + "--fire-once-after")) { + goto error; + } - switch (item_non_opt->non_opt_index) { - case 0: - session_name_arg = item_non_opt->arg; - break; - default: - ERR("Unexpected argument `%s`.", item_non_opt->arg); - goto error; + break; + } + case OPT_FIRE_EVERY: + { + if (!assign_string(&fire_every_str, + item_opt->arg, + "--fire-every")) { + goto error; + } + + break; + } + + default: + abort(); + } + } else { + const struct argpar_item_non_opt *item_non_opt; + item_non_opt = (const struct argpar_item_non_opt *) item; + + switch (item_non_opt->non_opt_index) { + case 0: + session_name_arg = item_non_opt->arg; + break; + default: + ERR("Unexpected argument `%s`.", + item_non_opt->arg); + goto error; + } } } @@ -1411,6 +1613,55 @@ struct lttng_action *handle_action_simple_session( goto error; } + if (fire_once_after_str && fire_every_str) { + ERR("--fire-once and --fire-every are mutually exclusive."); + goto error; + } + + if (fire_once_after_str) { + unsigned long long threshold; + + if (utils_parse_unsigned_long_long( + fire_once_after_str, &threshold) != 0) { + ERR("Failed to parse `%s` as an integer.", + fire_once_after_str); + goto error; + } + + if (threshold == 0) { + ERR("Once after N policy threshold cannot be `0`."); + goto error; + } + + policy = lttng_firing_policy_once_after_n_create(threshold); + if (!policy) { + ERR("Failed to create policy once after `%s`.", + fire_once_after_str); + goto error; + } + } + + if (fire_every_str) { + unsigned long long interval; + if (utils_parse_unsigned_long_long(fire_every_str, &interval) != + 0) { + ERR("Failed to parse `%s` as an integer.", + fire_every_str); + goto error; + } + if (interval == 0) { + ERR("Every N policy interval cannot be `0`."); + goto error; + } + + policy = lttng_firing_policy_every_n_create(interval); + if (!policy) { + ERR("Failed to create policy every `%s`.", + fire_every_str); + goto error; + } + } + action = create_action_cb(); if (!action) { ERR("Failed to allocate %s session action.", action_name); @@ -1424,6 +1675,14 @@ struct lttng_action *handle_action_simple_session( goto error; } + if (policy) { + action_status = set_firing_policy_cb(action, policy); + if (action_status != LTTNG_ACTION_STATUS_OK) { + ERR("Failed to set firing policy"); + goto error; + } + } + goto end; error: @@ -1431,6 +1690,7 @@ error: action = NULL; argpar_item_destroy(item); end: + lttng_firing_policy_destroy(policy); free(error); argpar_state_destroy(state); return action; @@ -1440,29 +1700,30 @@ static struct lttng_action *handle_action_start_session(int *argc, const char ***argv) { - return handle_action_simple_session(argc, argv, - lttng_action_start_session_create, - lttng_action_start_session_set_session_name, - "start"); + return handle_action_simple_session_with_policy(argc, argv, + lttng_action_start_session_create, + lttng_action_start_session_set_session_name, + lttng_action_start_session_set_firing_policy, "start"); } static struct lttng_action *handle_action_stop_session(int *argc, const char ***argv) { - return handle_action_simple_session(argc, argv, - lttng_action_stop_session_create, - lttng_action_stop_session_set_session_name, - "stop"); + return handle_action_simple_session_with_policy(argc, argv, + lttng_action_stop_session_create, + lttng_action_stop_session_set_session_name, + lttng_action_stop_session_set_firing_policy, "stop"); } static struct lttng_action *handle_action_rotate_session(int *argc, const char ***argv) { - return handle_action_simple_session(argc, argv, + return handle_action_simple_session_with_policy(argc, argv, lttng_action_rotate_session_create, lttng_action_rotate_session_set_session_name, + lttng_action_rotate_session_set_firing_policy, "rotate"); } @@ -1473,6 +1734,8 @@ static const struct argpar_opt_descr snapshot_action_opt_descrs[] = { { OPT_DATA_URL, '\0', "data-url", true }, { OPT_URL, '\0', "url", true }, { OPT_PATH, '\0', "path", true }, + { OPT_FIRE_ONCE_AFTER, '\0', "fire-once-after", true }, + { OPT_FIRE_EVERY, '\0', "fire-every", true }, ARGPAR_OPT_DESCR_SENTINEL }; @@ -1491,8 +1754,11 @@ struct lttng_action *handle_action_snapshot_session(int *argc, char *url_arg = NULL; char *path_arg = NULL; char *error = NULL; + char *fire_once_after_str = NULL; + char *fire_every_str = NULL; enum lttng_action_status action_status; struct lttng_snapshot_output *snapshot_output = NULL; + struct lttng_firing_policy *policy = NULL; int ret; unsigned int locations_specified = 0; @@ -1560,6 +1826,27 @@ struct lttng_action *handle_action_snapshot_session(int *argc, } break; + case OPT_FIRE_ONCE_AFTER: + { + if (!assign_string(&fire_once_after_str, + item_opt->arg, + "--fire-once-after")) { + goto error; + } + + break; + } + case OPT_FIRE_EVERY: + { + if (!assign_string(&fire_every_str, + item_opt->arg, + "--fire-every")) { + goto error; + } + + break; + } + default: abort(); } @@ -1623,6 +1910,56 @@ struct lttng_action *handle_action_snapshot_session(int *argc, } } + /* Any firing policy ? */ + if (fire_once_after_str && fire_every_str) { + ERR("--fire-once and --fire-every are mutually exclusive."); + goto error; + } + + if (fire_once_after_str) { + unsigned long long threshold; + + if (utils_parse_unsigned_long_long( + fire_once_after_str, &threshold) != 0) { + ERR("Failed to parse `%s` as an integer.", + fire_once_after_str); + goto error; + } + + if (threshold == 0) { + ERR("Once after N policy threshold cannot be `0`."); + goto error; + } + + policy = lttng_firing_policy_once_after_n_create(threshold); + if (!policy) { + ERR("Failed to create policy once after `%s`.", + fire_once_after_str); + goto error; + } + } + + if (fire_every_str) { + unsigned long long interval; + if (utils_parse_unsigned_long_long(fire_every_str, &interval) != + 0) { + ERR("Failed to parse `%s` as an integer.", + fire_every_str); + goto error; + } + if (interval == 0) { + ERR("Every N policy interval cannot be `0`."); + goto error; + } + + policy = lttng_firing_policy_every_n_create(interval); + if (!policy) { + ERR("Failed to create policy every `%s`.", + fire_every_str); + goto error; + } + } + action = lttng_action_snapshot_session_create(); if (!action) { ERR("Failed to allocate snapshot session action."); @@ -1744,6 +2081,16 @@ struct lttng_action *handle_action_snapshot_session(int *argc, snapshot_output = NULL; } + if (policy) { + enum lttng_action_status status; + status = lttng_action_snapshot_session_set_firing_policy( + action, policy); + if (status != LTTNG_ACTION_STATUS_OK) { + ERR("Failed to set firing policy"); + goto error; + } + } + goto end; error: @@ -1758,6 +2105,7 @@ end: free(data_url_arg); free(snapshot_output); free(max_size_arg); + lttng_firing_policy_destroy(policy); argpar_state_destroy(state); argpar_item_destroy(item); return action; @@ -1827,8 +2175,6 @@ struct argpar_opt_descr add_trigger_options[] = { { OPT_CONDITION, '\0', "condition", false }, { OPT_ACTION, '\0', "action", false }, { OPT_ID, '\0', "id", true }, - { OPT_FIRE_ONCE_AFTER, '\0', "fire-once-after", true }, - { OPT_FIRE_EVERY, '\0', "fire-every", true }, { OPT_USER_ID, '\0', "user-id", true }, ARGPAR_OPT_DESCR_SENTINEL, }; @@ -1856,8 +2202,6 @@ int cmd_add_trigger(int argc, const char **argv) char *error = NULL; char *id = NULL; int i; - char *fire_once_after_str = NULL; - char *fire_every_str = NULL; char *user_id = NULL; lttng_dynamic_pointer_array_init(&actions, lttng_actions_destructor); @@ -1964,24 +2308,6 @@ int cmd_add_trigger(int argc, const char **argv) break; } - case OPT_FIRE_ONCE_AFTER: - { - if (!assign_string(&fire_once_after_str, item_opt->arg, - "--fire-once-after")) { - goto error; - } - - break; - } - case OPT_FIRE_EVERY: - { - if (!assign_string(&fire_every_str, item_opt->arg, - "--fire-every")) { - goto error; - } - - break; - } case OPT_USER_ID: { if (!assign_string(&user_id, item_opt->arg, @@ -2006,11 +2332,6 @@ int cmd_add_trigger(int argc, const char **argv) goto error; } - if (fire_every_str && fire_once_after_str) { - ERR("Can't specify both --fire-once-after and --fire-every."); - goto error; - } - action_group = lttng_action_group_create(); if (!action_group) { goto error; @@ -2049,41 +2370,6 @@ int cmd_add_trigger(int argc, const char **argv) } } - if (fire_once_after_str) { - unsigned long long threshold; - enum lttng_trigger_status trigger_status; - - if (utils_parse_unsigned_long_long(fire_once_after_str, &threshold) != 0) { - ERR("Failed to parse `%s` as an integer.", fire_once_after_str); - goto error; - } - - trigger_status = lttng_trigger_set_firing_policy(trigger, - LTTNG_TRIGGER_FIRING_POLICY_ONCE_AFTER_N, - threshold); - if (trigger_status != LTTNG_TRIGGER_STATUS_OK) { - ERR("Failed to set trigger's policy to `fire once after N`."); - goto error; - } - } - - if (fire_every_str) { - unsigned long long threshold; - enum lttng_trigger_status trigger_status; - - if (utils_parse_unsigned_long_long(fire_every_str, &threshold) != 0) { - ERR("Failed to parse `%s` as an integer.", fire_every_str); - goto error; - } - - trigger_status = lttng_trigger_set_firing_policy(trigger, - LTTNG_TRIGGER_FIRING_POLICY_EVERY_N, threshold); - if (trigger_status != LTTNG_TRIGGER_STATUS_OK) { - ERR("Failed to set trigger's policy to `fire every N`."); - goto error; - } - } - if (user_id) { enum lttng_trigger_status trigger_status; char *end; @@ -2125,8 +2411,6 @@ end: lttng_trigger_destroy(trigger); free(error); free(id); - free(fire_once_after_str); - free(fire_every_str); free(user_id); return ret; } diff --git a/src/bin/lttng/commands/list_triggers.c b/src/bin/lttng/commands/list_triggers.c index 512f25013..3321aacd3 100644 --- a/src/bin/lttng/commands/list_triggers.c +++ b/src/bin/lttng/commands/list_triggers.c @@ -454,6 +454,7 @@ void print_one_action(const struct lttng_action *action) { enum lttng_action_type action_type; enum lttng_action_status action_status; + const struct lttng_firing_policy *policy = NULL; const char *value; action_type = lttng_action_get_type(action); @@ -461,25 +462,53 @@ void print_one_action(const struct lttng_action *action) switch (action_type) { case LTTNG_ACTION_TYPE_NOTIFY: - MSG("notify"); + _MSG("notify"); + + action_status = lttng_action_notify_get_firing_policy( + action, &policy); + if (action_status != LTTNG_ACTION_STATUS_OK) { + ERR("Failed to retrieve firing policy."); + goto end; + } break; case LTTNG_ACTION_TYPE_START_SESSION: action_status = lttng_action_start_session_get_session_name( action, &value); assert(action_status == LTTNG_ACTION_STATUS_OK); - MSG("start session `%s`", value); + _MSG("start session `%s`", value); + + action_status = lttng_action_start_session_get_firing_policy( + action, &policy); + if (action_status != LTTNG_ACTION_STATUS_OK) { + ERR("Failed to retrieve firing policy."); + goto end; + } break; case LTTNG_ACTION_TYPE_STOP_SESSION: action_status = lttng_action_stop_session_get_session_name( action, &value); assert(action_status == LTTNG_ACTION_STATUS_OK); - MSG("stop session `%s`", value); + _MSG("stop session `%s`", value); + + action_status = lttng_action_stop_session_get_firing_policy( + action, &policy); + if (action_status != LTTNG_ACTION_STATUS_OK) { + ERR("Failed to retrieve firing policy."); + goto end; + } break; case LTTNG_ACTION_TYPE_ROTATE_SESSION: action_status = lttng_action_rotate_session_get_session_name( action, &value); assert(action_status == LTTNG_ACTION_STATUS_OK); - MSG("rotate session `%s`", value); + _MSG("rotate session `%s`", value); + + action_status = lttng_action_rotate_session_get_firing_policy( + action, &policy); + if (action_status != LTTNG_ACTION_STATUS_OK) { + ERR("Failed to retrieve firing policy."); + goto end; + } break; case LTTNG_ACTION_TYPE_SNAPSHOT_SESSION: { @@ -534,13 +563,61 @@ void print_one_action(const struct lttng_action *action) } } - MSG(""); + action_status = lttng_action_snapshot_session_get_firing_policy( + action, &policy); + if (action_status != LTTNG_ACTION_STATUS_OK) { + ERR("Failed to retrieve firing policy."); + goto end; + } break; } - default: abort(); } + + if (policy) { + enum lttng_firing_policy_type policy_type; + enum lttng_firing_policy_status policy_status; + uint64_t policy_value = 0; + + policy_type = lttng_firing_policy_get_type(policy); + + switch (policy_type) { + case LTTNG_FIRING_POLICY_TYPE_EVERY_N: + policy_status = lttng_firing_policy_every_n_get_interval( + policy, &policy_value); + if (policy_status != LTTNG_FIRING_POLICY_STATUS_OK) { + ERR("Failed to get action firing policy interval"); + goto end; + } + if (policy_value > 1) { + /* The default is 1 so print only when it is a + * special case. + */ + _MSG(", firing policy: after every %" PRIu64 + " occurrences", + policy_value); + } + break; + case LTTNG_FIRING_POLICY_TYPE_ONCE_AFTER_N: + policy_status = lttng_firing_policy_once_after_n_get_threshold( + policy, &policy_value); + if (policy_status != LTTNG_FIRING_POLICY_STATUS_OK) { + ERR("Failed to get action firing policy interval"); + goto end; + } + _MSG(", firing policy: once after %" PRIu64 + " occurrences", + policy_value); + break; + default: + abort(); + } + } + + MSG(""); +end: + return; } static @@ -552,8 +629,6 @@ void print_one_trigger(const struct lttng_trigger *trigger) enum lttng_action_type action_type; enum lttng_trigger_status trigger_status; const char *name; - enum lttng_trigger_firing_policy firing_policy_type; - uint64_t threshold; uid_t trigger_uid; trigger_status = lttng_trigger_get_name(trigger, &name); @@ -565,26 +640,6 @@ void print_one_trigger(const struct lttng_trigger *trigger) MSG("- id: %s", name); MSG(" user id: %d", trigger_uid); - trigger_status = lttng_trigger_get_firing_policy( - trigger, &firing_policy_type, &threshold); - if (trigger_status != LTTNG_TRIGGER_STATUS_OK) { - ERR("Failed to get trigger's policy."); - goto end; - } - - switch (firing_policy_type) { - case LTTNG_TRIGGER_FIRING_POLICY_EVERY_N: - if (threshold > 1) { - MSG(" firing policy: after every %" PRIu64 " occurences", threshold); - } - break; - case LTTNG_TRIGGER_FIRING_POLICY_ONCE_AFTER_N: - MSG(" firing policy: once after %" PRIu64 " occurences", threshold); - break; - default: - abort(); - } - condition = lttng_trigger_get_const_condition(trigger); condition_type = lttng_condition_get_type(condition); MSG(" condition: %s", lttng_condition_type_str(condition_type)); @@ -621,8 +676,6 @@ void print_one_trigger(const struct lttng_trigger *trigger) print_one_action(action); } -end: - return; } static diff --git a/src/common/trigger.c b/src/common/trigger.c index b79b9e044..125c871ea 100644 --- a/src/common/trigger.c +++ b/src/common/trigger.c @@ -62,9 +62,6 @@ struct lttng_trigger *lttng_trigger_create( urcu_ref_init(&trigger->ref); - trigger->firing_policy.type = LTTNG_TRIGGER_FIRING_POLICY_EVERY_N; - trigger->firing_policy.threshold = 1; - lttng_condition_get(condition); trigger->condition = condition; @@ -133,23 +130,6 @@ void lttng_trigger_destroy(struct lttng_trigger *trigger) lttng_trigger_put(trigger); } -static bool is_firing_policy_valid(enum lttng_trigger_firing_policy policy) -{ - bool valid = false; - - switch (policy) { - case LTTNG_TRIGGER_FIRING_POLICY_EVERY_N: - case LTTNG_TRIGGER_FIRING_POLICY_ONCE_AFTER_N: - valid = true; - break; - default: - valid = false; - break; - } - - return valid; -} - LTTNG_HIDDEN ssize_t lttng_trigger_create_from_payload( struct lttng_payload_view *src_view, @@ -160,8 +140,6 @@ ssize_t lttng_trigger_create_from_payload( struct lttng_action *action = NULL; const struct lttng_trigger_comm *trigger_comm; const char *name = NULL; - uint64_t firing_policy_threshold; - enum lttng_trigger_firing_policy firing_policy; struct lttng_credentials creds = { .uid = LTTNG_OPTIONAL_INIT_UNSET, .gid = LTTNG_OPTIONAL_INIT_UNSET, @@ -196,13 +174,6 @@ ssize_t lttng_trigger_create_from_payload( offset += sizeof(*trigger_comm); - firing_policy = trigger_comm->firing_policy_type; - if (!is_firing_policy_valid(firing_policy)) { - ret =-1; - goto end; - } - - firing_policy_threshold = trigger_comm->firing_policy_threshold; if (trigger_comm->name_length != 0) { /* Name. */ const struct lttng_payload_view name_view = @@ -291,19 +262,6 @@ ssize_t lttng_trigger_create_from_payload( } } - /* Set the policy. */ - { - const enum lttng_trigger_status status = - lttng_trigger_set_firing_policy(trigger, - firing_policy, - firing_policy_threshold); - - if (status != LTTNG_TRIGGER_STATUS_OK) { - ret = -1; - goto end; - } - } - ret = offset; error: @@ -345,8 +303,6 @@ int lttng_trigger_serialize(const struct lttng_trigger *trigger, } trigger_comm.name_length = size_name; - trigger_comm.firing_policy_type = (uint8_t) trigger->firing_policy.type; - trigger_comm.firing_policy_threshold = (uint64_t) trigger->firing_policy.threshold; header_offset = payload->buffer.size; ret = lttng_dynamic_buffer_append(&payload->buffer, &trigger_comm, @@ -385,14 +341,6 @@ LTTNG_HIDDEN bool lttng_trigger_is_equal( const struct lttng_trigger *a, const struct lttng_trigger *b) { - if (a->firing_policy.type != b->firing_policy.type) { - return false; - } - - if (a->firing_policy.threshold != b->firing_policy.threshold) { - return false; - } - if (strcmp(a->name, b->name) != 0) { return false; } @@ -796,97 +744,6 @@ end: return ret; } -enum lttng_trigger_status lttng_trigger_set_firing_policy( - struct lttng_trigger *trigger, - enum lttng_trigger_firing_policy policy_type, - uint64_t threshold) -{ - enum lttng_trigger_status ret = LTTNG_TRIGGER_STATUS_OK; - assert(trigger); - - if (threshold < 1) { - ret = LTTNG_TRIGGER_STATUS_INVALID; - goto end; - } - - trigger->firing_policy.type = policy_type; - trigger->firing_policy.threshold = threshold; - -end: - return ret; -} - -enum lttng_trigger_status lttng_trigger_get_firing_policy( - const struct lttng_trigger *trigger, - enum lttng_trigger_firing_policy *policy_type, - uint64_t *threshold) -{ - enum lttng_trigger_status status = LTTNG_TRIGGER_STATUS_OK; - - if (!trigger || !policy_type || !threshold) { - status = LTTNG_TRIGGER_STATUS_INVALID; - goto end; - } - - *policy_type = trigger->firing_policy.type; - *threshold = trigger->firing_policy.threshold; - -end: - return status; -} - -LTTNG_HIDDEN -bool lttng_trigger_should_fire(const struct lttng_trigger *trigger) -{ - bool ready_to_fire = false; - - assert(trigger); - - switch (trigger->firing_policy.type) { - case LTTNG_TRIGGER_FIRING_POLICY_EVERY_N: - if (trigger->firing_policy.current_count < trigger->firing_policy.threshold) { - ready_to_fire = true; - } - break; - case LTTNG_TRIGGER_FIRING_POLICY_ONCE_AFTER_N: - if (trigger->firing_policy.current_count < trigger->firing_policy.threshold) { - ready_to_fire = true; - } - break; - default: - abort(); - }; - - return ready_to_fire; -} - -LTTNG_HIDDEN -void lttng_trigger_fire(struct lttng_trigger *trigger) -{ - assert(trigger); - - trigger->firing_policy.current_count++; - - switch (trigger->firing_policy.type) { - case LTTNG_TRIGGER_FIRING_POLICY_EVERY_N: - if (trigger->firing_policy.current_count == trigger->firing_policy.threshold) { - trigger->firing_policy.current_count = 0; - } - - break; - case LTTNG_TRIGGER_FIRING_POLICY_ONCE_AFTER_N: - /* - * TODO: - * As an optimisation, deactivate the trigger condition and - * remove any checks in the traced application or kernel since - * the trigger will never fire again. - */ - break; - default: - abort(); - }; -} - LTTNG_HIDDEN enum lttng_domain_type lttng_trigger_get_underlying_domain_type_restriction( const struct lttng_trigger *trigger) diff --git a/tests/regression/tools/trigger/test_add_trigger_cli b/tests/regression/tools/trigger/test_add_trigger_cli index db325a90d..52c29120d 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 222 +plan_tests 231 FULL_LTTNG_BIN="${TESTDIR}/../src/bin/lttng/${LTTNG_BIN}" @@ -46,7 +46,9 @@ function test_success () shift diag "${FULL_LTTNG_BIN} add-trigger $*" + set -x "${FULL_LTTNG_BIN}" add-trigger "$@" > "${tmp_stdout}" 2> "${tmp_stderr}" + set +x ok $? "${test_name}: exit code is 0" diff -u "${tmp_stdout}" <(echo "Trigger registered successfully.") @@ -89,15 +91,40 @@ test_success "--condition on-event -a -u" \ --condition on-event -a -u \ --action notify -test_success "--fire-once-after" \ +test_success "notify action polices" \ --condition on-event -u test-fire-once-after \ --action notify \ + --fire-every=55 \ + --action notify \ --fire-once-after=55 -test_success "--fire-every" \ - --condition on-event -u test-fire-every \ - --action notify \ - --fire-every=55 +test_success "start session action polices" \ + --condition on-event -u test-fire-once-after \ + --action start-session my_session \ + --fire-every=55 \ + --action start-session my_session \ + --fire-once-after=55 + +test_success "stop session action polices" \ + --condition on-event -u test-fire-once-after \ + --action stop-session my_session \ + --fire-every=55 \ + --action stop-session my_session \ + --fire-once-after=55 + +test_success "snapshot session action polices" \ + --condition on-event -u test-fire-once-after \ + --action snapshot-session my_session \ + --fire-every=55 \ + --action snapshot-session my_session \ + --fire-once-after=55 + +test_success "rotate session action polices" \ + --condition on-event -u test-fire-once-after \ + --action rotate-session my_session \ + --fire-every=55 \ + --action rotate-session my_session \ + --fire-once-after=55 skip $ist_root "non-root user: skipping kprobe tests" 9 || { test_success "--condition on-event probe by symbol" \ diff --git a/tests/regression/tools/trigger/test_list_triggers_cli b/tests/regression/tools/trigger/test_list_triggers_cli index a11f8eca9..ac7f09925 100755 --- a/tests/regression/tools/trigger/test_list_triggers_cli +++ b/tests/regression/tools/trigger/test_list_triggers_cli @@ -23,7 +23,7 @@ TESTDIR="$CURDIR/../../.." # shellcheck source=../../../utils/utils.sh source "$TESTDIR/utils/utils.sh" -plan_tests 44 +plan_tests 49 FULL_LTTNG_BIN="${TESTDIR}/../src/bin/lttng/${LTTNG_BIN}" @@ -63,26 +63,8 @@ test_top_level_options () lttng_add_trigger_ok "hello" --condition on-event -u test-id --action notify - lttng_add_trigger_ok "T0" --fire-once-after 123 --condition on-event -u test-fire-once-after --action notify - lttng_add_trigger_ok "T1" --fire-every 124 --condition on-event -u test-fire-every --action notify cat > "${tmp_expected_stdout}" <<- EOF - - id: T0 - user id: ${uid} - firing policy: once after 123 occurences - condition: event rule hit - rule: test-fire-once-after (type: tracepoint, domain: ust) - tracer notifications discarded: 0 - actions: - notify - - id: T1 - user id: ${uid} - firing policy: after every 124 occurences - condition: event rule hit - rule: test-fire-every (type: tracepoint, domain: ust) - tracer notifications discarded: 0 - actions: - notify - id: hello user id: ${uid} condition: event rule hit @@ -314,6 +296,8 @@ test_snapshot_action () lttng_add_trigger_ok "T5" --condition on-event -u some-event --action snapshot-session ze-session --ctrl-url=tcp://1.2.3.4:1111 --data-url=tcp://1.2.3.4:1112 lttng_add_trigger_ok "T6" --condition on-event -u some-event --action snapshot-session ze-session --path /some/path --max-size=1234 lttng_add_trigger_ok "T7" --condition on-event -u some-event --action snapshot-session ze-session --path /some/path --name=meh + lttng_add_trigger_ok "T8" --condition on-event -u some-event --action snapshot-session ze-session --fire-every 10 + lttng_add_trigger_ok "T9" --condition on-event -u some-event --action snapshot-session ze-session --fire-once-after 10 cat > "${tmp_expected_stdout}" <<- EOF @@ -373,6 +357,50 @@ test_snapshot_action () tracer notifications discarded: 0 actions: snapshot session \`ze-session\`, path: /some/path, name: meh + - id: T8 + user id: ${uid} + condition: event rule hit + rule: some-event (type: tracepoint, domain: ust) + tracer notifications discarded: 0 + actions: + snapshot session \`ze-session\`, firing policy: after every 10 occurrences + - id: T9 + user id: ${uid} + condition: event rule hit + rule: some-event (type: tracepoint, domain: ust) + tracer notifications discarded: 0 + actions: + snapshot session \`ze-session\`, firing policy: once after 10 occurrences + EOF + + list_triggers "snapshot action" "${tmp_expected_stdout}" + + stop_lttng_sessiond_notap +} + +test_notify_action () +{ + start_lttng_sessiond_notap + + lttng_add_trigger_ok "T0" --condition on-event -u some-event --action notify --fire-once-after 5 + lttng_add_trigger_ok "T1" --condition on-event -u some-event --action notify --fire-every 10 + + + cat > "${tmp_expected_stdout}" <<- EOF + - id: T0 + user id: ${uid} + condition: event rule hit + rule: some-event (type: tracepoint, domain: ust) + tracer notifications discarded: 0 + actions: + notify, firing policy: once after 5 occurrences + - id: T1 + user id: ${uid} + condition: event rule hit + rule: some-event (type: tracepoint, domain: ust) + tracer notifications discarded: 0 + actions: + notify, firing policy: after every 10 occurrences EOF list_triggers "snapshot action" "${tmp_expected_stdout}" @@ -386,6 +414,7 @@ skip $ist_root "non-root user: skipping kprobe tests" 6 || test_on_event_probe skip $ist_root "non-root user: skipping uprobe tests" 4 || test_on_event_userspace_probe skip $ist_root "non-root user: skipping syscall tests" 5 || test_on_event_syscall test_snapshot_action +test_notify_action # Cleanup rm -f "${tmp_stdout}"