lttng-tools.git
3 months agorelayd: use lfht_iteration_adapter to iterate on stream indices
Jérémie Galarneau [Mon, 22 Jul 2024 19:04:48 +0000 (19:04 +0000)] 
relayd: use lfht_iteration_adapter to iterate on stream indices

offsetof conditionally support for non-POD types and is used by
the cds_lfht_for_each_[...] macros. Replace them with
lfht_iteration_adapter which also provides ranged-for semantics.

Change-Id: Id1460004322e1e778a1affd41c82527a05190f59
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 months agorelayd: offsetof conditionally supported for non-POD types
Jérémie Galarneau [Mon, 22 Jul 2024 19:02:49 +0000 (19:02 +0000)] 
relayd: offsetof conditionally supported for non-POD types

caa_container_of makes use of offsetof which is conditionally
supported for non-POD types. Replace its use by
lttng_ht_node_container_of.

Change-Id: If2f19ab1aba8c59cf9ca2e0d5b815a396512b2d9
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 months agourcu: add lfht_iteration_adapter helper
Jérémie Galarneau [Wed, 17 Jul 2024 20:39:54 +0000 (20:39 +0000)] 
urcu: add lfht_iteration_adapter helper

The urcu lfht macros often make use of caa_container_of (and other
equivalent variations) which use offsetof. Unfortunately, offsetof is
conditionally supported by compilers for non-POD types.

The tree already has lttng::utils::container_of to work around this
problem. This new utils makes it possible to iterate on all of the
elements of an lfht without using those macros. Those iterations are the
main reason such warnings are emitted. The interface of
lfht_iteration_adapter also allows the use of ranged-for loops.

Change-Id: I61906e025bd0dd7512f02180700f3ddb3c9cf3ca
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 months agosessiond: manage syscall table allocation using std::vector
Jérémie Galarneau [Thu, 18 Jul 2024 17:37:43 +0000 (17:37 +0000)] 
sessiond: manage syscall table allocation using std::vector

Transitioning to the use of std::vector to manage the allocation and
resizing of the syscall table allows us to do away with memset() of the
syscall structure. The memset becomes problematic when lttng_ht_node_*
becomes non-POD in a follow-up change.

Change-Id: I94096782360f85ffffb0f72d188224af15e54d07
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 months agoFix: remove use of offset_of for ust_app_session lists
Jérémie Galarneau [Mon, 8 Jul 2024 20:14:36 +0000 (16:14 -0400)] 
Fix: remove use of offset_of for ust_app_session lists

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

3 months agoFix: sessiond: g++ reports shm_path truncation issue
Jérémie Galarneau [Mon, 8 Jul 2024 15:50:01 +0000 (11:50 -0400)] 
Fix: sessiond: g++ reports shm_path truncation issue

On the CI, g++ reports:
  char* strncpy(char*, const char*, size_t)' specified bound 4096 equals destination size [-Wstringop-truncation]

The shm_path provided by liblttng-ctl is truncated to PATH_MAX
regardless of its contents.

LTTN_ERR_INVALID is returned if the shm_path is not null terminated.

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

3 months agoFix: relayd: incorrect use of lfht add_unique API
Jérémie Galarneau [Fri, 19 Jul 2024 20:38:29 +0000 (20:38 +0000)] 
Fix: relayd: incorrect use of lfht add_unique API

cds_lfht_add_unique is misused in relay_index_add_unique. The address
of the node is used instead of the address of the key for both the
key parameter and to compute the hash of the index entry's key.

This caused problems in an upcoming change that changes the layout
of the lttng_ht node since the key is no longer the first member
of the node.

Change-Id: Iec67f78b8ced4f4e520dc0dbebda8d30e283aa38
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
3 months agoAdapt to LTTng-UST ABI/control API updates
Mathieu Desnoyers [Mon, 29 Jul 2024 13:55:50 +0000 (09:55 -0400)] 
Adapt to LTTng-UST ABI/control API updates

Adapt to ABI/control API updates introduced by LTTng-UST commit
24f7193c9b91 ("Introduce extension points for trace hit counters")

Note that this commit reverts back to uint32_t for channel event ids.

UST notification LTTNG_UST_CTL_NOTIFY_CMD_KEY is currently handled as
-LTTNG_UST_ERR_NOSYS (TODO).

The type of the argument to is_max_event_id() is changed to
lttng::sessiond::ust::event_id to match the newly introduced event_id
type definition. Likewise for the registry_channel _next_event_id
member.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I6299b07c02d35b067aed5e94f11353d3cc14f24c

4 months agokernel: Add new counter ABI IOCTL macros
Francis Deslauriers [Fri, 28 May 2021 20:18:05 +0000 (16:18 -0400)] 
kernel: Add new counter ABI IOCTL macros

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Depends-on: lttng-ust: I1a706cf60059c7106525cf484d4a290c38e197c7
Depends-on: lttng-modules: Ide6aa85f4fc6055b7b76604b107094373d0bf874
Change-Id: I1a1d8f265e42e68632863127721e755f7948b660

4 months agosessiond: ust: Update UST definitions for counters
Francis Deslauriers [Fri, 28 May 2021 12:57:47 +0000 (08:57 -0400)] 
sessiond: ust: Update UST definitions for counters

Update the internal ust-abi and ust-ctl headers to match LTTng-UST's
changes to support counter events.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I85fdb02710eaecebab950407d12c85ebfe27615f
Depends-on: lttng-ust: I1a706cf60059c7106525cf484d4a290c38e197c7

4 months agoRevert "lttng-ctl: Hide symbol introduced by fix"
Jérémie Galarneau [Mon, 8 Jul 2024 15:17:52 +0000 (11:17 -0400)] 
Revert "lttng-ctl: Hide symbol introduced by fix"

This reverts commit 83f2539c5c084d853308993a05999016264be90e.
The change is only needed for stable branches.

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

4 months agoClean-up: lock-file: quote lock file path in debug logging message
Jérémie Galarneau [Thu, 14 Mar 2024 19:01:54 +0000 (15:01 -0400)] 
Clean-up: lock-file: quote lock file path in debug logging message

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

4 months agolttng-ctl: Hide symbol introduced by fix
Jérémie Galarneau [Mon, 22 Apr 2024 19:52:28 +0000 (15:52 -0400)] 
lttng-ctl: Hide symbol introduced by fix

Michael Jeanson noticed that f94318611 mistakenly introduces a new
public symbol in liblttng-ctl.so. This change was not intended and is
due to a bad backport of a fix initially developed against the master
branch.

The master branch (and upcoming version) of LTTng-tools is built with
the -fvisibility=hidden. Hence, the initial version of the fix had no
need to hide the utils_create_lock_file symbol.

Since the supported stable releases (2.12 and 2.13) are not built with
those options, that symbol has to be explicitly marked as hidden.

Reported-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I874538317617003eb1c58c2d3b7b0a2bdf905ef7

4 months agosessiond: open_packets: use user_space_consumer_channel_keys util
Jérémie Galarneau [Thu, 20 Jun 2024 18:25:21 +0000 (18:25 +0000)] 
sessiond: open_packets: use user_space_consumer_channel_keys util

Replace the hand-rolled consumer channel key iteration logic of
ust_app_open_packets by an iteration on the user_space_consumer_channel_keys
returned by the ltt_session.

Change-Id: Ida9eaecbc1146764df9935ca9af9cf71d267906d
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agosessiond: clear_session: use user_space_consumer_channel_keys util
Jérémie Galarneau [Thu, 20 Jun 2024 20:07:57 +0000 (20:07 +0000)] 
sessiond: clear_session: use user_space_consumer_channel_keys util

Replace the hand-rolled consumer channel key iteration logic of
ust_app_clear_session by an iteration on the user_space_consumer_channel_keys
returned by the ltt_session.

Change-Id: Id47af95d7cdad55581339c1a5319baf424848912
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agoclang-tidy: apply suggested fixes
Jérémie Galarneau [Thu, 20 Jun 2024 17:31:20 +0000 (17:31 +0000)] 
clang-tidy: apply suggested fixes

Apply FixIts proposed by clang-tidy (mostly missing 'const' qualifiers).

Change-Id: Ic616ddf6e826ee787c048de3f18aec18f9481532
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agorefactor: session: provide an iterator over consumer data channel keys
Jérémie Galarneau [Thu, 13 Jun 2024 21:50:04 +0000 (21:50 +0000)] 
refactor: session: provide an iterator over consumer data channel keys

The code iterating over the various data channel keys of the userspace domain is
relatively complex and repeated in many places. It is refactored to provide an
iterator that can be used in those various places.

Change-Id: I560484f63757b5da0313dd651724bbea9bbd6867
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agosessiond: ust-app: Remove unused buffer_type attribute
Jérémie Galarneau [Thu, 13 Jun 2024 22:00:53 +0000 (22:00 +0000)] 
sessiond: ust-app: Remove unused buffer_type attribute

Change-Id: Iea510844f7d1dec439b56ab509eb510e195464d8
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agosessiond: ust_app: use RAII to manage ust_app_session locking
Jérémie Galarneau [Thu, 13 Jun 2024 21:52:18 +0000 (21:52 +0000)] 
sessiond: ust_app: use RAII to manage ust_app_session locking

Like ltt_session, use the non_copyable_reference util to implement weak locked
reference helpers that are used instead of directly manipulating the
ust_app_session's pthread mutex.

Change-Id: I2c362f2b5944b3d8e089beabc07987b797bb5d36
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agohashtable: replace non-const iterator node accessors by a const version
Jérémie Galarneau [Thu, 13 Jun 2024 19:00:17 +0000 (19:00 +0000)] 
hashtable: replace non-const iterator node accessors by a const version

The various lttng_ht_iter_get_node_* functions are not const-correct which makes
their use difficult in some places. This is mostly due to the fact that
cds_lfht_iter_get_node (from liburcu) is, itself, not const-correct.

These functions are replaced by a single templated function that is
const-correct by virtue of inlining the trivial iterator node accessor of
liburcu.

Change-Id: I4df87abedb1ec6b14eb52ce4c212a10805898954
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agoclang-tidy: sessiond: superfluous static specifier
Jérémie Galarneau [Wed, 29 May 2024 18:32:44 +0000 (18:32 +0000)] 
clang-tidy: sessiond: superfluous static specifier

clang-tidy reports:
'get_session_registry' is a static definition in anonymous namespace; static is redundant here

Remove the redundant static specifier.

Change-Id: Iae7be221cde1f0789d3c3a560f1ea70d4d4e5e7a
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agoclang-tidy: c_string_view: unmatched NOLINTBEGIN
Jérémie Galarneau [Wed, 29 May 2024 18:29:23 +0000 (18:29 +0000)] 
clang-tidy: c_string_view: unmatched NOLINTBEGIN

clang-tidy reports:
'NOLINTBEGIN' comment without a subsequent 'NOLINTEND' comment

