Log backend for rust frontend #1

pawelrutkaq · 2025-09-18T08:46:20Z

This is logger impl that bridges rust log frontend with C++ mw_log implementation.
This requires https://github.com/eclipse-score/baselibs/pull/18/files to be fully funcional (currently this PR is used in bazel override)

This is transfered from: eclipse-score/inc_mw_log#6

github-actions · 2025-09-18T08:47:19Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
2025/10/08 08:26:18 Downloading https://releases.bazel.build/8.3.0/release/bazel-8.3.0-linux-x86_64...
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: 499de832-a15f-4df3-9750-36bb9a5d084c
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 3 packages loaded
Loading: 3 packages loaded
    currently loading: 
Loading: 3 packages loaded
    currently loading: 
Analyzing: target //:license-check (4 packages loaded, 0 targets configured)
Analyzing: target //:license-check (4 packages loaded, 0 targets configured)

Analyzing: target //:license-check (68 packages loaded, 9 targets configured)

Analyzing: target //:license-check (76 packages loaded, 9 targets configured)

Analyzing: target //:license-check (119 packages loaded, 604 targets configured)

Analyzing: target //:license-check (128 packages loaded, 2016 targets configured)

Analyzing: target //:license-check (135 packages loaded, 2390 targets configured)

Analyzing: target //:license-check (140 packages loaded, 2439 targets configured)

Analyzing: target //:license-check (140 packages loaded, 2439 targets configured)

Analyzing: target //:license-check (144 packages loaded, 4573 targets configured)

INFO: Analyzed target //:license-check (145 packages loaded, 4699 targets configured).
[9 / 13] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox ... (2 actions running)
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 2 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
[11 / 13] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache, processwrapper-sandbox
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
[13 / 13] no actions running
INFO: Elapsed time: 24.505s, Critical Path: 2.67s
INFO: 13 processes: 9 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 13 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

github-actions · 2025-09-18T08:49:27Z

The created documentation from the pull request is available at: docu-html

pawelrutkaq · 2025-09-18T08:51:10Z

@arsibo @hoppe-and-dreams @rmaddikery

This is logger impl that bridges rust log frontend with C++ mw_log implementation Signed-off-by: Pawel Rutka <pawel.rutka.ext@qorix.ai>

MathiasDanzeisen

Code looks good to me.
I was just wondering why we have the "mw" in namespace in Rust and C++.

MathiasDanzeisen · 2025-10-02T12:38:08Z

src/mw_log_subscriber/src/lib.rs

