Fix: relayd: harmonize path format in backward-compat mode

Observed issue
==============

Currently, the relay daemon produces the following path formats
depending on the whether a tracepath is provided, the version of the
session daemon peer, and the grouping option specified on launch of
the relay daemon.

Hostname grouping, no custom trace path
pre 2.11: $BASE/$SHOSTNAME/$SESSION-$DATETIME
    2.12: $BASE/$SHOSTNAME/$SESSION-$DATETIME

Hostname grouping, custom trace path
pre 2.11: $BASE/$HOSTNAME/$TRACEPATH
    2.12: $BASE/$HOSTNAME/$TRACEPATH

Tracing session grouping, no custom trace path
pre 2.11: $BASE/$SESSION/$HOSTNAME-$DATETIME
    2.12: $BASE/$SESSION/$HOSTNAME-$DATETIME

Tracing session grouping, custom trace path
pre 2.11: $BASE/$SESSION/$HOSTNAME/$TRACEPATH
    2.12: $BASE/$SESSION/$HOSTNAME-$DATETIME/$TRACEPATH

As you can see, there is a single case where the format
diverges based on the version of the session daemon.

Cause
=====

Pre-2.11 session daemons do not transmit a session creation time when
a TRACEPATH is specified as part of the streaming url (e.g.
`lttng create my_session --set-url net://localhost/a_path`)

Hence, the backward compatibility path formatting code does not
insert a "DATETIME" string in the resulting path.

Solution
========

The relay daemon samples the time when it creates its session and that
time is formatted into the DATETIME representation if no DATETIME is
present in the path provided by pre-2.11 peers.

Drawbacks
=========

Sampling the relay session creation time will not yield the exact same
behaviour as what a 2.11+ peer would produce, but it is a reasonable
approximation for most use-cases.

Users depending on this time being the exact same as that sampled by
the session daemon will need to adapt tools anyhow if they use the new
--group-output-by-session option, so the change doesn't introduce more
problems.

This behaviour can be surprising when snapshots are streamed by
pre-2.11 peers as the session creation DATETIME will be different for
all snapshots. This is not ideal, but still less jarring than getting
a completely different path format depending on a peer's version.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3316aa35a34985e83ae759851af3a899b0011789
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-relayd/backward-compatibility-group-by.c b/src/bin/lttng-relayd/backward-compatibility-group-by.c
index 45bbe17284b551a5f7ddb5b667a445bfc486b725..241c21f469b8356b813926037550ab60cbb8c8c7 100644 (file)
--- a/src/bin/lttng-relayd/backward-compatibility-group-by.c
+++ b/src/bin/lttng-relayd/backward-compatibility-group-by.c
@@ -5,6 +5,7 @@
  *
  */
 
+#include "common/time.h"
 #include <assert.h>
 #include <regex.h>
 #include <stdio.h>
@@ -28,8 +29,9 @@
  *
  * Return the allocated string containing the new stream path or else NULL.
  */