If a specific warning is mentionned as part of the NOLINTBEGIN clause, it must
also be mentionned in the matching NOLINTEND clause. Otherwise, clang-tidy
doesn't match them.

Change-Id: I6adef610227b162ba8a5715c3b426e4c49a1b346
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agosessiond: propagate the use of ltt_session::locked_ref
Jérémie Galarneau [Tue, 28 May 2024 03:21:18 +0000 (03:21 +0000)] 
sessiond: propagate the use of ltt_session::locked_ref

The mix of managed and unmanaged ltt_session references makes their use
potentially confusing. Move to managed locked references for most of the client
request processing paths.

Change-Id: Iecb1668969ba07acfd84aa25127adb0addddbf10
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agosessiond: introduce ltt_session::locked_ref look-up functions
Jérémie Galarneau [Fri, 24 May 2024 02:09:04 +0000 (02:09 +0000)] 
sessiond: introduce ltt_session::locked_ref look-up functions

Introduce ltt_session::locked_ref look-up functions and use them at the various
sites performing session look-ups by id or name.

The users of those look-up functions are refactored to make them exception-safe.
The biggest change is that process_client_msg is now exception-safe. The
functions are also moved inside of the ltt_session class as static methods. This
namespaces them implicitly.

A number of functions that expect a locked ltt_session are modified to accept an
ltt_session::locked_ref, thus enforcing the locking assumptions of the session
at compile time.

Change-Id: If9e4f8b25d03fa8d36a5898dd421da947ec4030c
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agoException: make source location print-out optional
Jérémie Galarneau [Wed, 15 May 2024 23:29:55 +0000 (23:29 +0000)] 
Exception: make source location print-out optional

The project-specific exceptions are changed to not include the source location
as part of the exception's message (return of what()) since it is unsuitable in
some contexts (the output of the lttng client for example).

Moreover, the exceptions are modified to provide their own message formatting
instead of relying on the user doing it at the site at which it is thrown since
that formatting is often repetitive. The sites at which the exceptions are
thrown can still augment the exceptions with more context.

The intention of our exceptions is that they still contain the requisite context
to change the print-out based on their members; that context should not be baked
into the message string at the site at which the exception is thrown.

Change-Id: I2733b58d4daf6b9fffc1221c9d03ffef2e246893
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agosessiond: transition session_list synchronization from pthread to std
Jérémie Galarneau [Wed, 22 May 2024 19:48:04 +0000 (19:48 +0000)] 
sessiond: transition session_list synchronization from pthread to std

In order to allow the use of RAII to manage the locking in the client command
handler (process_client_msg), the session list's synchronization primitives are
changed to use those of libstd.

No change in behavior is intended by this change.

Change-Id: I6367e25557b05e16a48436304791cd7a37dc8904
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agosessiond: rename locked_ptr to locked_ref
Jérémie Galarneau [Wed, 22 May 2024 17:20:46 +0000 (17:20 +0000)] 
sessiond: rename locked_ptr to locked_ref