+        self,
+    ) -> MwLogger {
+        let context_cstr = self.context.unwrap_or(CString::new("DFLT").unwrap());
+        let c_logger_ptr = unsafe { mw_log_create_logger(context_cstr.as_ptr() as *const _) };


The type could be specified:

Suggested change

let c_logger_ptr = unsafe { mw_log_create_logger(context_cstr.as_ptr() as *const _) };

let c_logger_ptr = unsafe { mw_log_create_logger(context_cstr.as_ptr() as *const c_char) };

This is also recommended according to:
https://coding-guidelines.arewesafetycriticalyet.org/coding-guidelines/expressions.html#gui_HDnAZ7EZ4z6G

MathiasDanzeisen · 2025-10-02T15:45:08Z

src/mw_log_subscriber/src/lib.rs

+    }
+
+    /// Builds the MwLogger with the specified context and configuration and returns it.
+    pub fn build<const SHOW_MODULE: bool, const SHOW_FILE: bool, const SHOW_LINE: bool>(


From api user perspective I would prefer a builder with a more self-explanatory api signature:

pub fn build() pub fn enable_log_file(mut self) -> MwLoggerBuilder ..

but I see the issue with the const and the that generics only support scalar types.

This would be maybe doable via type state pattern, but for now, since this is just first version I would skip it.

hoppe-and-dreams · 2025-10-06T08:12:35Z

.bazelrc

+
+# magic logging configuration
+common --@score-baselibs//score/mw/log/detail/flags:KUse_Stub_Implementation_Only=False
+common --@score-baselibs//score/mw/log/flags:KRemote_Logging=False


Why would we want to set this flag "globally" for anyone? Especially as remote logging is the standard way of logging ...

As I understand, currently this is not supported. The code does not compile without it and I copied it from other repos.

ERROR: no such package '@@score-baselibs+//score/mw/log/detail/data_router': BUILD file not found in directory 'score/mw/log/detail/data_router' of external repository @@score-baselibs+. Add a BUILD file to a directory to mark it as a package. ERROR: /home/pawel/.cache/bazel/_bazel_pawel/d25adba3030c308deb575e8d3085ff8c/external/score-baselibs+/score/mw/log/detail/BUILD:45:11: no such package '@@score-baselibs+//score/mw/log/detail/data_router': BUILD file not found in directory 'score/mw/log/detail/data_router' of external repository @@score-baselibs+. Add a BUILD file to a directory to mark it as a package. and referenced by '@@score-baselibs+//score/mw/log/detail:recorder_config' ERROR: Analysis of target '//src/mw_log_subscriber:example' failed; build aborted: Analysis failed INFO: Elapsed time: 0.484s, Critical Path: 0.00s

Maybe I am missing some knowledge here ?

yeah, your right. I forgot about that. We just have to make sure, this get's changed once we provide the remote backend.

hoppe-and-dreams · 2025-10-06T08:15:58Z

src/mw_log_subscriber/src/rust_cpp_log_adapter.cpp

I was expecting a file name along the lines of rust_cpp_log_adapter.
I'm not happy with the current name as it repeats the module name "mw_log" needlessly and adds the "ffi" which does apply to all files in the module but make it seem this is not the case.

hoppe-and-dreams · 2025-10-06T08:16:52Z

src/mw_log_subscriber/src/rust_cpp_log_adapter.cpp

+    extern "C"
+    {
+
+        Logger *mw_log_create_logger(const char *context)


The rest of the mw/log C++ codebase follows CamelCase notation for function names.
Does S-CORE have an alternating style guide?
Applies to all functions in the file.

Maybe the problem is that there is no released one? oding_guidelines.html#code-style

clang format files I found being used in the project:
eclipse-score/baselibs · score/language/futurecpp/.clang-format
eclipse-score/inc_feo · .clang-format

Okay, this is not nice :(
We should at least (until overridden by overall project) decide on one for Logging.
Let's add it to the agenda of the CFT Meeting ...

since Rust is snake_case, the FFI is snake case. Otherwise, major usage of ffi will mix camelCase and snake_case and will cause Rust warning needed to be supressed. I would not chnage it since its common practice in Rust.

Its fine if this file (and this file only) has a different format for the reason you mentioned. I just wanted to make sure we align on the overall picture

hoppe-and-dreams · 2025-10-06T08:19:17Z

BUILD

The changes in this file seem unrelated to rust log backend. Should be moved to individual PR (or at least individual commit)

they are related, to use new code in other repo You need them. I would not split this into seprate PR because it's direct consequence of adding a code and making it usable to other repos.

hoppe-and-dreams · 2025-10-06T08:54:38Z

src/mw_log_subscriber/BUILD

+cc_library(
+    name = "libmw_log_ffi",
+    srcs = ["src/mw_log_ffi.cpp"],
+    visibility = ["//visibility:public"],


Why would the cpp mapper be public? I thought it needs only be seen by the rust backend? 🤔

right, done

hoppe-and-dreams · 2025-10-06T08:55:21Z

src/mw_log_subscriber/BUILD

+load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library")
+
+cc_library(
+    name = "libmw_log_ffi",


Same comment as on file. I'd advocate for a name along rust_cpp_log_mapper

hoppe-and-dreams · 2025-10-06T08:55:52Z

MODULE.bazel

The changes in this file seem unrelated to rust log backend. Should be moved to individual PR (or at least individual commit)

they are related, to use new code in other repo You need them. I would not split this into seprate PR because it's direct consequence of adding a code and making it usable to other repos.

hoppe-and-dreams · 2025-10-06T08:56:54Z

MODULE.bazel

-llvm.toolchain(
-    cxx_standard = {"": "c++17"},
-    llvm_version = "19.1.0",
+gcc = use_extension("@score_toolchains_gcc//extentions:gcc.bzl", "gcc", dev_dependency = True)


Isn't the toolchain fixed across S-CORE? I was expecting it fixed, as at some point the software has to be build using safety certified compiler.

Do you mean a fixed version? This declared L47 like described in toolchain repo There is a newer version 0.6 available, but I don't know the version deployment/usage schema in the project.

Yeah, I mean in the end if an some entity want to put logging into a Vehicle under ISO26262 certification it needs to be compliable/linkable by a certified compiler. Typically those are older versions. Eg. for QNX7 Safety core it was GCC9. Hence a lot of brand new compiler goodies were not allowed to be used.

Here it seems you pull a very recent version, and I'm afraid it will break users who need to compile with the qualified compiler. Do you rely on any such brand new feature?

For the above reason, I thought all S-CORE Repos would have a common GCC = Version X.Y set.

We dont rely on any, as You saw, usage is simple usage of baselib code. I think the compiler story, deps alignment and final checking is still not setup up finally in S-CORE. But I think the operational team is working on it. So for now I would skip this discussion here in this PR.

Yeah, I mean in the end if an some entity want to put logging into a Vehicle under ISO26262 certification it needs to be compliable/linkable by a certified compiler. Typically those are older versions. Eg. for QNX7 Safety core it was GCC9. Hence a lot of brand new compiler goodies were not allowed to be used.

Here it seems you pull a very recent version, and I'm afraid it will break users who need to compile with the qualified compiler. Do you rely on any such brand new feature?

For the above reason, I thought all S-CORE Repos would have a common GCC = Version X.Y set.

So, to my understanding the s-core toolchain repo offers "fixed toolchains" which are versioned. In this PR we make already use of it ( https://github.com/eclipse-score/logging/pull/1/files#diff-6136fc12446089c3db7360e923203dd114b6a1466252e71667c6791c20fe6bdcR47 ).
What we should figure out, is which s-core gcc version shall be used at the moment e.g. 0.4 or the latest 0.6? But to me this is not a blocker, instead can be done later.

We dont rely on any, as You saw, usage is simple usage of baselib code. I think the compiler story, deps alignment and final checking is still not setup up finally in S-CORE. But I think the operational team is working on it. So for now I would skip this discussion here in this PR.

Okay, then I might just not understand the change being introduced here. To me it looks as if we pull a very recent compiler version in that is out of line with what the rest of the S-core project is using.

If this is not the case and/or it is seen as tech debt with a clear roadmap of when and where to change to. Disregard my comment.

MathiasDanzeisen

Thanks for the work. The changes look good to me.

pawelrutkaq force-pushed the pawelrutkaq_introduce_log_backend_enabler branch from f5bf372 to c63f846 Compare September 18, 2025 08:49

pawelrutkaq changed the title ~~Log backend for rust forontend~~ Log backend for rust frontend Sep 18, 2025

pawelrutkaq force-pushed the pawelrutkaq_introduce_log_backend_enabler branch from c63f846 to 721f788 Compare September 18, 2025 08:53

Log backend for rust frontend

5105b3a

This is logger impl that bridges rust log frontend with C++ mw_log implementation Signed-off-by: Pawel Rutka <pawel.rutka.ext@qorix.ai>

pawelrutkaq force-pushed the pawelrutkaq_introduce_log_backend_enabler branch from 721f788 to 5105b3a Compare September 18, 2025 08:55

vinodreddy-g requested review from arsibo, hoppe-and-dreams and rmaddikery September 19, 2025 06:34

johannes-esr mentioned this pull request Sep 30, 2025

FEO: Integration of Logging Feature eclipse-score/score#1073

Open

MathiasDanzeisen previously approved these changes Oct 2, 2025

View reviewed changes

hoppe-and-dreams suggested changes Oct 6, 2025

View reviewed changes

Adress review findings

02f33db

pawelrutkaq dismissed MathiasDanzeisen’s stale review via 02f33db October 8, 2025 08:26

pawelrutkaq requested review from MathiasDanzeisen and hoppe-and-dreams October 8, 2025 08:26

MathiasDanzeisen approved these changes Oct 10, 2025

View reviewed changes

	let c_logger_ptr = unsafe { mw_log_create_logger(context_cstr.as_ptr() as *const _) };
	let c_logger_ptr = unsafe { mw_log_create_logger(context_cstr.as_ptr() as *const c_char) };

Log backend for rust frontend #1

Are you sure you want to change the base?

Log backend for rust frontend #1

Uh oh!

Conversation

pawelrutkaq commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions bot commented Sep 18, 2025

Uh oh!

pawelrutkaq commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MathiasDanzeisen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MathiasDanzeisen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

pawelrutkaq commented Sep 18, 2025 •

edited

Loading

github-actions bot commented Sep 18, 2025 •

edited

Loading

pawelrutkaq commented Sep 18, 2025 •

edited

Loading