babeltrace | sink.ctf.fs escaping of struct member names

babeltrace | sink.ctf.fs escaping of struct member names

[MOESSBAUER, Felix via lttng-dev]

Hi,

while working on the LTTng index addition, I noticed that the traces
with index are not picked up correctly in trace-compass. The reason for
that is the escaping of the package.context -> cpu_id field. When
creating the CTF metadata with sink.ctf.fs, all user-provided struct
member names are escaped with an underscore.

As of CTF 1.8.3, readers are only recommended to strip single
underscores from field names, but they don't have to. And unfortunately
trace-compass does not do this for the package context cpu_id field
(surprisingly only when having an index...). Just manually changing the
packet->context _cpu_id to cpu_id fixes the issue.

In my opinion, the escaping of the struct member names in the CTF
writer is not correct according to the CTF 1.8.3 spec: Section 4.2.1
states, that reserved keywords must not be used and the recommended
escaping is a underscore prefix. However, the spec does not state any
other names that need to be escaped and by that also not names starting
with an underscore itself.

IOW: The CTF writer currently unconditionally escapes all struct member
names, which I assume to be incorrect

I tried fixing this with the attached patch, however this breaks the
test-trace-copy.sh.
To me it at least partially looks like these tests just check against
the implementation, but not against the spec.
Anyways, I would like to clarify this topic upfront before proposing
any changes.

I also tried to find the reason via a git blame, however there were
that many refactorings that the original idea remains hidden.

Best regards,
Felix

diff --git a/src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.cpp
b/src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.cpp
index 12ab213d1..43bc8e098 100644
--- a/src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.cpp
+++ b/src/plugins/ctf/fs-sink/translate-trace-ir-to-ctf-ir.cpp
@@ -159,12 +159,6 @@ static inline bool must_protect_identifier(const
char *name)
         }
     }
 
     }
 
-    /* Protect an identifier which already starts with `_` */
-    if (name[0] == '_') {
-        must_protect = true;
-        goto end;
-    }
-
 end:
     return must_protect;
 }