The locked_ref semantics garantee that an instance of an is locked and that it
exists (it can't be null). It is currently implemented with a unique_ptr, but
that will most likely change in the future to enforce the non-null requirement.

Change-Id: I6442840f944ded1ddc46cad3d23e8e0fe8fef586
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agoClean-up: sessiond: session.hpp: remove duplicated comment
Jérémie Galarneau [Wed, 22 May 2024 19:12:48 +0000 (19:12 +0000)] 
Clean-up: sessiond: session.hpp: remove duplicated comment

Change-Id: I65eeb7af77bf9160f94bf6f9c381aeddccb4df31
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agoClean-up: sessiond: client.cpp: list of inclusions
Jérémie Galarneau [Wed, 22 May 2024 18:42:13 +0000 (18:42 +0000)] 
Clean-up: sessiond: client.cpp: list of inclusions

Correct some inconsistencies in the list of inclusions in client.cpp:
  - include headers under common using angle-brackets
  - only include the top-level lttng.h lttng-ctl header

Change-Id: I1dcbacf9d4546205752f2083e1f95f958fbbfe83
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agoFix: sessiond: unchecked null-termination of session name
Jérémie Galarneau [Tue, 21 May 2024 21:56:06 +0000 (21:56 +0000)] 
Fix: sessiond: unchecked null-termination of session name

Change-Id: I56e7d21aafe3eb0472caca59dd6ff578760378f2
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agovscode: export MI_XSD_PATH and LD_LIBRARY_PATH in gdb wrapper
Jérémie Galarneau [Wed, 22 May 2024 00:42:19 +0000 (00:42 +0000)] 
vscode: export MI_XSD_PATH and LD_LIBRARY_PATH in gdb wrapper

vscode fails to launch a session daemon using the libtool wrapper on machines
that don't have lttng installed since the XSD can't be loaded.

Export the MI_XSD_PATH to point to the file in the source tree to allow the
daemon to launch.

Moreover, launching the session daemon fails since it can't find
liblttng-ctl.so.0. Adding the library's .libs to LD_LIBRARY_PATH works around
the issue.

Change-Id: I52929b858917e5a103e067e6861cbdb5172744be
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
4 months agoFix: consumerd: consumed size miscomputed during statistics sampling
Jérémie Galarneau [Wed, 26 Jun 2024 20:09:57 +0000 (20:09 +0000)] 
Fix: consumerd: consumed size miscomputed during statistics sampling

Issue observed
==============

When specifying a size-based automatic rotation, sessions sometimes continuously
rotate after the first size-based rotation is triggered.

Steps to reproduce:

  $ ./lttng create the_test
  $ ./lttng enable-channel -u the_channel
  $ ./lttng enable-rotation --size=32M
  $ ./lttng enable-event -ua --channel the_channel
  $ ./lttng start
  # Produce more than 32MiB of tracing data and stop the application(s), then
  # observe a rotation occuring on every expiration of the channel's monitor timer.

Cause
=====

The consumer daemon samples statistics of the various streams it monitors on
every expiration of their channel's "monitor" timer. One of the statistic it
maintains is the channel's total consumed size.

Once the stats are collected, the consumer sends a channel statistics sample
message to the session daemon. One of the fields of this message is the amount
of data that was consumed for the channel since the last statistics sample was
sent. The session daemon relies on that information to aggregate the consumed
size a session based on the consumed sizes of its various channels.

The consumer maintains a per-channel 'last_consumed_size_sample_sent' which is
used to compute the amount of data consumed since the last statistics sample was
sent to the session daemon. The computation is erroneous since the count sent to
the session daemon is 'the channel's total consumed data amount' minus 'the data
consumed since the last statistics sample'.

Solution
========

The counter maintained on a per-channel basis is now
consumed_size_as_of_last_sample_sent: the total amount of data consumed for that
channel as of the last statistics sample. On every expiration of the monitor
timer, the total amount of data consumed for the channel is determined and the
difference from that counter is sent to the sessiond daemon. The counter is then
updated with the latest computed value.

Known drawbacks
===============

None.

Change-Id: I327fde946826ab1da0b6511a13132f3afba1ac16
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
5 months agotests: Make test_per_application_leaks more robust
Kienan Stewart [Wed, 22 May 2024 14:45:08 +0000 (10:45 -0400)] 
tests: Make test_per_application_leaks more robust

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

On some systems (e.g. sles15sp4, RHEL 7), test_per_application_leaks
would fail spuriously.

Cause
=====

When walking the the FDs in `/proc/XX/fds`, the symbolic links exist but
link to unlinked files. E.g.

```
lrwx------ 1 root root 64 May 22 14:49 /proc/83578/fd/58 -> '/dev/shm/shm-ust-consumer-83578 (deleted)'
```

Solution
========

The test has been modified to loop, waiting for the post-destroy shm
count to drop back to the post-start count. In the case of a failure,
the test will hang forever but doesn't fail spuriously.

Known drawbacks
===============

None.

Change-Id: Id3c8a9f6db83fe888e79b8f06cb8308b4d90da87
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
5 months agolttng: Indicate file path and error reason when open_config fails
Kienan Stewart [Mon, 15 Apr 2024 19:15:28 +0000 (15:15 -0400)] 
lttng: Indicate file path and error reason when open_config fails

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

In a test environment where the configuration file had been chowned to
no longer be writeable by the current user, `lttng` commands would work
but return a non-zero exit code and print `Error: Unable to create
config file`.

This doesn't give the user much information to work with to address the
issue.

Solution
========

The `PWARN` macro is used to print the file_path and reason when the
call to `fopen` fails.

This is done since the `file_path` percolated up to where the existing
error message is may be null for multiple reasons (eg. allocation
failure).

Known drawbacks
===============

The output of `PWARN` is inconsistent with the output used for most of
the `lttng` client.

Eg.

```
$ lttng create
Spawning a session daemon
Session auto-20240415-151450 created.
Traces will be output to /home/lttng-traces/auto-20240415-151450
PWARN - 15:14:50.420628389 [1184515/1184515]: fopen '/home/.lttngrc': Permission denied (in open_config() at conf.cpp:57)
Error: Unable to create config file
```

Change-Id: I5a9253fb02f3a712e7c5c2567311920dc33d36c7
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
5 months agolttng: Add PWARN macro
Kienan Stewart [Fri, 17 May 2024 17:15:25 +0000 (13:15 -0400)] 
lttng: Add PWARN macro

Change-Id: Id6d15e094b8fe7ef779022e44d8414214af3444a
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
5 months agodocs: lttng-sessiond(8): bitness mix-up in env variable name
Jérémie Galarneau [Wed, 22 May 2024 20:15:39 +0000 (16:15 -0400)] 
docs: lttng-sessiond(8): bitness mix-up in env variable name

The section about the --consumerd64-path parameter erroneously refers to
the 32-bit consumer daemon binary environment
variable (LTTNG_CONSUMERD32_BIN).

It is modified to mention LTTNG_CONSUMERD64_BIN.

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

5 months ago.clang-tidy: remove modernize-concat-nested-namespaces
Jérémie Galarneau [Tue, 14 May 2024 13:58:55 +0000 (13:58 +0000)] 
.clang-tidy: remove modernize-concat-nested-namespaces

The concatenated namespace syntax (e.g. namespace my::ns::woah{}) was
introduced in C++17 while this project is built as C++11. Leaving this
rule it in the .clang-tidy only adds noise (and salt to the wound).

Change-Id: Ib9550aa602fbcccc7500b14603dfd698fd7b69de
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
5 months agotests: Correct use of taskset in snapshot tests
Kienan Stewart [Fri, 10 May 2024 18:41:11 +0000 (14:41 -0400)] 
tests: Correct use of taskset in snapshot tests

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

While investigating potential changes to how buffers are flushed
during snapshots, I discovered that the changes made the small
overwrite test flaky.

After investigation, the issue appears to be a problem with the test
that was exposed by the other changes.

Cause
=====

`taskset` handles its arguments more strictly than many other CLI
programs, and the argument order used is not valid. As the return
of the taskset invocation in the test isn't validated, there was
little indication of the issue save a warning that is easily lost
in the text.

The following example demonstrates the situation:

```
$ bash -x test.sh
+ taskset -p 910843
pid 910843's current affinity mask: f
+ echo 0
0
+ ./tests/utils/testapp/gen-ust-events/gen-ust-events
Process 910845 has 4 cpus in its affinity set
        CPU 0 is set? 1
        CPU 1 is set? 1
        CPU 2 is set? 1
        CPU 3 is set? 1
+ taskset -c 0 -p 910843
taskset: failed to execute -p: No such file or directory
+ echo 127
127
+ ./tests/utils/testapp/gen-ust-events/gen-ust-events
Process 910849 has 4 cpus in its affinity set
        CPU 0 is set? 1
        CPU 1 is set? 1
        CPU 2 is set? 1
        CPU 3 is set? 1
+ taskset -pc 0 910843
pid 910843's current affinity list: 0-3
pid 910843's new affinity list: 0
+ echo 0
0
+ ./tests/utils/testapp/gen-ust-events/gen-ust-events
Process 910853 has 1 cpus in its affinity set
        CPU 0 is set? 1
```

Solution
========

Correct the invocation of taskset and add a check on its return code
when used to set the cpu affinity of the current process.

Known drawbacks
===============

None.

Change-Id: Ia629121624532746431875b2031dd65df207666d
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
5 months agotests: Correct text when enabling a small overwrite kernel channel
Kienan Stewart [Fri, 10 May 2024 17:15:38 +0000 (13:15 -0400)] 
tests: Correct text when enabling a small overwrite kernel channel

Change-Id: I937aa6334c4f3ab0788ef1898ac9b323e82ea44c
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 months ago.gitignore: ignore local vscode workspace settings file
Jérémie Galarneau [Mon, 13 May 2024 18:41:41 +0000 (14:41 -0400)] 
.gitignore: ignore local vscode workspace settings file

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

6 months agoImport CStringView from the Babeltrace tree
Jérémie Galarneau [Tue, 23 Apr 2024 20:46:47 +0000 (16:46 -0400)] 
Import CStringView from the Babeltrace tree

The class is imported as of 1dcaf4b74 in the babeltrace tree. It is
renamed and reformated to fit the LTTng-tools conventions.

No changes in behaviour are intended.

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

6 months agoClean-up: ust-app: use const condition and event rule
Jonathan Rajotte [Wed, 24 Feb 2021 19:24:09 +0000 (14:24 -0500)] 
Clean-up: ust-app: use const condition and event rule

At this point only const object are required.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9c297fdd6e8026db4e0dc56bd9238fec05fbae3b

6 months agoClean-up: modernize pretty_xml.cpp
Jérémie Galarneau [Wed, 1 May 2024 15:21:04 +0000 (11:21 -0400)] 
Clean-up: modernize pretty_xml.cpp

Since pretty_xml was converted to C++, clang-tidy had a number of
grievances such as the use of NULL (instead of nullptr).

Since the file is very small, it is modernized to address those issues:
  - Wrap libxml2 resources in RAII wrappers
  - Use C++ IO APIs in lieu of fprintf

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

6 months ago.clang-format: remove duplicated BreakConstructorInitializers
Jérémie Galarneau [Wed, 24 Apr 2024 20:50:32 +0000 (16:50 -0400)] 
.clang-format: remove duplicated BreakConstructorInitializers

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

6 months agotests: remove uses of `xmlKeepBlanksDefault()`
Simon Marchi [Tue, 30 Apr 2024 19:19:32 +0000 (15:19 -0400)] 
tests: remove uses of `xmlKeepBlanksDefault()`

When compiling against libxml 2.12.6, I get:

      CXX      extract_xml-extract_xml.o
    /home/simark/src/lttng-tools/tests/utils/xml-utils/extract_xml.cpp: In function ‘int main(int, char**)’:
    /home/simark/src/lttng-tools/tests/utils/xml-utils/extract_xml.cpp:256:29: error: ‘int xmlKeepBlanksDefault(int)’ is deprecated [-Werror=deprecated-declarations]
      256 |         xmlKeepBlanksDefault(0);
          |         ~~~~~~~~~~~~~~~~~~~~^~~
    In file included from /home/simark/src/lttng-tools/tests/utils/xml-utils/extract_xml.cpp:29:
    /usr/include/libxml2/libxml/parser.h:957:17: note: declared here
      957 |                 xmlKeepBlanksDefault    (int val);
          |                 ^~~~~~~~~~~~~~~~~~~~

The documentation[1] suggests moving to "the modern options API with
XML_PARSE_NOBLANKS", do that.

Add a new `xml_parser_ctx_uptr` RAII object to manage the lifetime of
the `xmlParserCtxt` object.

[1] https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-parser.html#xmlKeepBlanksDefault

Change-Id: I7f9e70d2245fa333e01296aa2b4e3efa4e7fbefa
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 months agotests: convert pretty_xml.c to C++
Simon Marchi [Tue, 30 Apr 2024 19:22:15 +0000 (15:22 -0400)] 
tests: convert pretty_xml.c to C++

Change-Id: I40e0aa849193e789bf6634068a92b55484ab17af
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 months agofix: relayd: unaligned access in trace_chunk_registry_ht_key_hash
Michael Jeanson [Tue, 30 Apr 2024 19:17:52 +0000 (15:17 -0400)] 
fix: relayd: unaligned access in trace_chunk_registry_ht_key_hash

In 328c2fe7297c941aa9cbcfa4ce944fca1bd7300f, the type of 'lttng_uuid'
was changed from a C array of 16 'uint8_t' to a C++ std::array of the
same type and length.

In 'trace_chunk_registry_ht_key_hash()' we access these 16 bytes as 2
'uint64_t', to do so we used to cast the array to '(uint64_t *)' and
then access index 0 and 1.

When it was converted to C++, an error was introduced where instead we
reinterpret_cast to 'const uint64_t *' the index 0 and 1 of the array
which results in a 'uint64_t' pointer to the first and second bytes of
the array. These values overlap but since they are used as keys for an
hash table it still works. However, on platforms that don't allow
unaligned access, the second pointer being only offset by one byte
results in a 'Bus error'.

Reintroduce the old behavior by applying the index 0 and 1 to the
pointer resulting from the reinterpret_cast.

Change-Id: I2bc287edbe6864a2a870f9de1f3b4dd8f8a90ace
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 months agoClean-up: consumer.hpp: coding style indentation fix
Jérémie Galarneau [Tue, 30 Apr 2024 19:35:32 +0000 (15:35 -0400)] 
Clean-up: consumer.hpp: coding style indentation fix

Caught this that slipped by when running ./format-cpp

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

6 months agoFix: syscall event rule: emission sites not compared in is_equal
Jérémie Galarneau [Tue, 23 Apr 2024 19:13:16 +0000 (15:13 -0400)] 
Fix: syscall event rule: emission sites not compared in is_equal

The emission sites are not compared when comparing two kernel-syscall
event rules. This prevents users from subscribing to notifications of
triggers that use event-rule-matches syscall conditions which differ
only by their emission site (entry, exit, entry+exit).

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

6 months agovscode: Add configurations to run the executables under the debugger
Jérémie Galarneau [Wed, 24 Apr 2024 19:34:46 +0000 (15:34 -0400)] 
vscode: Add configurations to run the executables under the debugger

Add tasks.json and launch.json which allow VSCode users to build the
project and run the various binaries (lttng, lttng-relayd,
lttng-sessiond) under the integrated debugger.

For the moment, the configuration assumes the user wants to build
"in-tree" and has setup the tree to build the project (running
./bootstrap and ./configure).

The build job attempts to build a compile database if 'bear' is
available on the system.

To debug the LTTng client, make sure to edit the matching configuration
in .vscode/launch.json to provide your desired arguments (for the
moment, 'help' is passed by default).

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

6 months agoTests: Add test to check shared-memory FD leaks after relayd dies
Kienan Stewart [Wed, 20 Mar 2024 20:02:53 +0000 (16:02 -0400)] 
Tests: Add test to check shared-memory FD leaks after relayd dies

Refs: https://bugs.lttng.org/issues/1411

Change-Id: I9804011320c28a9867af1fdc6a8d82ad0671fe3d
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
6 months agoFix: consumerd: leak of tracing buffers on relayd connectivity issue
Jérémie Galarneau [Thu, 21 Mar 2024 20:55:08 +0000 (16:55 -0400)] 
Fix: consumerd: leak of tracing buffers on relayd connectivity issue

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

A leak of the tracing buffers can be noticed when the relay daemon is
terminated following the creation of a live session, but prior to the
initiation of any applications.

The issue can be reproduced with the following steps:
  # Create a live session
  $ lttng create --live

  # Kill the relay daemon before the allocation of the buffers
  $ killall lttng-relayd
  $ lttng enable-event --userspace --all
  $ lttng start

  # Launch an instrumented application
  $ ./my_app

  # Destroy all sessions
  $ lttng destroy --all

  # List the open file descriptors of the lttng-consumerd process
  # and notice how the tracing buffer are still visible.
  $ ls -lah /proc/$pid_of_lttng_consumerd/fd

[...]
lrwx------ 1 root root   64 Mar 19 19:50 987 -> '/dev/shm/shm-ust-consumer-358446 (deleted)'
lrwx------ 1 root root   64 Mar 19 19:50 988 -> '/dev/shm/shm-ust-consumer-358446 (deleted)'
lrwx------ 1 root root   64 Mar 19 19:50 989 -> '/dev/shm/shm-ust-consumer-358446 (deleted)'
[...]

Cause
=====

The consumer daemon allocates recording channels and their tracing
buffers in a two-step process.

First, the session daemon emits an `ASK_CHANNEL_CREATION` command, which
results in the allocation of the internal consumer channel structures
and of the actual tracing buffers. The channel's unique key is returned
to the session daemon.

After this command, the channel temporarily holds a list of streams
which are waiting to be sent to the session and relay daemons as a
result of the `GET_CHANNEL` command.

At this moment, the channel's reference count is one over the number of
streams as they all hold a back-reference to their parent channel and
there is a global reference held by the session daemon.

The session daemon uses the key it received to emit the `GET_CHANNEL`
command. When executing this command, the consumer daemon attempts to
send the streams to the relay daemon.

On failure to do so, the session daemon is informed of the connection
error. The consumer daemon then omits a step of the command: the streams
are never handed-off from the channel's internal list to the
consumption/monitoring thread. This hand-off is what is internally
referred-to as making the streams "globally visible".

The session daemon, upon receiving the failure error code of the
GET_CHANNEL command, tears down its internal ust_app channel structures.
As part of that process, it emits the `DESTROY_CHANNEL` command to
reclaim the channel on the consumer daemon's end. This command is
deferred to the channel poll thread as the `CHANNEL_DEL`
internal command.

As part of this internal command, the channel poll thread cleans the
channel's stream list to clean-up any streams that are not "globally
visible": all of them, in our case.

Then, the session daemon's global reference is released which should
normally result in the reclamation of the channel itself.

While reproducing the problem, we noted that channel wasn't reclaimed
and that its reference count matched the number of CPUs on the system at
the time the `CHANNEL_DEL` command completed.

This hinted at the streams holding a reference to the channel even after
the completion of the reclamation command.

Looking at clean_channel_stream_list(), which cleans up the channel's
temporary stream list, we note that the streams' monitor property is
overridden to `false` just before the call to consumer_stream_destroy().

This is strange and a comment (added as part of 212d67a2 in 2014) hints
at a locking problem that was being worked-around. In all likelihood,
this no longer applies as the locking strategies used have evolved quite
a bit since then.

Still, setting the monitor property to `false` is problematic as, in
that mode (say, channels that are used to record a snapshot), streams do
not hold a reference to their parent channel. This causes the clean-up
code to forego the clean-up of the channel, resulting in its leak.

Since the channel ultimately owns the 'stream_fds' which represent the
shared memory files, those files (and associated memory) are also leaked
(they are closed during the execution of lttng_ustconsumer_del_channel()).

Solution
========

We simply remove the stream monitor mode override to leave it in its
appropriate state. The clean-up then proceeds normally, ensuring the
tracing buffers are properly reclaimed.

Known drawbacks
===============

None.

Fixes #1411

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

7 months agodocs: Add supported versions and fix-backport policy
Kienan Stewart [Thu, 14 Mar 2024 15:33:47 +0000 (11:33 -0400)] 
docs: Add supported versions and fix-backport policy

Change-Id: Idb22c6487e2397b807c5d1b78acbc2adb03be363
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
7 months agodocs: Partially document the liblttng-ctl C API
Philippe Proulx [Tue, 15 Jun 2021 01:41:43 +0000 (21:41 -0400)] 
docs: Partially document the liblttng-ctl C API

This patch:

1. Performs the required changes to make the build system able to build
   an HTML API documentation using Doxygen.

   The way it's done is a replica of what does the Babeltrace 2 project,
   which you may be familiar with.

   `doc/api` is for all API documentation projects while
   `doc/api/liblttng-ctl` is the specific liblttng-ctl API documentation
   project.

   To build and view the HTML API documentation:

   a) Configure the project with the `--enable-api-doc` option.

   b) Build and install the project.

   c) Open
      `$prefix/share/doc/lttng-tools/api/liblttng-ctl/html/index.html`,
      where `$prefix` is the installation prefix (for example,
      `/usr/local`).

