hmon: add logic monitor by arkjedrz · Pull Request #95 · eclipse-score/lifecycle

arkjedrz · 2026-02-27T11:50:54Z

Add logic monitor to HMON.

Resolved #15

github-actions · 2026-02-27T11:51:38Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.4.2) and connecting to it...
INFO: Invocation ID: 770b8c1f-8326-4cac-ada9-4a7b2d761675
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'score_rust_policies', the root module requires module version score_rust_policies@0.0.3, but got score_rust_policies@0.0.5 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

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

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

Analyzing: target //:license-check (145 packages loaded, 2642 targets configured)

Analyzing: target //:license-check (153 packages loaded, 6066 targets configured)

Analyzing: target //:license-check (159 packages loaded, 7867 targets configured)

Analyzing: target //:license-check (165 packages loaded, 7901 targets configured)

Analyzing: target //:license-check (165 packages loaded, 7901 targets configured)

Analyzing: target //:license-check (165 packages loaded, 7901 targets configured)

Analyzing: target //:license-check (165 packages loaded, 7901 targets configured)

Analyzing: target //:license-check (169 packages loaded, 9913 targets configured)

INFO: Analyzed target //:license-check (170 packages loaded, 10039 targets configured).
[12 / 16] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox ... (2 actions, 1 running)
[15 / 16] [Prepa] Building license.check.license_check.jar ()
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
INFO: Elapsed time: 25.897s, Critical Path: 2.67s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 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>]

Copilot

Pull request overview

This PR extends the health monitoring library with new monitor types (heartbeat + logic) and refactors the worker/supervisor client plumbing so the worker can evaluate multiple monitor kinds and report typed evaluation errors across Rust + C++/FFI.

Changes:

Add new HeartbeatMonitor (state + evaluator + FFI + C++ wrapper) and LogicMonitor (state-transition validator).
Refactor monitor evaluation to pass an HMON starting Instant and to report typed MonitorEvaluationError variants (deadline/heartbeat/logic).
Move supervisor API client implementations into a feature-gated supervisor_api_client/ module and adjust build config defaults/features.

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/health_monitoring_lib/rust/worker.rs	Updates monitoring loop to pass a shared starting `Instant` and log typed monitor errors; removes in-file supervisor client impls.
src/health_monitoring_lib/rust/tag.rs	Adds `StateTag` newtype for logic monitor states; updates tag tests.
src/health_monitoring_lib/rust/supervisor_api_client/stub_supervisor_api_client.rs	Adds stub supervisor client implementation behind feature gate.
src/health_monitoring_lib/rust/supervisor_api_client/score_supervisor_api_client.rs	Adds SCORE supervisor client implementation (monitor_rs-backed).
src/health_monitoring_lib/rust/supervisor_api_client/mod.rs	Introduces `SupervisorAPIClient` trait and feature-gated implementations.
src/health_monitoring_lib/rust/logic/mod.rs	Adds logic module exports.
src/health_monitoring_lib/rust/logic/logic_monitor.rs	Implements logic monitor state machine + evaluator + tests.
src/health_monitoring_lib/rust/lib.rs	Integrates deadline/heartbeat/logic monitors into `HealthMonitorBuilder` + `HealthMonitor`; changes build/start to return `Result`.
src/health_monitoring_lib/rust/heartbeat/mod.rs	Adds heartbeat module wiring and FFI submodule.
src/health_monitoring_lib/rust/heartbeat/heartbeat_state.rs	Adds compact atomic heartbeat state representation (+ loom-aware atomics).
src/health_monitoring_lib/rust/heartbeat/heartbeat_monitor.rs	Adds heartbeat monitor implementation + builder validation + tests.
src/health_monitoring_lib/rust/heartbeat/ffi.rs	Adds FFI for heartbeat monitor builder/monitor and tests.
src/health_monitoring_lib/rust/ffi.rs	Extends core FFI to build/start using `Result`, adds heartbeat builder/get FFI, and maps `HealthMonitorError` to `FFICode`.
src/health_monitoring_lib/rust/deadline/mod.rs	Re-exports `DeadlineEvaluationError` for typed evaluation errors.
src/health_monitoring_lib/rust/deadline/deadline_monitor.rs	Refactors deadline monitor to use typed `DeadlineEvaluationError` and updated evaluator signature.
src/health_monitoring_lib/rust/common.rs	Introduces `HasEvalHandle`, typed `MonitorEvaluationError`, shared evaluation signature, and time conversion helpers.
src/health_monitoring_lib/cpp/tests/health_monitor_test.cpp	Extends C++ test to build/get/use heartbeat monitor.
src/health_monitoring_lib/cpp/include/score/hm/heartbeat/heartbeat_monitor.h	Adds C++ API surface for heartbeat monitor + builder.
src/health_monitoring_lib/cpp/include/score/hm/health_monitor.h	Adds heartbeat monitor APIs to C++ `HealthMonitor(Builder)`.
src/health_monitoring_lib/cpp/heartbeat_monitor.cpp	Implements C++ heartbeat monitor wrapper calling Rust FFI.
src/health_monitoring_lib/cpp/health_monitor.cpp	Wires heartbeat builder/get calls through Rust FFI.
src/health_monitoring_lib/Cargo.toml	Adds features (default stub; optional score uses monitor_rs) and loom dependency config.
src/health_monitoring_lib/BUILD	Adds heartbeat C++ sources/headers and adjusts crate feature flags for Bazel targets.
examples/rust_supervised_app/src/main.rs	Updates example to handle `build()`/`start()` returning `Result`.
Cargo.toml	Sets workspace `default-members` and adds `cfg(loom)` lint configuration.
Cargo.lock	Adds new dependency entries (loom + transitive deps).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/health_monitoring_lib/rust/heartbeat/heartbeat_monitor.rs

