Move firing policy from lttng_trigger to lttng_action
authorJonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Tue, 30 Mar 2021 01:43:12 +0000 (21:43 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Sun, 18 Apr 2021 23:28:57 +0000 (19:28 -0400)
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 <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib29d4319922096c0c4b3f00782f3bbefb17e2f40

include/lttng/trigger/trigger-internal.h
include/lttng/trigger/trigger.h
src/bin/lttng-sessiond/notification-thread-events.c
src/bin/lttng/commands/add_trigger.c
src/bin/lttng/commands/list_triggers.c
src/common/trigger.c
tests/regression/tools/trigger/test_add_trigger_cli
tests/regression/tools/trigger/test_list_triggers_cli

index 59fe771aa32ef196ef3e89af5c87466dadf5cf21..c5ee9c64b4c1db9642251417fa812f3e18a9a68d 100644 (file)
@@ -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.
index 3b1cedda1fb4709457537ded9df978e206d6d3ca..6490af118a4333eb857d203d46dfce05a78455aa 100644 (file)
@@ -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.
  */
index 827cf2c1459f5ee4cb683ac3fd5b0699ab5d9b82..29318432f842b2f26193388aa563da350ed8ae04 100644 (file)
@@ -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.
index ef5b167c535c8667f565525c2f55751bd6d933bc..17e06445b49ca9f8d6b9a935beecd77c38ca05a8 100644 (file)
@@ -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;
 }
index 512f250138fa906ade4073f012374328732d22d5..3321aacd3f9443cb22d7d078e786940ceaae66d2 100644 (file)
@@ -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
index b79b9e0448c6e624e6e819dc3a1b9fe254dc8382..125c871eaf80c98d63579713866d88339de4f760 100644 (file)
@@ -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)
index db325a90d61884225eab325f1de7c9a211c500c4..52c29120dc2b328b3b3aaf367352d8ed43410595 100755 (executable)
@@ -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" \
index a11f8eca9de028ed43b93b48826fae3cdca875b5..ac7f099251a13781eef3d76bec511225c216c934 100755 (executable)
@@ -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}"
This page took 0.06887 seconds and 4 git commands to generate.