2. Fully documents some modules while not documenting others at all.

   Because some liblttng-ctl headers contain functions/types which
   conceptually belong to more than one module (unlike in Babeltrace 2),
   I decided to put all the Doxygen group (module) definitions and any
   "extra" module documentation in `dox/groups.dox`. The latter is a
   huge file of which most of the contents was copied from the
   LTTng-tools manual pages (and from the online LTTng Documentation)
   and adapted to the C API context.

   Images are direct copies from the LTTng Documentation.

   The complete module tree and its state, as of this patch, is as
   follows, where ✅ means it's fully documented and ❌ means it's not
   documented at all:

       ✅ Home page

       ✅ General API (error codes, session daemon connection,
          common definitions)

          Includes parts of `lttng.h`, `lttng-error.h`, and
          `constant.h`.

       ✅ Recording session API

          Includes parts of `lttng.h`, `channel.h`, `handle.h`,
          `domain.h`, and `session.h`.

          ✅ Recording session descriptor API

             Includes all `session-descriptor.h`.

          ✅ Recording session destruction handle API

             Includes all `destruction-handle.h`.

          ✅ Domain and channel API

             Includes parts of `channel.h`, `domain.h`, and `event.h`.

             ✅ Recording event rule API

                Includes parts of `event.h`.

          ❌ Process attribute inclusion set API

             Would include parts of `tracker.h`.

          ✅ Recording session clearing API

             Includes all `clear.h` and `clear-handle.h`.

          ❌ Recording session snapshot API

             Would include all `snapshot.h`.

          ❌ Recording session rotation API

             Would include all `rotation.h` and `location.h`.

          ❌ Recording session saving and loading API

             Would include all `save.h` and `load.h`.

       ✅ Instrumentation point listing API

          Includes parts of `event.h`.

       ❌ Trigger API

          Would include all `trigger/trigger.h`.

          ❌ Trigger condition API

             Would include all `condition/buffer-usage.h`,
             `condition/condition.h`, `condition/evaluation.h`,
             `condition/session-consumed-size.h`, and
             `condition/session-rotation.h`.

             ❌ "Event rule matches" trigger condition API

                Would include all `condition/event-rule-matches.h`.

                ❌ Event rule API

                   Would include all headers in `event-rule` as well
                   as all `kernel-probe.h` and `userspace-probe.h`.

                   ❌ Log level rule API

                      Would include all `log-level-rule.h`.

                ❌ Event expression API

                   Would include all `event-expr.h`.

                ❌ Event field value API

                   Would include all `event-field-value.h`.

          ❌ Trigger action API

             Would include all `action/action.h`,
             `action/firing-policy.h`, `action/list.h`, `action/path.h`,
             `action/rate-policy.h`, `action/rotate-session.h`,
             `action/snapshot-session.h`, `action/start-session.h`, and
             `action/stop-session.h`.

             ❌ Notify trigger action API

                Would include all `action/notify.h`,
                `notification/channel.h`, and
                `notification/notification.h`, as well as parts of
                `endpoint.h`.

       ❌ Error query API

          Would include all `error-query.h`.

   I'm voluntarily not documenting the health API (`health.h`), as I
   believe it's not super important for most users. We could document it
   on demand.

   In `groups.dox`, the groups of the undocumented modules are already
   defined, so that the complete tree above is visible in the rendered
   "API reference" section. The undocumented modules simply show the
   text "To be done". Because there are references to undocumented
   modules in `groups.dox` and in the documented headers, this means
   that the links at least resolve.

   Note that there are non-comment changes in `include/lttng`: I needed
   to name some anonymous, nested types so that I could reference their
   members, as you can only link to the member of a named type with
   Doxygen. For example, the type of the `u` union member of
   `struct lttng_event_context` is now `union lttng_event_context_u`;
   then you can reference its `probe` member as such:
   `lttng_event_context::lttng_event_context_u::probe` (_not_
   `lttng_event_context::u::probe`).

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I2783419159f4892a992fe5bc760b6e2cd6d13a60

7 months agoFix: rotation-destroy-flush: fix session daemon abort if no kernel module present
Xiangyu Chen [Mon, 25 Mar 2024 10:20:14 +0000 (18:20 +0800)] 
Fix: rotation-destroy-flush: fix session daemon abort if no kernel module present

Testing rotation-destroy-flush when no lttng kernel modules present, it
would be failed with error message:

  Error: Unable to load required module lttng-ring-buffer-client-discard
  not ok 1 - Start session daemon
  Failed test 'Start session daemon'
  not ok 2 - Create session rotation_destroy_flush in -o /tmp/tmp.test_rot ...
  ...

This because test script that sets the LTTNG_ABORT_ON_ERROR environment
variable. It's this environment variable that causes the sessiond to
handle the kernel module loading failure as an abort rather than a
warning.

Using "check_skip_kernel_test" to detect whether the kernel module fails
to load is expected or not. If the failure is expected, the script won't
set that environment variable any more.

Fixes: 3a174400
("tests:add check_skip_kernel_test to check root user and lttng kernel modules")

Change-Id: I371e9ba717613e2940186f710cf3cccd35baed6c
Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
7 months agoFix: consumerd: wrong timer mentioned in error logging
Jérémie Galarneau [Wed, 15 Mar 2023 20:42:27 +0000 (16:42 -0400)] 
Fix: consumerd: wrong timer mentioned in error logging

As its name indicates, consumer_timer_monitor_stop() stops the _monitor_
timer; not the live timer. This is most likely a copy-paste error.

The error logging is fixed to mention the appropriate timer.

Change-Id: I418580d8928752a0702d522e3ca74fe54cbe6f8f
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
7 months agoFix: consumerd: type confusion in lttng_consumer_send_error
Jérémie Galarneau [Tue, 7 Mar 2023 19:38:32 +0000 (14:38 -0500)] 
Fix: consumerd: type confusion in lttng_consumer_send_error

lttng_consumer_send_error sends an lttcomm_return_code to the session
daemon. However, the size of lttcomm_sessiond_command was used.

This was probably missed since the function accepts an integer instead
of a proper enum type.

The size accepted by the function is changed to use lttcomm_return_code
and the size of a fixed-size type is used to send the error code to the
session daemon.

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

7 months agoscope-exit: Clarify scope_exit noexcept requirement
Jérémie Galarneau [Tue, 26 Mar 2024 20:33:35 +0000 (16:33 -0400)] 
scope-exit: Clarify scope_exit noexcept requirement

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

8 months agoClean-up: lttng: utils: missing special member functions
Jérémie Galarneau [Fri, 22 Mar 2024 19:35:29 +0000 (15:35 -0400)] 
Clean-up: lttng: utils: missing special member functions

clang-tidy warns:
  warning: class 'session_list' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]

This warning related to the "Rule of Five":

If a class requires a custom destructor, copy constructor, or copy
assignment operator due to manual resource management, it likely needs
to explicitly define all five (including move constructor and move
assignment operator) to correctly manage the resources across all types
of object copying and moving scenarios. This rule helps prevent resource
leaks, double frees, and other common issues related to resource
management.

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

8 months agoClean-up: clang-tidy: missing headers prevent analysis
Jérémie Galarneau [Fri, 22 Mar 2024 19:33:57 +0000 (15:33 -0400)] 
Clean-up: clang-tidy: missing headers prevent analysis

clang-tidy complains that some headers omit the inclusion of their
dependencies, which prevents the analysis from completing.

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

8 months agoCodingStyle.md: remove redundant virtual specifier
Jérémie Galarneau [Fri, 22 Mar 2024 19:27:08 +0000 (15:27 -0400)] 
CodingStyle.md: remove redundant virtual specifier

The coding style mentions that override and virtual should not be used
together (as virtual would be redundant). However, one of the examples
commits that cardinal sin.

Mea culpa.

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

8 months agoClean-up: tests: utils: const_cast
Jérémie Galarneau [Fri, 22 Mar 2024 18:32:56 +0000 (14:32 -0400)] 
Clean-up: tests: utils: const_cast

clang-tidy warns that const_cast should not be used because of the
cppcoreguidelines-pro-type-const-cast rule.

In this case, const_cast is not strictly necessary and can be replaced
by a static_cast to silence the warning.

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

8 months agoClean-up: apply suggested clang-tidy fixes
Jérémie Galarneau [Fri, 22 Mar 2024 16:24:57 +0000 (12:24 -0400)] 
Clean-up: apply suggested clang-tidy fixes

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