Copilot · 2026-02-27T11:56:02Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+        match value {
+            value if value == LogicEvaluationError::InvalidState as u8 => LogicEvaluationError::InvalidState,
+            value if value == LogicEvaluationError::InvalidTransition as u8 => LogicEvaluationError::InvalidTransition,
+            _ => panic!("Invalid underlying value of logic evaluation error."),


impl From<u8> for LogicEvaluationError panics on unknown values. Since this conversion is used when interpreting AtomicU8 state during evaluate, an unexpected value would crash the monitoring thread/process instead of reporting an error. Please make this conversion non-panicking (e.g., map unknown values to InvalidState or return Option/Result) and handle that path in evaluate.

Suggested change

_ => panic!("Invalid underlying value of logic evaluation error."),

_ => LogicEvaluationError::InvalidState,

src/health_monitoring_lib/rust/supervisor_api_client/score_supervisor_api_client.rs

src/health_monitoring_lib/rust/lib.rs

src/health_monitoring_lib/rust/heartbeat/heartbeat_state.rs

github-actions · 2026-02-27T11:58:49Z

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

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

src/health_monitoring_lib/rust/logic/logic_monitor.rs

src/health_monitoring_lib/rust/tag.rs

src/health_monitoring_lib/rust/logic/logic_monitor.rs

pawelrutkaq

Stopped as my comment will probably simplify more code

src/health_monitoring_lib/rust/logic/logic_monitor.rs

src/health_monitoring_lib/rust/tag.rs

src/health_monitoring_lib/rust/logic/logic_monitor.rs

paulquiring · 2026-03-06T09:31:09Z

src/health_monitoring_lib/cpp/include/score/hm/deadline/deadline_monitor.h


  protected:
-    std::optional<internal::FFIHandle> __drop_by_rust_impl()
+    std::optional<internal::FFIHandle> _drop_by_rust_impl()


_drop_by_rust_impl -> drop_by_rust_impl

Compilers like GCC, Clang, and MSVC often use leading underscores for internal macros or system-level functions. If a future update to your compiler introduces a macro or an internal header named _drop_by_rust_impl, your code will fail to compile—or worse, behave unpredictably—and it will be considered your fault, not the compiler’s, because you used a reserved naming pattern.

__ and _ is just considered not a good style. They are reserved.

That one's fine. It was previously double underscore, and that's actually an issue.
https://devblogs.microsoft.com/oldnewthing/20230109-00/?p=107685

Yes one underscore flowed by a lower letter is not and issue. One underscore followed by a capital letter is also reserved. From the link you shared:

It is best practice to use the namespace detail or internal to indicate to user to not use it directly.

paulquiring · 2026-03-06T09:59:40Z

The rust part looks like it supports monitoring from multiple threads. I could not find any hint how to set this up correctly form the cpp side.

Add logic monitor to HMON.

- Add const constructor to `StateTag`. - Atomic `LogicState` containing current state index and monitor status. - Updated logic monitor API. - Reworked internal of logic monitor.

pawelrutkaq · 2026-03-06T16:02:48Z

src/health_monitoring_lib/rust/logic/logic_state.rs

+#[cfg(not(loom))]
+use core::sync::atomic::{AtomicU64, Ordering};
+#[cfg(loom)]
+use loom::sync::atomic::{AtomicU64, Ordering};


This ifdef shall be hidden in some common place, maybe even baselibs

pawelrutkaq · 2026-03-06T16:04:34Z

src/health_monitoring_lib/rust/logic/logic_state.rs

+    /// Monitor status.
+    /// - zero if healthy.
+    /// - `LogicEvaluationError` if not.
+    pub fn monitor_status(&self) -> u8 {


Shouldn't that be bool returned?

pawelrutkaq · 2026-03-06T16:06:49Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+    InvalidState = OK_STATE + 1,
+    /// Transition is invalid.
+    InvalidTransition,
+}


if this is what you store as error in the shared state, we shall make this type as input into snapshot and not raw u8

pawelrutkaq · 2026-03-06T16:07:43Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+impl From<u8> for LogicEvaluationError {
+    fn from(value: u8) -> Self {
+        match value {
+            value if value == LogicEvaluationError::InvalidState as u8 => LogicEvaluationError::InvalidState,


why we cannot match for LogicEvaluationError::InvalidState, LogicEvaluationError::InvalidTransition and for _ without ifs ?

Btw We shall implement TryFrom if this can fail. as From is not failable

pawelrutkaq · 2026-03-06T16:16:51Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+                current_state_node.tag, target_state
+            );
+            let error = LogicEvaluationError::InvalidTransition;
+            let _ = self.logic_state.update(|mut current_state| {


since the handle is only reading the values, you can simply use store or swap, no need to use update

pawelrutkaq · 2026-03-06T16:17:30Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+        }
+
+        // Find index of target state, then change current state.
+        let target_state_index = self.find_index_by_tag(target_state)?;


Further optimization would be to keep with the target states their indexes so you don't need to do any lookups.

pawelrutkaq · 2026-03-06T16:17:54Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+
+        // Find index of target state, then change current state.
+        let target_state_index = self.find_index_by_tag(target_state)?;
+        let _ = self.logic_state.update(|mut current_state| {


no update needed as above

pawelrutkaq · 2026-03-06T16:18:42Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+
+        // Disallow operation in erroneous state.
+        if snapshot.monitor_status() != OK_STATE {
+            warn!("Current logic monitor state cannot be determined");


actually it can and its error. maybe log is not needed

pawelrutkaq · 2026-03-06T16:19:08Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+        // Disallow operation in erroneous state.
+        if snapshot.monitor_status() != OK_STATE {
+            warn!("Current logic monitor state cannot be determined");
+            return Err(LogicEvaluationError::InvalidState);


shdnt status be properly converted here instead hardocoded?

pawelrutkaq · 2026-03-06T16:24:24Z

src/health_monitoring_lib/rust/logic/logic_monitor.rs

+
+    /// Perform transition to a new state.
+    /// On success, current state is returned.
+    pub fn transition(&self, state: StateTag) -> Result<StateTag, LogicEvaluationError> {


LogicMonitor shall be restricted to a single thread only. Currently, I think it's both Send and Sync. So either we remove Sync or make the public api mutable just to remove cross-thread usage without proper locking. I think other Monitors have same issue.

arkjedrz requested review from Copilot and pawelrutkaq February 27, 2026 11:50

arkjedrz self-assigned this Feb 27, 2026

arkjedrz had a problem deploying to workflow-approval February 27, 2026 11:50 — with GitHub Actions Failure

arkjedrz had a problem deploying to workflow-approval February 27, 2026 11:50 — with GitHub Actions Error

Copilot started reviewing on behalf of arkjedrz February 27, 2026 11:51 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

arkjedrz force-pushed the arkjedrz_logic-monitor branch from 38a2bdf to 07352fd Compare March 2, 2026 07:34

arkjedrz temporarily deployed to workflow-approval March 2, 2026 07:34 — with GitHub Actions Inactive

arkjedrz force-pushed the arkjedrz_logic-monitor branch from 07352fd to f96e585 Compare March 3, 2026 13:07

arkjedrz temporarily deployed to workflow-approval March 3, 2026 13:07 — with GitHub Actions Inactive

arkjedrz force-pushed the arkjedrz_logic-monitor branch from f96e585 to 4ab21c9 Compare March 4, 2026 14:50

arkjedrz requested a deployment to workflow-approval March 4, 2026 14:50 — with GitHub Actions Waiting

arkjedrz requested a review from NicolasFussberger March 5, 2026 07:58

arkjedrz force-pushed the arkjedrz_logic-monitor branch from 4ab21c9 to 743411c Compare March 5, 2026 10:32

arkjedrz requested a deployment to workflow-approval March 5, 2026 10:32 — with GitHub Actions Waiting

arkjedrz marked this pull request as ready for review March 5, 2026 10:45

arkjedrz requested a review from Copilot March 5, 2026 10:46

Copilot started reviewing on behalf of arkjedrz March 5, 2026 10:46 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

pawelrutkaq requested changes Mar 5, 2026

View reviewed changes

src/health_monitoring_lib/rust/logic/logic_monitor.rs Show resolved Hide resolved

src/health_monitoring_lib/rust/tag.rs Show resolved Hide resolved

src/health_monitoring_lib/rust/logic/logic_monitor.rs Outdated Show resolved Hide resolved

arkjedrz force-pushed the arkjedrz_logic-monitor branch from 743411c to 4686fb0 Compare March 5, 2026 13:58

arkjedrz requested a deployment to workflow-approval March 5, 2026 13:58 — with GitHub Actions Waiting

paulquiring reviewed Mar 6, 2026

View reviewed changes

arkjedrz force-pushed the arkjedrz_logic-monitor branch from 4686fb0 to 6247d38 Compare March 6, 2026 12:58

arkjedrz temporarily deployed to workflow-approval March 6, 2026 12:58 — with GitHub Actions Inactive

arkjedrz added 2 commits March 6, 2026 14:25

hmon: add logic monitor

7c739ba

Add logic monitor to HMON.

hmon: reworked logic monitor

48bc6f2

- Add const constructor to `StateTag`. - Atomic `LogicState` containing current state index and monitor status. - Updated logic monitor API. - Reworked internal of logic monitor.

arkjedrz force-pushed the arkjedrz_logic-monitor branch from 6247d38 to 48bc6f2 Compare March 6, 2026 13:28

arkjedrz temporarily deployed to workflow-approval March 6, 2026 13:28 — with GitHub Actions Inactive

arkjedrz deployed to workflow-approval March 6, 2026 13:28 — with GitHub Actions Active

pawelrutkaq requested changes Mar 6, 2026

View reviewed changes

	_ => panic!("Invalid underlying value of logic evaluation error."),
	_ => LogicEvaluationError::InvalidState,

Conversation

arkjedrz commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pawelrutkaq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulquiring Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

paulquiring commented Mar 6, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arkjedrz commented Feb 27, 2026 •

edited

Loading

github-actions bot commented Feb 27, 2026 •

edited

Loading

paulquiring Mar 6, 2026 •

edited

Loading