Ask the narrower question once, instead of in every handler - #31
Merged
Conversation
ivanvyd
force-pushed
the
feature/health-changed-event
branch
from
July 29, 2026 19:12
1c87cb3 to
8ad11f4
Compare
StateChanged has been listed as a known issue since the event existed: it is raised on every check rather than only when something meaningful changes, because a stored result always moves the last execution time and that counts towards state equality. It is not a defect. The dashboard redraws from exactly those events -- every check updates a row's last-run time and pushes a blip onto its pulse strip -- so raising it less often would break the thing the event mainly exists for. What was missing was a way to ask the narrower question, and without one, every handler that only cared about a component going down reached into OldState and NewState.LastResult?.Health and worked it out for itself. The alerting and uptime packages were doing that identically, three lines each, which is the point at which a concept belongs to the type rather than to its readers. PulseCheckerStateChangedEventArgs now carries PreviousHealth, CurrentHealth and HealthChanged, and both packages read them. Two edges are deliberate. A first result is a change: going from nothing known to Healthy is the first thing anyone learns about the component, and a rule that ignored it would stay silent through a cold start into an outage. Losing a result is not: a state with no result says nothing about health, so treating it as a transition would raise an alert whose current health is nothing at all. The changelog and the README roadmap both described this as an outstanding problem. They now describe what the event does and how to ask for less.
ivanvyd
force-pushed
the
feature/health-changed-event
branch
from
July 29, 2026 19:30
8ad11f4 to
1f5d039
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
StateChangedhas been listed as a known issue since the event existed — inCHANGELOG.mdunder Known issues and in the README roadmap under What is left:It is not a defect, and "fixing" it would have broken the dashboard.
The dashboard is event-driven by design: it subscribes to
StateChangedand updates one entry in place, with a full read only on first load. Every check updates that row's last-run time and pushes a blip onto its pulse strip. Raising the event less often would silently stop the board from updating between health transitions — which, given the 1s clock tick redraws anyway, would have looked like it still worked.What was actually missing was a way to ask the narrower question. Without one, every handler that only cared about a component going down reached into the two states and worked it out itself. Two packages were doing that identically:
Three lines, twice, is where a concept stops belonging to its readers and starts belonging to the type.
The change
PulseCheckerStateChangedEventArgsgains three computed properties:PreviousHealthnullif no check had produced oneCurrentHealthnullif there is still no resultHealthChangedBoth packages now read them. Purely additive — no existing member changed, no behaviour changed.
Two edges, both deliberate and both tested
A first result is a change. Going from nothing known to
Healthyis the first thing anyone learns about the component; a rule that ignored it would stay silent through a cold start straight into an outage.Losing a result is not. A state with no result says nothing about health, so treating it as a transition would raise an alert whose current health is nothing at all. This is why
HealthChangedisCurrentHealth is not null && PreviousHealth != CurrentHealthrather than a plain!=.Verification
374 tests green on both net8.0 and net10.0. The 32 existing alerting and uptime tests pass unchanged, which is the evidence that the refactor preserved behaviour rather than merely compiling.
Six new tests, mutation-checked — a passing test proves nothing until it has been seen to fail:
HealthChangeddrops the null guard (plain!=)CurrentHealthreads the old stateNote on ordering
Cut from
main, so it does not depend on #30. Both branches delete a different bullet from the same Known issues list, so whichever merges second may need a one-line conflict resolution there. Nothing else overlaps.No release — version stays at 3.1.4.