8 months agoClean-up: waiter: coding style changes
Jérémie Galarneau [Fri, 22 Mar 2024 15:58:51 +0000 (11:58 -0400)] 
Clean-up: waiter: coding style changes

Mark non-throwing functions as noexcept when possible and mark waker as
final since we don't expect user to derive from it.

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

8 months agoFix: waiter: use std::reference_wrapper instead of a raw reference
Jérémie Galarneau [Fri, 22 Mar 2024 15:46:36 +0000 (11:46 -0400)] 
Fix: waiter: use std::reference_wrapper instead of a raw reference

clang-tidy warns that
  /home/jgalar/EfficiOS/src/lttng-tools/src/common/waiter.hpp:32:9: warning: use '= default' to define a trivial copy-assignment operator [modernize-use-equals-default]

The warning itself is bogus since it is not possible to use the default
copy-assignment operator in this case. Indeed, _state being a reference,
it cannot be rebound after its initialization. The compiler will refuse
to generate a default implementation as it would be ill-formed.

However, this does highlight a problem in the implementation, namely
that the explicit assignment operator does attempt to re-assign _state.

Switching the _state reference to use std::reference_wrapper does make
it legal to re-bind the reference to a different object.

Having the ability to assign a waker is conceptually a bit strange (vs
obtaining another instance from the waiter) and should probably be
disabled, but it is required for the moment as one the of use in a huge
C structure that gets copied using by assignment.

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

8 months agoFix: lttng-destroy: string formating error when default session is unset
Jérémie Galarneau [Thu, 21 Mar 2024 19:31:25 +0000 (15:31 -0400)] 
Fix: lttng-destroy: string formating error when default session is unset

Using `lttng destroy` when no default session is set in .lttngrc results
in the following print-out:

  Error: Can't find valid lttng config /root/lttng-build/home/.lttngrc
  Did you create a session? (lttng create <my_session>)
  Error: Failed to format string: string pointer is null

This is because the client attempts to format the following message:
  ERR_FMT("Session `{}` not found", spec.value);

When no default session could be found in .lttngrc, spec.value is left
at nullptr and it is assumed that the listing succeeded.

A new CLI-specific exception, no_default_session_error, is added to the
project and thrown when the session listing fails. This allows the
calling code to mark the listing as having failed.

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

8 months agoRevert "Fix: lttng-destroy: string formating error when default session is unset"
Jérémie Galarneau [Fri, 22 Mar 2024 14:51:20 +0000 (10:51 -0400)] 
Revert "Fix: lttng-destroy: string formating error when default session is unset"

This reverts commit e8c353ad12851366573d9bfe45d333a9e9ff742d as it is
missing files (wrong revision of the change picked-up from gerrit).

Change-Id: Ia426a8004d9827ee72ed39db52d7894f5d4eb34f
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoFix: lttng-destroy: string formating error when default session is unset
Jérémie Galarneau [Thu, 21 Mar 2024 19:31:25 +0000 (15:31 -0400)] 
Fix: lttng-destroy: string formating error when default session is unset

Using `lttng destroy` when no default session is set in .lttngrc results
in the following print-out:

  Error: Can't find valid lttng config /root/lttng-build/home/.lttngrc
  Did you create a session? (lttng create <my_session>)
  Error: Failed to format string: string pointer is null

This is because the client attempts to format the following message:
  ERR_FMT("Session `{}` not found", spec.value);

When no default session could be found in .lttngrc, spec.value is left
at nullptr and it is assumed that the listing succeeded.

A new CLI-specific exception, no_default_session_error, is added to the
project and thrown when the session listing fails. This allows the
calling code to mark the listing as having failed.

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

8 months ago.clang-tidy: add anonymous namespace-related checks
Simon Marchi [Fri, 15 Mar 2024 15:50:11 +0000 (11:50 -0400)] 
.clang-tidy: add anonymous namespace-related checks

Change-Id: I96fc6cf56d5c2d47addbc944ceae3992162a0958
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoFix: .clang-tidy: replace . with ,
Simon Marchi [Fri, 15 Mar 2024 15:49:19 +0000 (11:49 -0400)] 
Fix: .clang-tidy: replace . with ,

Change-Id: Ia10b3aea5cecaf277febd5c22b2b05916fbb51e9
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoFix: erroneous specifier used with ERR_FMT macro
Jérémie Galarneau [Fri, 25 Aug 2023 23:10:21 +0000 (19:10 -0400)] 
Fix: erroneous specifier used with ERR_FMT macro

ERR_FMT uses fmtlib's specifiers rather than the POSIX ones.

Change-Id: Iadd914fe2f5556b4d2463a9a6861bcb4d40d9643
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoFix: lttng-destroy: string formating error when default session is unset
Jérémie Galarneau [Thu, 21 Mar 2024 19:31:25 +0000 (15:31 -0400)] 
Fix: lttng-destroy: string formating error when default session is unset

Using `lttng destroy` when no default session is set in .lttngrc results
in the following print-out:

  Error: Can't find valid lttng config /root/lttng-build/home/.lttngrc
  Did you create a session? (lttng create <my_session>)
  Error: Failed to format string: string pointer is null

This is because the client attempts to format the following message:
  ERR_FMT("Session `{}` not found", spec.value);

When no default session could be found in .lttngrc, spec.value is left
at nullptr and it is assumed that the listing succeeded.

A new CLI-specific exception, no_default_session_error, is added to the
project and thrown when the session listing fails. This allows the
calling code to mark the listing as having failed.

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

8 months agoFix: baddr-statedump: use $(LIBTOOL) --mode=execute
orbea [Fri, 8 Mar 2024 16:17:37 +0000 (08:17 -0800)] 
Fix: baddr-statedump: use $(LIBTOOL) --mode=execute

GNU libtool inconsistently places the compiled executable in the source
directory or in the .libs directory where a libtool wrapper script is
placed in the source directory.

While slibtool will always place the compiled executable in the .libs
directory and a wrapper script in the source directory.

This will result with a build error when using slibtool since objcopy
needs the executable and not the shell wrapper script, but this can be
solved for both implementations by using $(LIBTOOL) --mode=execute on all
commands that operate on the libtool compiled executables.

Gentoo issue: https://bugs.gentoo.org/858095

The GNU libtool --mode=excute is documented upstream.

https://www.gnu.org/software/libtool/manual/html_node/Execute-mode.html
https://www.gnu.org/software/libtool/manual/html_node/Debugging-executables.html

And the GNU libtool behavior of when to create a wrapper script is
documented in the 'Linking Executables' section.

  "Notice that the executable, hell, was actually created in the .libs
  subdirectory. Then, a wrapper script (or, on certain platforms, a
  wrapper executable see Wrapper executables) was created in the current
  directory.

  Since libtool created a wrapper script, you should use libtool to
  install it and debug it too. However, since the program does not depend
  on any uninstalled libtool library, it is probably usable even without
  the wrapper script."

https://www.gnu.org/software/libtool/manual/html_node/Linking-executables.html

And the inconsistency between GNU libtool and slibtool is documented at
the Gentoo wiki.

  "One difference between GNU libtool and slibtool is that the former will
  conditionally place the compiled executable or a shell wrapper script in
  the build directory, depending on whether or not the executable depends
  on a build-local libtool library (e.g. libfoo.la). Where slibtool will
  always place a compatible wrapper script in the build directory where
  GNU libtool would have conditionally placed the executable. When the
  wrapper script is created both GNU libtool and slibtool will place the
  executable in the .libs directory within the build directory.
  Consequently build systems, ebuilds, and other users should take care to
  avoid scenarios like installing the wrapper script to the system instead
  of the executable. In these cases ideally the executable would be
  installed by the same libtool implementation that compiled it."

https: //wiki.gentoo.org/wiki/Slibtool#Installing_or_using_binaries_created_by_libtool_manually
Signed-off-by: orbea <orbea@riseup.net>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I03102ed78af835daa9b9a5836c2979a5f5d4bd8c

8 months agoClean-up: relayd: live: run clang-format against live.cpp
Jérémie Galarneau [Mon, 18 Mar 2024 18:39:46 +0000 (14:39 -0400)] 
Clean-up: relayd: live: run clang-format against live.cpp

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

8 months agoextras/zsh-completion/_lttng: add missing "requires" verb
Philippe Proulx [Thu, 14 Mar 2024 18:41:02 +0000 (14:41 -0400)] 
extras/zsh-completion/_lttng: add missing "requires" verb

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I9221a31e2a265e93b17242b152a00e8e1851cab7
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoClean-up: sessiond: use empty() instead of comparing size to 0
Jérémie Galarneau [Tue, 12 Mar 2024 19:57:31 +0000 (15:57 -0400)] 
Clean-up: sessiond: use empty() instead of comparing size to 0

Harmonize the project's coding style a little by favoring the use of the
'empty()' methood of containers rather than comparing their size to 0.

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

8 months agoBuild fix: missing operator- for iterator on g++7
Jérémie Galarneau [Tue, 12 Mar 2024 19:48:09 +0000 (15:48 -0400)] 
Build fix: missing operator- for iterator on g++7

The project fails to build on 'g++ (SUSE Linux) 7.5.0' since its STL
implementation assumes that operator- is available for random access
iterators.

The build fails with the following error:

  event_name.cpp:82:71:   required from here
  /usr/include/c++/7/bits/stl_iterator_base_funcs.h:104:21: error: no match for ‘operator-’ (operand types are ‘lttng::utils::random_access_container_wrapper<const bt_value*, const char*, event_name_set_operations>::_iterator<const lttng::utils::random_access_container_wrapper<const bt_value*, const char*, event_name_set_operations>, const char* const>’ and ‘lttng::utils::random_access_container_wrapper<const bt_value*, const char*, event_name_set_operations>::_iterator<const lttng::utils::random_access_container_wrapper<const bt_value*, const char*, event_name_set_operations>, const char* const>’)

A trivial implementation of that operator is provided and allows the
build to succeed.

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

8 months agoFix: relayd: live client not notified of inactive streams
Jérémie Galarneau [Fri, 15 Dec 2023 16:44:34 +0000 (11:44 -0500)] 
Fix: relayd: live client not notified of inactive streams

Observed issue
--------------

Some LTTng-tools live tests failures appear to show babeltrace2
hanging (failing to print expected events). The problem is intermittent,
but Kienan was able to develop a test case that's reproducible for him.

The test case performs the following steps:
  - Start a ust application and leave it running
  - Configure and then start an lttng live session
  - Connect a live viewer (babeltrace)
  - Run a second ust application
  - Wait for the expected number of events
    - In the failing case, no events are seen by babeltrace

Using per-uid buffers, the test typically completes normally. With
per-pid buffers the test fails, hanging indefinitely if waiting for the
specified number of events. While "hanging", babeltrace2 is polling the
relayd.

This affects for babeltrace2 stable-2.0 and master while using
lttng-tools master.

For more information, see the description of bug #1406[1]

Cause
-----