@@ -182,7 +176,7 @@ static inline int
cur_path_stack_push(ctf::sink::TraceIrToCtfIrCtx *ctx, const c
 
     if (name) {
         if (ctx->ctf_version == 1) {
-            if (force_protect_name) {
+            if (force_protect_name || must_protect_identifier(name)) {
                 g_string_assign(field_path_elem->name, "_");
             }
 
@@ -641,7 +635,7 @@
translate_structure_field_class_members(ctf::sink::TraceIrToCtfIrCtx
*ctx,
         member =
bt_field_class_structure_borrow_member_by_index_const(ir_fc, i);
         name = bt_field_class_structure_member_get_name(member);
         memb_ir_fc =
bt_field_class_structure_member_borrow_field_class_const(member);
-        ret = cur_path_stack_push(ctx, name, true, memb_ir_fc,
&struct_fc->base);
+        ret = cur_path_stack_push(ctx, name, false, memb_ir_fc,
&struct_fc->base);
         if (ret) {
             BT_CPPLOGE_SPEC(ctx->logger,
                             "Cannot translate structure field class
member: "

-- 
Siemens AG
Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany

Re: babeltrace | sink.ctf.fs escaping of struct member names

[Philippe Proulx via lttng-dev]

On Thu, Dec 4, 2025 at 5:10 AM MOESSBAUER, Felix via lttng-dev
<lttng-dev@lists.lttng.org> wrote:
>
> In my opinion, the escaping of the struct member names in the CTF
> writer is not correct according to the CTF 1.8.3 spec: Section 4.2.1
> states, that reserved keywords must not be used and the recommended
> escaping is a underscore prefix. However, the spec does not state any
> other names that need to be escaped and by that also not names starting
> with an underscore itself.
>
> IOW: The CTF writer currently unconditionally escapes all struct member
> names, which I assume to be incorrect

It's not incorrect per se.

Underscore as a member name escaping mechanism has always been a
nightmare in CTF 1.8. For example, given:

    uint8 _len;
    string _strings[length here];

Should we use `_len` or `len`? And what about this:

    struct {
        uint8 meow;
        uint8 _meow;
    } _mix;

LTTng chooses to escape everything with `_`, except the packet header,
packet context, and event record header names it knows. We did exactly
the same in `sink.ctf.fs`.

CTF 1.8 doesn't specify `cpu_id` by the way; it's an LTTngism.

CTF 2 solves all of that!

Since CTF 1.8 is pretty much deprecated at this point, I want to invest
as little time as possible in anything related to this legacy format,
whatever the project. Just make it work considering the known use cases.

Therefore, for your specific problem, I suggest that you specifically
avoid the "protection" (escaping) for `cpu_id` in the packet context
field class _only for LTTng traces_. It should be good enough and make
Trace Compass work. Trace Compass could also unescape packet context
member names, but it will be faster to do this in Babeltrace 2 and it
will match the LTTng behaviour more closely.

>
> I tried fixing this with the attached patch,

If you need comments on a patch, even if it's an RFC, please prefer
Gerrit so that we can comment specific lines and follow the discussion
more easily.

BR,

Philippe

Re: babeltrace | sink.ctf.fs escaping of struct member names

[MOESSBAUER, Felix via lttng-dev]

On Thu, 2025-12-04 at 10:15 -0500, Philippe Proulx wrote:
> On Thu, Dec 4, 2025 at 5:10 AM MOESSBAUER, Felix via lttng-dev
> <lttng-dev@lists.lttng.org> wrote:
> > 
> > In my opinion, the escaping of the struct member names in the CTF
> > writer is not correct according to the CTF 1.8.3 spec: Section 4.2.1
> > states, that reserved keywords must not be used and the recommended
> > escaping is a underscore prefix. However, the spec does not state any
> > other names that need to be escaped and by that also not names starting
> > with an underscore itself.
> > 
> > IOW: The CTF writer currently unconditionally escapes all struct member
> > names, which I assume to be incorrect
> 
> It's not incorrect per se.
> 
> Underscore as a member name escaping mechanism has always been a
> nightmare in CTF 1.8. For example, given:
> 
>     uint8 _len;
>     string _strings[length here];
> 
> Should we use `_len` or `len`? And what about this:
> 
>     struct {
>         uint8 meow;
>         uint8 _meow;
>     } _mix;

Okay... that's the reason. Thanks for the example.

> 
> LTTng chooses to escape everything with `_`, except the packet header,
> packet context, and event record header names it knows. We did exactly
> the same in `sink.ctf.fs`.
> 
> CTF 1.8 doesn't specify `cpu_id` by the way; it's an LTTngism.

Well... This is a more generic question if it is really the job of the
serializer to fix possible name clashes in the output. Anyways,
unfortunately LTTng uses this field and does not allow escaping.

> 
> CTF 2 solves all of that!

I would love to switch to CTF2, however we need support for it in
trace-compass, which AFAIK is not even implemented yet.

> 
> Since CTF 1.8 is pretty much deprecated at this point, I want to invest
> as little time as possible in anything related to this legacy format,
> whatever the project. Just make it work considering the known use cases.
> 
> Therefore, for your specific problem, I suggest that you specifically
> avoid the "protection" (escaping) for `cpu_id` in the packet context
> field class _only for LTTng traces_. It should be good enough and make
> Trace Compass work. Trace Compass could also unescape packet context
> member names, but it will be faster to do this in Babeltrace 2 and it
> will match the LTTng behaviour more closely.

This is tricky, as we don't really know if the trace is an LTTng trace
or not. However, we can hide it behind the proposed create-lttng-index
flag, as it is anyways only needed in combination with an index (I
still don't know why, but ok...).

> 
> > 
> > I tried fixing this with the attached patch,
> 
> If you need comments on a patch, even if it's an RFC, please prefer
> Gerrit so that we can comment specific lines and follow the discussion
> more easily.

Ok, got it. I'll add the new implementation to the create-lttng-index
series (as it anyways depends on this).

Best regards,
Felix

> 
> BR,
> 
> Philippe

-- 
Siemens AG
Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany

Re: babeltrace | sink.ctf.fs escaping of struct member names

[Philippe Proulx via lttng-dev]

On Thu, Dec 4, 2025 at 10:47 AM MOESSBAUER, Felix
<felix.moessbauer@siemens.com> wrote:
>
> Well... This is a more generic question if it is really the job of the
> serializer to fix possible name clashes in the output. Anyways,
> unfortunately LTTng uses this field and does not allow escaping.

Although I understand the question, I will not take the time to
re-evaluate a design that involves CTF 1.8. I'm currently in a "quick
fixes only" mode for this format, with the sole aim of keeping existing
scenarios working.

> I would love to switch to CTF2, however we need support for it in
> trace-compass, which AFAIK is not even implemented yet.

We're currently investigating the status of CTF 2 support in Trace
Compass and what amount of work is remaining. We'll let you know.

> This is tricky, as we don't really know if the trace is an LTTng trace
> or not. However, we can hide it behind the proposed create-lttng-index
> flag, as it is anyways only needed in combination with an index (I
> still don't know why, but ok...).

Yes we know. Have a look at make_lttng_trace_path_rel().

`create-lttng-index` would be another hint, but the trace environment
has been good enough so far.

Phil

Re: babeltrace | sink.ctf.fs escaping of struct member names

[MOESSBAUER, Felix via lttng-dev]

On Thu, 2025-12-04 at 11:01 -0500, Philippe Proulx wrote:
> On Thu, Dec 4, 2025 at 10:47 AM MOESSBAUER, Felix
> <felix.moessbauer@siemens.com> wrote:
> > 
> > Well... This is a more generic question if it is really the job of the
> > serializer to fix possible name clashes in the output. Anyways,
> > unfortunately LTTng uses this field and does not allow escaping.
> 
> Although I understand the question, I will not take the time to
> re-evaluate a design that involves CTF 1.8. I'm currently in a "quick
> fixes only" mode for this format, with the sole aim of keeping existing
> scenarios working.
> 
> > I would love to switch to CTF2, however we need support for it in
> > trace-compass, which AFAIK is not even implemented yet.
> 
> We're currently investigating the status of CTF 2 support in Trace
> Compass and what amount of work is remaining. We'll let you know.
> 
> > This is tricky, as we don't really know if the trace is an LTTng trace
> > or not. However, we can hide it behind the proposed create-lttng-index
> > flag, as it is anyways only needed in combination with an index (I
> > still don't know why, but ok...).
> 
> Yes we know. Have a look at make_lttng_trace_path_rel().

I already checked this function, but this is a bit too-strict, as
really only adding the index breaks the import. I also want to be able
to add the index to non lttng traces (which for instance lack some env
metadata) as otherwise the ftrace-to-ctf converter needs to fake the
metadata.

> 
> `create-lttng-index` would be another hint, but the trace environment
> has been good enough so far.

This is easy to implement and works perfectly fine. By that, traces
without an lttng index have "_cpu_id" and can be imported in trace-
compass. Traces with an lttng index get "cpu_id" and also now import
correctly.

I hope this solution is acceptable.

Anyways, thanks for the quick support.

Felix

> 
> Phil

-- 
Siemens AG
Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany