From a1865e1b43a6793ae555c59e3aee69fb80f913d7 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 12:58:20 +0200 Subject: [PATCH 1/9] Service::GetSeverity(): simplify nested if, add braces No change in functionality, just making the code a bit nicer and more compact. --- lib/icinga/service.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/icinga/service.cpp b/lib/icinga/service.cpp index d831136bb..2993916d3 100644 --- a/lib/icinga/service.cpp +++ b/lib/icinga/service.cpp @@ -139,13 +139,12 @@ int Service::GetSeverity() const ObjectLock hlock (host); if (host->GetState() != HostUp) { severity += 1024; + } else if (IsAcknowledged()) { + severity += 512; + } else if (IsInDowntime()) { + severity += 256; } else { - if (IsAcknowledged()) - severity += 512; - else if (IsInDowntime()) - severity += 256; - else - severity += 2048; + severity += 2048; } hlock.Unlock(); } From 5ca6047b35f280a9d625fe15c65fdf657a0fdd7a Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 13:01:03 +0200 Subject: [PATCH 2/9] Service::GetSeverity(): replace switch with if No change in functionality, just making the code a bit more compact. --- lib/icinga/service.cpp | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/lib/icinga/service.cpp b/lib/icinga/service.cpp index 2993916d3..954cd3d5e 100644 --- a/lib/icinga/service.cpp +++ b/lib/icinga/service.cpp @@ -121,18 +121,14 @@ int Service::GetSeverity() const } else if (state == ServiceOK) { severity = 0; } else { - switch (state) { - case ServiceWarning: - severity = 32; - break; - case ServiceUnknown: - severity = 64; - break; - case ServiceCritical: - severity = 128; - break; - default: - severity = 256; + if (state == ServiceWarning) { + severity = 32; + } else if (state == ServiceUnknown) { + severity = 64; + } else if (state == ServiceCritical) { + severity = 128; + } else { + severity = 256; } Host::Ptr host = GetHost(); From 01acfb47a99928dc378e60336a606cdaceb40276 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 13:03:43 +0200 Subject: [PATCH 3/9] Service::GetHost(): return early to remove a nesting level No change in functionality. The first two branches actually set the final return value for the method, so they can just return directly, removing the need to have the rest of the function inside an else block. --- lib/icinga/service.cpp | 57 +++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/lib/icinga/service.cpp b/lib/icinga/service.cpp index 954cd3d5e..406c7958c 100644 --- a/lib/icinga/service.cpp +++ b/lib/icinga/service.cpp @@ -111,39 +111,40 @@ Host::Ptr Service::GetHost() const * sort by severity. It is therefore easier to keep them seperated here. */ int Service::GetSeverity() const { - int severity; - ObjectLock olock(this); ServiceState state = GetStateRaw(); if (!HasBeenChecked()) { - severity = 16; - } else if (state == ServiceOK) { - severity = 0; - } else { - if (state == ServiceWarning) { - severity = 32; - } else if (state == ServiceUnknown) { - severity = 64; - } else if (state == ServiceCritical) { - severity = 128; - } else { - severity = 256; - } - - Host::Ptr host = GetHost(); - ObjectLock hlock (host); - if (host->GetState() != HostUp) { - severity += 1024; - } else if (IsAcknowledged()) { - severity += 512; - } else if (IsInDowntime()) { - severity += 256; - } else { - severity += 2048; - } - hlock.Unlock(); + return 16; } + if (state == ServiceOK) { + return 0; + } + + int severity = 0; + + if (state == ServiceWarning) { + severity = 32; + } else if (state == ServiceUnknown) { + severity = 64; + } else if (state == ServiceCritical) { + severity = 128; + } else { + severity = 256; + } + + Host::Ptr host = GetHost(); + ObjectLock hlock (host); + if (host->GetState() != HostUp) { + severity += 1024; + } else if (IsAcknowledged()) { + severity += 512; + } else if (IsInDowntime()) { + severity += 256; + } else { + severity += 2048; + } + hlock.Unlock(); olock.Unlock(); From c899d52e2fd5487ab07c87e5ce79693045c13360 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 13:05:47 +0200 Subject: [PATCH 4/9] Service::GetSeverity(): remove explicit unlocking No change in functionality. The ObjectLock destructor will implicitly release the locks when returning from the function. --- lib/icinga/service.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/icinga/service.cpp b/lib/icinga/service.cpp index 406c7958c..11d7c9633 100644 --- a/lib/icinga/service.cpp +++ b/lib/icinga/service.cpp @@ -144,9 +144,6 @@ int Service::GetSeverity() const } else { severity += 2048; } - hlock.Unlock(); - - olock.Unlock(); return severity; } From 6443f8997fa7bc20900548698b38e38d3beae9c7 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 13:19:21 +0200 Subject: [PATCH 5/9] Host::GetSeverity(): add braces to if statements No change in functionality, just makes the code a bit nicer. --- lib/icinga/host.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp index 36149d3dc..0fe7ec8eb 100644 --- a/lib/icinga/host.cpp +++ b/lib/icinga/host.cpp @@ -180,17 +180,19 @@ int Host::GetSeverity() const } else if (state == HostUp) { severity = 0; } else { - if (IsReachable()) + if (IsReachable()) { severity = 64; - else + } else { severity = 32; + } - if (IsAcknowledged()) + if (IsAcknowledged()) { severity += 512; - else if (IsInDowntime()) + } else if (IsInDowntime()) { severity += 256; - else + } else { severity += 2048; + } } olock.Unlock(); From 2ebee010f03e7e27eb45add59e65b4c1ca64ff1c Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 13:21:02 +0200 Subject: [PATCH 6/9] Host::GetHost(): return early to remove a nesting level No change in functionality. The first two branches actually set the final return value for the method, so they can just return directly, removing the need to have the rest of the function inside an else block. --- lib/icinga/host.cpp | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp index 0fe7ec8eb..0dbe361a9 100644 --- a/lib/icinga/host.cpp +++ b/lib/icinga/host.cpp @@ -170,29 +170,30 @@ HostState Host::GetLastHardState() const * sort by severity. It is therefore easier to keep them seperated here. */ int Host::GetSeverity() const { - int severity = 0; - ObjectLock olock(this); HostState state = GetState(); if (!HasBeenChecked()) { - severity = 16; - } else if (state == HostUp) { - severity = 0; - } else { - if (IsReachable()) { - severity = 64; - } else { - severity = 32; - } + return 16; + } + if (state == HostUp) { + return 0; + } - if (IsAcknowledged()) { - severity += 512; - } else if (IsInDowntime()) { - severity += 256; - } else { - severity += 2048; - } + int severity = 0; + + if (IsReachable()) { + severity = 64; + } else { + severity = 32; + } + + if (IsAcknowledged()) { + severity += 512; + } else if (IsInDowntime()) { + severity += 256; + } else { + severity += 2048; } olock.Unlock(); From d8271c656872ee98c1a3d888054315b94459c798 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 13:23:07 +0200 Subject: [PATCH 7/9] Host::GetSeverity(): remove explicit unlocking No change in functionality. The ObjectLock destructor will implicitly release the locks when returning from the function. --- lib/icinga/host.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp index 0dbe361a9..b85f8ccc9 100644 --- a/lib/icinga/host.cpp +++ b/lib/icinga/host.cpp @@ -196,8 +196,6 @@ int Host::GetSeverity() const severity += 2048; } - olock.Unlock(); - return severity; } From 1e05a166f13a718e443e20fad33dc968e8727270 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 13:25:01 +0200 Subject: [PATCH 8/9] Host::GetSeverity(): remove empty line at end of method --- lib/icinga/host.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp index b85f8ccc9..e50ba9b6f 100644 --- a/lib/icinga/host.cpp +++ b/lib/icinga/host.cpp @@ -197,7 +197,6 @@ int Host::GetSeverity() const } return severity; - } bool Host::IsStateOK(ServiceState state) const From 31a224c5095bc7918b2ca84875c8e54415ab2550 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 31 Mar 2025 13:53:14 +0200 Subject: [PATCH 9/9] Checkable::GetSeverity(): always take reachability into account So far, Service::GetSeverity() only considered the state of its own host, i.e. the implicit service to its own host dependency, and treated it similar to acknowledgements and downtimes. In contrast, Host::GetSeverity() considered reachability and treated it like a state, i.e. for the severity calculation, the host was either up, down, or unreachable. This commit changes the following things: 1. Make the service severity also consider explicitly configured dependencies by using IsReachable(). 2. Prefer acknowledgements and downtimes over unreachability in the severity calculation so that if an already acknowledged or in-downtime services (i.e. already handled service) becomes unreachable, it shouln't become more severe. 3. To unify host and service severities a bit, hosts now use the same logic that treats reachability more like acknowledgements/downtimes instead of like a state (changing the other way around would the state from the check plugin would not affect the severity for unrachable services anymore). --- lib/icinga/host.cpp | 10 +++------- lib/icinga/service.cpp | 8 +++----- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp index e50ba9b6f..22dd79b40 100644 --- a/lib/icinga/host.cpp +++ b/lib/icinga/host.cpp @@ -180,18 +180,14 @@ int Host::GetSeverity() const return 0; } - int severity = 0; - - if (IsReachable()) { - severity = 64; - } else { - severity = 32; - } + int severity = 32; // DOWN if (IsAcknowledged()) { severity += 512; } else if (IsInDowntime()) { severity += 256; + } else if (!IsReachable()) { + severity += 1024; } else { severity += 2048; } diff --git a/lib/icinga/service.cpp b/lib/icinga/service.cpp index 11d7c9633..c24647c82 100644 --- a/lib/icinga/service.cpp +++ b/lib/icinga/service.cpp @@ -133,14 +133,12 @@ int Service::GetSeverity() const severity = 256; } - Host::Ptr host = GetHost(); - ObjectLock hlock (host); - if (host->GetState() != HostUp) { - severity += 1024; - } else if (IsAcknowledged()) { + if (IsAcknowledged()) { severity += 512; } else if (IsInDowntime()) { severity += 256; + } else if (!IsReachable()) { + severity += 1024; } else { severity += 2048; }