When consuming a live trace captured in per-PID mode, Babeltrace
periodically requests the index of the next packet it should consume.

As part of the reply, it gets a 'flags' field which is used to announce
that new streams, or new metadata, are available to the viewer.
Unfortunately, these 'flags' are only set when the relay daemon has new
tracing data to deliver. It is not set when the relay daemon indicates
that the stream is inactive (see LTTNG_VIEWER_INDEX_INACTIVE).

In the average case where an application is spawned while others are
actively emiting events, a request for new data will result in a reply
that returns an index entry (code LTTNG_VIEWER_INDEX_OK) for an
available packet accompanied by the LTTNG_VIEWER_FLAG_NEW_STREAM flag.

This flag indicates to the viewer that it should request new
streams (using the LTTNG_VIEWER_GET_NEW_STREAMS live protocol command)
before consuming the new data.

In the cases where we observe a hang, an application is running but not
emiting new events. As such, the relay daemon periodically emits "live
beacons" to indicate that the session's streams are inactive up to a
given time 'T'.

Since the existing application remains inactive and the viewer is never
notified that new streams are available, the viewer effectively remains
"stuck" and never notices the new application being traced.

The LTTNG_VIEWER_FLAG_NEW_METADATA communicates a similar semantic with
regards to the metadata. However, ignoring it for inactive streams isn't
as deleterious: the same information is made available to the viewer the
next time it will successfully request a new index to the relay daemon.

This would only become a problem if the tracers start to express
non-layout data (like supplemental environment information, but I don't
see a real use-case) as part of the metadata stream that should be made
available downstream even during periods of inactivity.

Note that the same problem most likely affects the per-UID buffer
allocation mode when multiple users are being traced.

Solution
--------

On the producer end, LTTNG_VIEWER_FLAG_NEW_STREAM is set even when
returning an inactivity index.

Note that to preserve compatibility with older live consumers that don't
expect this flag in non-OK response, the LTTNG_VIEWER_FLAG_NEW_STREAM
notification is repeated until the next LTTNG_VIEWER_GET_NEW_STREAMS
command that returns LTTNG_VIEWER_INDEX_OK.

The 'new_streams' state is no longer cleared from relay sessions during
the processing of the LTTNG_VIEWER_GET_NEXT_INDEX commands. Instead, it
is cleared when the viewer requests new streams.

On Babeltrace's end, the handler of the LTTNG_VIEWER_GET_NEXT_INDEX
command (lttng_live_get_next_index) is modified to expect
LTTNG_VIEWER_FLAG_NEW_STREAM in the cases where the command returns:
  - LTTNG_VIEWER_INDEX_OK (as done previously),
  - LTTNG_VIEWER_INDEX_HUP (new),
  - LTTNG_VIEWER_INDEX_INACTIVE (new).

Drawbacks
---------

This is arguably a protocol change as none of the producers ever set the
NEW_METADATA/NEW_STREAM flags when indicating an inactive stream.

References
----------

[1] https://bugs.lttng.org/issues/1406

Fixes #1406

Change-Id: I84f53f089597ac7b22ce8bd0962d4b28112b7ab6
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoClean-up: tests: bt2 plug-ins: modernize the plug-ins
Jérémie Galarneau [Fri, 8 Mar 2024 21:18:29 +0000 (16:18 -0500)] 
Clean-up: tests: bt2 plug-ins: modernize the plug-ins

By virtue of their use of the C Babeltrace 2 APIs, the test plug-ins
perform a fair amount of manual resource management.

To make it possible to adopt a more modern C++ style in those plug-ins,
a number of helpers are introduced.

Introduce reference wrappers for the Babeltrace 2 interface:
  - value_ref: wraps a bt_value reference using std::unique_ptr
  - message_const_ref: wraps a constant message reference using a
    unique_ptr
  - message_iterator_ref: wraps a message iterator reference using a
    unique_ptr
  - event_class_const_ref: wraps a constant event class reference using
    a unique_ptr

A specialized random_access_container_wrapper is specialized to wrap
bt_value arrays of strings.

In doing so, it is possible to eliminate the use of gotos and manual
reference management on error paths. Some struct/classes are renamed to
eliminate ambiguities that arose over the refactoring.

The changes allow some simplifications of the code flow in places which
are applied directly.

Change-Id: I25c148d7970cb89add55a86f2c162973d3d27e4a
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoClean-up: typo in make_unique_wrapper comment
Jérémie Galarneau [Tue, 12 Mar 2024 01:53:15 +0000 (21:53 -0400)] 
Clean-up: typo in make_unique_wrapper comment

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

8 months agoMove the lttng::free util under the lttng::memory namespace
Jérémie Galarneau [Fri, 8 Mar 2024 21:17:46 +0000 (16:17 -0500)] 
Move the lttng::free util under the lttng::memory namespace

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

8 months agoformat: use unique_ptr to wrap unmangled string
Jérémie Galarneau [Fri, 8 Mar 2024 17:06:30 +0000 (12:06 -0500)] 
format: use unique_ptr to wrap unmangled string

Change-Id: I8459507a55caf2c77a21fcc3442bcde069b2601b
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: Replace babelstats.pl with bt2 plugins
Kienan Stewart [Thu, 28 Sep 2023 20:54:42 +0000 (16:54 -0400)] 
tests: Replace babelstats.pl with bt2 plugins

Observed Issue
==============

`tests/regression/tools/filtering/test_valid_filters` is a long running
test, especially when running as root and exercising the tests across
the kernel domain.

I observed that a sizable amount of time was being spent in the analysis
of the results using `babelstats.pl`.

Solution
========

Instead of using a script to parse the pretty output of babeltrace2, I
decided to write two C++ plugins to replicate the behaviour of the
`babelstats.pl` script.

I measured the time using `sudo -E time ./path/to/test`

| Test                 | Time with `babelstats.pl` | Time with bt2 plugins |
| test_tracefile_count | 13.04s                    | 11.73s                |
| test_exclusion       | 22.75s                    | 22.07s                |
| test_valid_filter    | 301.04s                   | 144.41s               |

The switch to using babeltrace2 plugins reduces the runtime of the
`test_valid_filter` test (when running with kernel tests) by half. The
runtime changes to the other tests that were modified are not
significant.

Known drawbacks
===============

The field_stats plugin behaviour differs from `babelstats.pl` with
regards to enumeration fields ("container" in `babelstats.pl`). However,
no tests depend on that behaviour to pass.

The field_stats sink plugin doesn't perform a lot of run-time
error-checking of functions it invokes, and doesn't fully clean up all
the references it allocates though the babeltrace2 API. As the intended
usage is for short lived invocations with relatively small traces, the
principal drawback of this approach is that errors in the plugin may be
harder to debug.

Building tests of lttng-tools will now depend on having the babeltrace2
development headers and libraries available.

Change-Id: Ie8ebdd255b6901a7d0d7c4cd584a02096cccd4fb
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: Run relayd-grouping tests by grouping type
Kienan Stewart [Tue, 26 Sep 2023 13:39:41 +0000 (09:39 -0400)] 
tests: Run relayd-grouping tests by grouping type

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

The `relayd-grouping/test_ust` test takes ~2 minutes to run. A
significant amount of that time is statring and stopping the relay and
sesion daemons.

Solution
========

Each test function is run with a different grouping setup for the
relayd. Rather than iterating over each test and then grouping
variations, the iteration can be changed to organize the tests run by
grouping setup. This allows us to start th relay and session daemons
once per grouping setup, rather than twice for each test function.

Further more, each test function is run twice: once with auto-generated
session names, once with user-defined session names. This behaviour can
be cut out to reduce the runtime of the test further.

On my development machine, the test went from running in 113s to 18s.

Known drawbacks
===============

This no longer exercises the automatic session naming. I don't think
that the automatic session naming paths are pertinent with regards to
the grouping settings; however it appears it can impact output
directories (eg. in
`test_ust_uid_streaming_snapshot_add_output_custom_name`).

Change-Id: I89d8cb224e594dd68b7e8f3367d1907ecfa2bf13
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: Split test_ust_constructor into several tests
Kienan Stewart [Thu, 7 Mar 2024 20:20:17 +0000 (15:20 -0500)] 
tests: Split test_ust_constructor into several tests

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

TAP parsers fail when parsing a single executable that contains
several plans. Eg.,

```
ok 44 - Found no unexpected events
PASS: ust/ust-constructor/test_ust_constructor.py 44 - Found no unexpected events

1..44
ERROR: ust/ust-constructor/test_ust_constructor.py - multiple test plans
ok 1 - Create a session
ERROR: ust/ust-constructor/test_ust_constructor.py 1 - Create a session # UNPLANNED
```

and

```
14:03:23 org.tap4j.parser.ParserException: Error parsing TAP Stream: Duplicated TAP Plan found.
14:03:23  at org.tap4j.parser.Tap13Parser.parseTapStream(Tap13Parser.java:257)
14:03:23  at org.tap4j.parser.Tap13Parser.parseFile(Tap13Parser.java:231)
14:03:23  at org.tap4j.plugin.TapParser.parse(TapParser.java:172)
14:03:23  at org.tap4j.plugin.TapPublisher.loadResults(TapPublisher.java:475)
14:03:23  at org.tap4j.plugin.TapPublisher.performImpl(TapPublisher.java:352)
14:03:23  at org.tap4j.plugin.TapPublisher.perform(TapPublisher.java:312)
14:03:23  at jenkins.tasks.SimpleBuildStep.perform(SimpleBuildStep.java:123)
14:03:23  at hudson.tasks.BuildStepCompatibilityLayer.perform(BuildStepCompatibilityLayer.java:80)
14:03:23  at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20)
14:03:23  at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:818)
14:03:23  at hudson.model.AbstractBuild$AbstractBuildExecution.performAllBuildSteps(AbstractBuild.java:767)
14:03:23  at hudson.model.Build$BuildExecution.post2(Build.java:179)
14:03:23  at hudson.model.AbstractBuild$AbstractBuildExecution.post(AbstractBuild.java:711)
14:03:23  at hudson.model.Run.execute(Run.java:1918)
14:03:23  at hudson.matrix.MatrixRun.run(MatrixRun.java:153)
14:03:23  at hudson.model.ResourceController.execute(ResourceController.java:101)
14:03:23  at hudson.model.Executor.run(Executor.java:442)
14:03:23 Caused by: org.tap4j.parser.ParserException: Duplicated TAP Plan found.
14:03:23  at org.tap4j.parser.Tap13Parser.parseLine(Tap13Parser.java:354)
14:03:23  at org.tap4j.parser.Tap13Parser.parseTapStream(Tap13Parser.java:252)
14:03:23  ... 16 more
```

Cause
=====

09a872ef0b4e1432329aa42fecc61f50e9baa367 introduced multiple plans in
to test_ust_constructor

Solution
========

Split the script into several smaller test scripts sharing a common
import for data and the bulk of execution.

Known drawbacks
===============

None.

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