-char *backward_compat_group_by_session(
-               const char *path, const char *local_session_name)
+char *backward_compat_group_by_session(const char *path,
+               const char *local_session_name,
+               time_t relay_session_creation_time)
 {
        int ret;
        size_t len;
@@ -162,7 +164,7 @@ char *backward_compat_group_by_session(
                ret = regexec(&regex, local_session_name, 0, NULL, 0);
                if (ret == 0) {
                        const ssize_t local_session_name_offset =
-                                       strlen(local_session_name) - DATETIME_STRING_SIZE + 1;
+                                       strlen(local_session_name) - DATETIME_STR_LEN + 1;
 
                        assert(local_session_name_offset >= 0);
                        datetime = strdup(local_session_name +
@@ -171,8 +173,21 @@ char *backward_compat_group_by_session(
                                PERROR("Failed to parse session path: couldn't copy datetime on regex match");
                                goto error_regex;
                        }
+               } else {
+                       datetime = zmalloc(DATETIME_STR_LEN);
+                       if (!datetime) {
+                               PERROR("Failed to allocate DATETIME string");
+                               goto error;
+                       }
+
+                       ret = time_to_datetime_str(relay_session_creation_time,
+                                       datetime, DATETIME_STR_LEN);
+                       if (ret) {
+                               /* time_to_datetime_str already logs errors. */
+                               goto error;
+                       }
                }
-       } else if (len == DATETIME_STRING_SIZE &&
+       } else if (len == DATETIME_STR_LEN &&
                        !regexec(&regex, leftover_second_token_ptr, 0, NULL,
                                        0)) {
                /*

diff --git a/src/bin/lttng-relayd/backward-compatibility-group-by.h b/src/bin/lttng-relayd/backward-compatibility-group-by.h
index 9d8aced508a74c42f97f90c9d8cc884024e5dc2c..4cf7462de55fb22cc72eb42211c3d389f04cea9a 100644 (file)
--- a/src/bin/lttng-relayd/backward-compatibility-group-by.h
+++ b/src/bin/lttng-relayd/backward-compatibility-group-by.h
@@ -8,7 +8,10 @@
 #ifndef BACKWARD_COMPATIBILITY_GROUP_BY_H
 #define BACKWARD_COMPATIBILITY_GROUP_BY_H
 
-char *backward_compat_group_by_session(
-               const char *path, const char *local_session_name);
+#include <time.h>
+
+char *backward_compat_group_by_session(const char *path,
+               const char *local_session_name,
+               time_t session_creation_time);
 
 #endif /* BACKWARD_COMPATIBILITY_GROUP_BY_H */

diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
index 71a2d925f4bfa0fa983410ebc1b297573b39d561..5f70df481d9f60f0aec35d1aa1d34894aeed577b 100644 (file)
--- a/src/bin/lttng-relayd/main.c
+++ b/src/bin/lttng-relayd/main.c
@@ -1632,7 +1632,8 @@ static int relay_add_stream(const struct lttcomm_relayd_hdr *recv_hdr,
                        group_by_session_path_name =
                                        backward_compat_group_by_session(
                                                        path_name,
-                                                       session->session_name);
+                                                       session->session_name,
+                                                       session->creation_time.value);
                        if (!group_by_session_path_name) {
                                ERR("Failed to apply group by session to stream of session %" PRIu64,
                                                session->id);

diff --git a/src/bin/lttng-relayd/session.c b/src/bin/lttng-relayd/session.c
index 9f690d3af68d5eaf0a319b2c8439534587249101..6c30821ff1348a405816af553a7b2951120c8a5c 100644 (file)
--- a/src/bin/lttng-relayd/session.c
+++ b/src/bin/lttng-relayd/session.c
@@ -343,6 +343,12 @@ struct relay_session *session_create(const char *session_name,
        }
        if (creation_time) {
                LTTNG_OPTIONAL_SET(&session->creation_time, *creation_time);
+       } else {
+               LTTNG_OPTIONAL_SET(&session->creation_time, time(NULL));
+               if (session->creation_time.value == (time_t) -1) {
+                       PERROR("Failed to sample session creation time");
+                       goto error;
+               }
        }
        session->session_name_contains_creation_time =
                        session_name_contains_creation_time;

diff --git a/src/bin/lttng-relayd/session.h b/src/bin/lttng-relayd/session.h
index 9f668b5fbc0ed4fc522079a2a6e621ab12e55d54..3a9b92436f3aa44601d8b65ea0b050fe06b5bce3 100644 (file)
--- a/src/bin/lttng-relayd/session.h
+++ b/src/bin/lttng-relayd/session.h
@@ -42,6 +42,11 @@ struct relay_session {
         * the other cases.
         */
        lttng_uuid sessiond_uuid;
+       /*
+        * Contains the creation time on the session daemon's end for 2.11+
+        * peers. Otherwise, this contains the session creation time on the
+        * relay daemon's end.
+        */
        LTTNG_OPTIONAL(time_t) creation_time;
        /* Must _not_ be empty for 2.4+ peers. */
        char session_name[LTTNG_NAME_MAX];

diff --git a/tests/unit/test_relayd_backward_compat_group_by_session.c b/tests/unit/test_relayd_backward_compat_group_by_session.c
index d068372faff80e4b2282b960cda8d45981ce6af7..0ebcf34ab87cf873bd1ad6df6c54807ba61c6a60 100644 (file)
--- a/tests/unit/test_relayd_backward_compat_group_by_session.c
+++ b/tests/unit/test_relayd_backward_compat_group_by_session.c
@@ -12,6 +12,8 @@
 #include <string.h>
 #include <tap/tap.h>
 
+#include <common/time.h>
+
 #include "backward-compatibility-group-by.h"
 
 /* Number of TAP tests in this file */
@@ -97,19 +99,32 @@ struct test tests[] = {
                                "", "ust/uid/1000/64-bit", true},
 };
 
-static char *craft_expected(struct test *test)
+static char *craft_expected(struct test *test, time_t relay_session_creation_time)
 {
        int ret;
        char *result = NULL;
+       char relay_session_creation_datetime[DATETIME_STR_LEN];
+
+       ret = time_to_datetime_str(relay_session_creation_time,
+                       relay_session_creation_datetime,
+                       sizeof(relay_session_creation_datetime));
+       if (ret < 0) {
+               result = NULL;
+               goto end;
+       }
 
-       ret = asprintf(&result, "%s/%s%s%s/%s%s%s", test->session_name,
+       ret = asprintf(&result, "%s/%s-%s/%s%s%s", test->session_name,
                        test->hostname,
-                       test->creation_time[0] != '\0' ? "-" : "",
-                       test->creation_time, test->extra_path,
+                       test->creation_time[0] == '\0' ?
+                                       relay_session_creation_datetime :
+                                       test->creation_time,
+                       test->extra_path,
                        test->extra_path[0] != '\0' ? "/" : "", test->leftover);
        if (ret < 0) {
                result = NULL;
+               goto end;
        }
+end:
        return result;
 }
 
@@ -117,21 +132,28 @@ int main(int argc, char **argv)
 {
        int i;
        int num_test = sizeof(tests) / sizeof(struct test);
+       const time_t test_time = time(NULL);
 
        plan_tests(NUM_TESTS_PER_TEST * num_test);
        diag("Backward compatibility utils for lttng-relayd --group-by-session");
+
+       if (test_time == (time_t) -1) {
+               perror("Failed to sample time");
+               return exit_status();
+       }
+
        for (i = 0; i < num_test; i++) {
                char *expected = NULL;
                char *result = NULL;
 
-               expected = craft_expected(&tests[i]);
+               expected = craft_expected(&tests[i], test_time);
                if (!expected) {
                        fprintf(stderr, "Failed to craft expected output\n");
                        goto loop;
                }
 
-               result = backward_compat_group_by_session(
-                               tests[i].stream_path, tests[i].session_name);
+               result = backward_compat_group_by_session(tests[i].stream_path,
+                               tests[i].session_name, test_time);
                if (!result && tests[i].is_valid) {
                        fprintf(stderr, "Failed to get result\n");
                        goto loop;