8 months agotests: Add diagnostic info for kernel bug, warning, and oops
Kienan Stewart [Tue, 23 Jan 2024 15:08:34 +0000 (10:08 -0500)] 
tests: Add diagnostic info for kernel bug, warning, and oops

When test_select_poll_epoll fails with an error due to hitting one a new
WARNING, OOPS, or BUG statements in dmesg, the user must go and read the
the logs themselves to try and find the matching statements.

Providing the previous and new messages in diagnostic output will allow
a person reading the test results to more quickly ascertain if the
messages are pertinent to lttng-modules or not. That being said, there
is no guarantee that there are not other WARNINGs, OOPs, or BUGs in
dmesg between before and after that are pertinent.

Change-Id: Ida026dfe852cafdcc55979089c92995949e2ef0d
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoClean-up: run clang-format 14 on the tree
Jérémie Galarneau [Thu, 7 Mar 2024 19:01:10 +0000 (14:01 -0500)] 
Clean-up: run clang-format 14 on the tree

Miscellaneous code style changes to correct little violations that
slipped through the cracks.

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

8 months agotests: Add C versions of gen-ust-events-constructor
Kienan Stewart [Fri, 9 Feb 2024 15:23:39 +0000 (10:23 -0500)] 
tests: Add C versions of gen-ust-events-constructor

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

The constructor tests exercise only the case where C++ applications
are built.

Solution
========

Adding C test applications allows us the reuse the existing test
infrastructure to cover these cases.

Known drawbacks
===============

None.

Change-Id: Ib178dfd33cce0f1d0aa125aaee078c2dcb84ecb9
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: test_ust_constructor: Split test_ust_constructor binary
Kienan Stewart [Thu, 8 Feb 2024 19:29:49 +0000 (14:29 -0500)] 
tests: test_ust_constructor: Split test_ust_constructor binary

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

The single test executable gen-ust-events-constructor covers a lot of
different cases in a single executable. This decreases the legibility of
the test results and debuggability of the test application as many
different pieces are in play.

Solution
========

The test functionality covered by the executable is split into two main
parts: one using a dynamically loaded shared object, and the second
using a statically linked archive.

Known drawbacks
===============

Rather than creating a second test script, the same script is re-used to
run multiple TapGenerator sequentially. This could hamper future efforts
to parallelize python-based tests.

Change-Id: I86d247780ce5412570eada6ebadb83a01547f2b0
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: Ensure `_process` is set in _TraceTestApplications
Kienan Stewart [Fri, 9 Feb 2024 14:16:26 +0000 (09:16 -0500)] 
tests: Ensure `_process` is set in _TraceTestApplications

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

An exception is thrown when deleting a _TraceTestApplication object that
has thrown an exception during it's `__init__` method. Eg.

```
Exception ignored in: <function _TraceTestApplication.__del__ at 0x7fcbc9a21620>
Traceback (most recent call last):
  File "/home/kstewart/src/efficios/lttng/master/src/lttng-tools/tests/utils/lttngtest/environment.py", line 348, in __del__
    self._process.kill()
    ^^^^^^^^^^^^^
AttributeError: '_TraceTestApplication' object has no attribute '_process'
```

Similarly, this can happen to _WaitTraceTestApplication objects.

Cause
=====

The object's `_process` attribute is set during `__init__`; however,
if an exception is thrown during `subprocess.Popen` a value is never
assigned to the attribute.

Solution
========

A default value for the `_process` attribute is set and checked as
part of the condition when executing the `__del__` method.

Known drawbacks
===============

None.

Change-Id: I2220ae764be49fafb3b977a5e723931421485d63
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: Correct tap_generator skip() when count is greater than 1
Kienan Stewart [Fri, 9 Feb 2024 14:08:07 +0000 (09:08 -0500)] 
tests: Correct tap_generator skip() when count is greater than 1

Issue observed
==============

Output when skipping multiple was incorrectly printing the test case number,
eg.

```
ok 3 - Start session `session_ldr8cxix`
41
ok 4 # Skip: Test application 'gen-ust-events-constructor/gen-ust-events-constructor-so' not found
ok 6 # Skip: Test application 'gen-ust-events-constructor/gen-ust-events-constructor-so' not found
ok 8 # Skip: Test application
'gen-ust-events-constructor/gen-ust-events-constructor-so' not found
```

Cause
=====

The `test_number` was adding the current index to the already modified
`self._last_test_case_id`.

Solution
========

Use `self._last_test_case_id` with no changes.

Known drawbacks
===============

None.

Change-Id: I8ff16b83619cf6e6db2636eeccd58725cc03d0f8
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: test_ust_constructor: Use a C-compiled shared object
Kienan Stewart [Thu, 8 Feb 2024 14:02:48 +0000 (09:02 -0500)] 
tests: test_ust_constructor: Use a C-compiled shared object

Similar to the previous change, this change splits the c-style
constructors for the shared object into a separate object which can be
compiled with gcc instead of g++.

This makes it possible to test the constructors are traced even if
LTTng-UST uses the LTTNG_UST_ALLOCATE_COMPOUND_LITERAL_ON_HEAP build
configuration.

Change-Id: Icd96cb30cedc1615951a6fec3c72731776f95d81
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: test_ust_constructor: Use a C-compiled static archive
Kienan Stewart [Thu, 8 Feb 2024 13:46:46 +0000 (08:46 -0500)] 
tests: test_ust_constructor: Use a C-compiled static archive

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

The test output describes the tracepoint as
`constructor_c_provider_static_archive`, which can be a bit misleading.

The tracepoints are indeed emitted inside C-style constructors. However,
as the tracepoints are being compiled inside a C++ translation unit,
they were never traceable when using a heap allocated implementation. If
the static archive is compiled as C and linked against the C++
application, the tracepoints are expected to always be visible.

Solution
========

In splitting the c-style constructors for the static archive into a
separate object the compilation can be made to use gcc instead of g++.

Drawback
========

This change doesn't keep a C-style constructor inside a C++ application
and asserts that it is indeed not traced when compiled using a heap
allocated implementation.

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

8 months agotests: Handle test failures for ust-constructors with heap allocation
Kienan Stewart [Wed, 7 Feb 2024 20:49:26 +0000 (15:49 -0500)] 
tests: Handle test failures for ust-constructors with heap allocation

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

A number of tests from `ust/ust-constructor/test_ust_constructor.py`
fail when compiled with gcc-4.8 (observed on SLES12SP5). Eg.

```
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 8 - Found
expected event name="tp_a:constructor_c_provider_static_archive"
msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 10 - Found expected event name="tp_a:constructor_cplusplus_provider_static_archive" msg="global - static archive define and provider"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 11 - Found expected event name="tp:constructor_c_across_units_before_define" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 12 - Found expected event name="tp:constructor_cplusplus" msg="global - across units before define"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 13 - Found expected event name="tp:constructor_c_same_unit_before_define" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 14 - Found expected event name="tp:constructor_c_same_unit_after_define" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 15 - Found expected event name="tp:constructor_cplusplus" msg="global - same unit before define"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 16 - Found expected event name="tp:constructor_cplusplus" msg="global - same unit after define"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 17 - Found expected event name="tp:constructor_c_across_units_after_define" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 18 - Found expected event name="tp:constructor_cplusplus" msg="global - across units after define"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 19 - Found expected event name="tp:constructor_c_same_unit_before_provider" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 20 - Found expected event name="tp:constructor_c_same_unit_after_provider" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 21 - Found
expected event name="tp:constructor_cplusplus" msg="global - same unit
before provider"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 34 - Found expected event name="tp:destructor_cplusplus" msg="global - same unit before provider"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 35 - Found expected event name="tp:destructor_cplusplus" msg="global - across units after define"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 36 - Found expected event name="tp:destructor_cplusplus" msg="global - same unit after define"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 37 - Found expected event name="tp:destructor_cplusplus" msg="global - same unit before define"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 38 - Found expected event name="tp:destructor_cplusplus" msg="global - across units before define"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 39 - Found
expected event
name="tp_a:destructor_cplusplus_provider_static_archive" msg="global -
static archive define and provider"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 41 - Found expected event name="tp:destructor_c_across_units_after_provider" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 42 - Found expected event name="tp:destructor_c_same_unit_after_provider" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 43 - Found expected event name="tp:destructor_c_same_unit_before_provider" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 44 - Found expected event name="tp:destructor_c_across_units_after_define" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 45 - Found expected event name="tp:destructor_c_same_unit_after_define" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 46 - Found expected event name="tp:destructor_c_same_unit_before_define" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 47 - Found expected event name="tp:destructor_c_across_units_before_define" msg="None"
12:22:17 FAIL: ust/ust-constructor/test_ust_constructor.py 48 - Found expected event name="tp_a:destructor_c_provider_static_archive" msg="None"
```

Cause
=====

As gcc-4.8 and earlier don't support C99 compound literals, the
lttngust `ust-compiler.h` falls back to using heap allocated
compound literals[1][2].

The probe registration in these cases is done via a C++ object[3].

As C-style constructors are executed before the C++ runtime is
processed, the probe is not yet registered[4].

In a case where g++ <= 4.8 is being used or
`-DLTTNG_UST_ALLOCATE_COMPOUND_LITERAL_ON_HEAP` is defined, the
following tracepoints will not be recorded:

* C-style constructors and destructors in statically linked archives
* C-style constructors and destructors in the application itself
* Some C++ constructors and destructors invoked during the
initialization of the static global variables
  * Note: this depends on the initialization order both between translation
  units, which is not specified, and the initialization order (usually
  lexicographical) within a given translation unit.

This is a known limitation; however, the test does not support
verifying that it's being run in a such a situation.

Solution
========

A small program has been added which returns a different status code
depending on whether `LTTNG_UST_ALLOCATE_COMPOUND_LITERAL_ON_HEAP` is
defined or not.

The test script uses this application to signal that certain events
may fail (in that they may be present, or they may be absent).

Drawbacks
=========

None.

References
==========

[1]: https://github.com/lttng/lttng-ust/commit/e1904921db97b70d94e69f0ab3264c6f7fe62f32
[2]: https://github.com/lttng/lttng-ust/commit/7edfc1722684982b9df894c054d69808dc588a6a
[3]: https://github.com/lttng/lttng-ust/commit/05bfa3dc3a6e6b2ece3686a5f384b6645c2a5010
[4]: https://github.com/lttng/lttng-ust/blob/3287f48be61ef3491aff0a80b7185ac57b3d8a5d/include/lttng/ust-compiler.h#L110

Change-Id: I49159df4f85126c641aaf5fb0a8b5b22fd91bf12
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: add check_skip_kernel_test to check root user and lttng kernel modules
Xiangyu Chen [Mon, 12 Feb 2024 14:23:54 +0000 (09:23 -0500)] 
tests: add check_skip_kernel_test to check root user and lttng kernel modules

The current tests will run both userspace and kernel testing. Some of
use cases only use lttng for one kind of tracing on an embedded
device (e.g. userspace), so in this scenario, the kernel modules might
not install to target rootfs, the test cases would be fail and exit.

Add LTTNG_TOOLS_DISABLE_KERNEL_TESTS to skip the lttng kernel features
test, this flag can be set via "make":

  make check LTTNG_TOOLS_DISABLE_KERNEL_TESTS=1

When this flag was set, all kernel related testcases would be marked as
SKIP in result.

Since the the LTTNG_TOOLS_DISABLE_KERNEL_TESTS was checked in function
check_skip_kernel_test, lots of testcases also need to check root
permission, so merging the root permission checking into
check_skip_kernel_test.

Change-Id: I49a1f642a9869c21a69e0186c296fd917bd7b525
Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoFix: force _lttng python binding to be linked with g++
Kienan Stewart [Fri, 1 Mar 2024 18:09:51 +0000 (13:09 -0500)] 
Fix: force _lttng python binding to be linked with g++

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

On Enterprise Linux 7 CI nodes, several tests using the python binding
were failing with errors such as the following:

```
ERROR: ust/exit-fast/test_exit-fast
===================================

Warning: Failed to produce a random seed using getrandom(), falling back to pseudo-random device seed generation which will block until its pool is initialized: getrandom() is not supported by this platform [getrandom_nonblock() random.cpp:90]
Traceback (most recent call last):
File "/home/jenkins/workspace/lttng-tools_master_elbuild/babeltrace_version/stable-2.0/build/std/conf/std/liburcu_version/master/platform/el7-amd64/src/lttng-tools/extras/bindings/swig/python/lttng.py", line 24, in swig_import_helper
fp, pathname, description = imp.find_module('_lttng', [dirname(__file__)])
File "/usr/lib64/python3.6/imp.py", line 297, in find_module
raise ImportError(_ERR_MSG.format(name), name=name)
ImportError: No module named '_lttng'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "./ust/exit-fast/test_exit-fast.py", line 19, in <module>
from test_utils import *
File "/home/jenkins/workspace/lttng-tools_master_elbuild/babeltrace_version/stable-2.0/build/std/conf/std/liburcu_version/master/platform/el7-amd64/src/lttng-tools/tests/utils/test_utils.py", line 24, in <module>
from lttng import *
File "/home/jenkins/workspace/lttng-tools_master_elbuild/babeltrace_version/stable-2.0/build/std/conf/std/liburcu_version/master/platform/el7-amd64/src/lttng-tools/extras/bindings/swig/python/lttng.py", line 34, in <module>
_lttng = swig_import_helper()
File "/home/jenkins/workspace/lttng-tools_master_elbuild/babeltrace_version/stable-2.0/build/std/conf/std/liburcu_version/master/platform/el7-amd64/src/lttng-tools/extras/bindings/swig/python/lttng.py", line 26, in swig_import_helper
import _lttng
ImportError: /home/jenkins/workspace/lttng-tools_master_elbuild/babeltrace_version/stable-2.0/build/std/conf/std/liburcu_version/master/platform/el7-amd64/src/lttng-tools/extras/bindings/swig/python/.libs/_lttng.so: undefined symbol: _ZNSt13runtime_errorC2EPKc
ERROR: ust/exit-fast/test_exit-fast - missing test plan
```

The link mode can be seen the commands use to do the linking in the CI
node build logs. For example,

```
libtool: link: gcc -shared  -fPIC -DPIC  .libs/lttng_wrap.o  -Wl,--whole-archive ../../../../src/common/.libs/libsessiond-comm.a ../../../../src/common/.libs/libcommon-gpl.a -Wl,--no-whole-archive  -Wl,-rpath -Wl,/home/jenkins/workspace/lttng-tools_master_elbuild/babeltrace_version/stable-2.0/build/std/conf/std/liburcu_version/master/platform/el8-amd64/src/lttng-tools/src/lib/lttng-ctl/.libs -Wl,-rpath -Wl,/build/lib64 -L/home/jenkins/workspace/lttng-tools_master_elbuild/babeltrace_version/stable-2.0/build/std/conf/std/liburcu_version/master/platform/el8-amd64/deps/build/lib64 ../../../../src/lib/lttng-ctl/.libs/liblttng-ctl.so -lxml2 -L/build/lib64 -lurcu -lurcu-common -lurcu-cds -lrt  -pthread -g -O2   -pthread -Wl,-soname -Wl,_lttng.so.0 -o .libs/_lttng.so.0.0.0
```

Cause
=====

Automake chooses the link mode based on the types of files in the
library or executable. Given that the generated bindings are only C
code, automake uses the gcc link mode.

Solution
========

By adding a dummy (non existant) C++ source file to the library,
automake can be 'forced' to the switch the link mode to `g++`.

Example link command in `g++` mode:

```
libtool: link: g++-10 -std=gnu++11  -fPIC -DPIC -shared -nostdlib
/usr/lib/gcc/x86_64-linux-gnu/10/../../../x86_64-linux-gnu/crti.o
/usr/lib/gcc/x86_64-linux-gnu/10/crtbeginS.o  .libs/lttng_wrap.o
-Wl,--whole-archive ../../../../src/common/.libs/libsessiond-comm.a
../../../../src/common/.libs/libcommon-gpl.a -Wl,--no-whole-archive
-Wl,-rpath
-Wl,/home/kstewart/src/efficios/lttng/master/src/lttng-tools/src/lib/lttng-ctl/.libs
-Wl,-rpath -Wl,/home/kstewart/src/efficios/lttng/master/usr/lib
-Wl,-rpath -Wl,/home/kstewart/src/efficios/lttng/master/usr/lib
../../../../src/lib/lttng-ctl/.libs/liblttng-ctl.so -lxml2
-L/home/kstewart/src/efficios/lttng/master/usr/lib
/home/kstewart/src/efficios/lttng/master/usr/lib/liburcu.so
/home/kstewart/src/efficios/lttng/master/usr/lib/liburcu-cds.so
/home/kstewart/src/efficios/lttng/master/usr/lib/liburcu-common.so
-lrt -L/usr/lib/gcc/x86_64-linux-gnu/10
-L/usr/lib/gcc/x86_64-linux-gnu/10/../../../x86_64-linux-gnu
-L/usr/lib/gcc/x86_64-linux-gnu/10/../../../../lib
-L/lib/x86_64-linux-gnu -L/lib/../lib -L/usr/lib/x86_64-linux-gnu
-L/usr/lib/../lib -L/usr/lib/gcc/x86_64-linux-gnu/10/../../.. -lstdc++
-lm -lc -lgcc_s /usr/lib/gcc/x86_64-linux-gnu/10/crtendS.o
/usr/lib/gcc/x86_64-linux-gnu/10/../../../x86_64-linux-gnu/crtn.o  -g
-O2 -fuse-ld=lld   -pthread -Wl,-soname -Wl,_lttng.so.0 -o
.libs/_lttng.so.0.0.0
```

Known drawbacks
===============

None.

Change-Id: I5f1dedec435089518e36cc12cd09c2bb151adb67
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: Add test for live viewer hanging when connecting after a clear
Kienan Stewart [Wed, 21 Feb 2024 13:57:43 +0000 (08:57 -0500)] 
tests: Add test for live viewer hanging when connecting after a clear

References: https://review.lttng.org/c/lttng-tools/+/11819

Change-Id: Ic40f3ee674657a802d4081e008cdb67247cd70ff
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: Add mechanism to start relayd in python testing environment
Kienan Stewart [Wed, 21 Feb 2024 13:57:19 +0000 (08:57 -0500)] 
tests: Add mechanism to start relayd in python testing environment

Change-Id: I787528c4d281d1047d1ab119bde86c95decb9cca
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agotests: Add clear command to python tests
Kienan Stewart [Wed, 21 Feb 2024 13:56:31 +0000 (08:56 -0500)] 
tests: Add clear command to python tests

Change-Id: I9eba90a4ebfe2b983a2fac1344d6a472d8b1c849
Signed-off-by: Kienan Stewart <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
8 months agoFix: relayd: live: dispose of zombie viewer metadata stream
Jérémie Galarneau [Sat, 17 Feb 2024 13:57:47 +0000 (08:57 -0500)] 
Fix: relayd: live: dispose of zombie viewer metadata stream

Issue observed
==============

In the CI, builds on SLES15SP5 frequently experience timeouts. From
prior inspections, there are hangs during
tests/regression/tools/clear/test_ust while waiting for babeltrace to
exit.

It is possible to reproduce the problem fairly easily:

  $ lttng create --live
  $ lttng enable-event --userspace --all
  $ lttng start

  # Launch an application that emits a couple of events
  $ ./my_app
  $ lttng stop

  # Clear the data, this eventually results in the deletion of all
  # trace files on the relay daemon's end.
  $ lttng clear

  # Attach to the live session from another terminal
  $ babeltrace -i lttng-live net://...

  # The 'destroy' command completes, but the viewer never exits.
  $ lttng destroy

Cause
=====

After the clear command completes, the relay daemon no longer has any
data to serve. We notice that the live client loops endlessly repeatably
sending GET_METADATA requests. In response, the relay daemon replies
with the NO_NEW_METADATA status.

In concrete terms, the viewer_get_metadata() function short-circuits to
send that reply when it sees that the metadata stream has no active
trace chunk (i.e., there are no backing files from which to read the
data at the moment).

This situation is not abnormal in itself: it is legitimate for a client
to wait for the metadata to become available again. For example, in the
reproducer above, it would be possible for the user to restart the
tracing (lttng start), which would create a new trace chunk and make the
metadata stream available. New events could also be emitted following
this restart.

However, when a session's connection is closed, there is no hope that
the metadata stream will ever transition back to an active trace chunk.

Solution
========

When the metadata stream has no active chunk and the corresponding
consumerd-side connection has been closed, there is no way the relay
daemon will be able to serve the metadata contents to the client.

As such, the viewer stream can be disposed-of since it will no longer be
of any use to the client. Since some client implementations expect at
least one GET_METADATA command to result in NO_NEW_METADATA, that status
code is initially returned.

Later, when the client emits a follow-up GET_METADATA request for that
same stream, it will receive an "error" status indicating that the
stream no longer exists. This situation is not treated as an error by
the clients. For instance, babeltrace2 will simply close the
corresponding trace and indicate it ended.

The 'no_new_metadata_notified' flag doesn't appear to be necessary to
implement the behaviour expected by the clients (seeing at least one
NO_NEW_METADATA status reply for every metadata stream). The
viewer_get_metadata() function is refactored a bit to drop the global
reference to the viewer metadata stream as it exits, while still
returning the NO_NEW_METADATA status code.

Known drawbacks
===============

None.

Note
====

The commit message of e8b269fa provides more details behind the
intention of the 'no_new_metadata_notified' flag.

Change-Id: Ib1b80148d7f214f7aed221d3559e479b69aedd82
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
This page took 0.061057 seconds and 4 git commands to generate.