test(core): stop the alarm collector losing events under concurrency - #79
Merged
Merged
Conversation
identity_route_e2e failed about 1 run in 6 under cargo test, always by collecting zero alarms. Callsite interest is global and cached, while with_default is thread-local: another test in the binary loading a config with no subscriber registered the same warn! sites under NoSubscriber, which caches never, and later collectors saw nothing. Install one subscriber globally, return Interest::sometimes so no verdict is cached, and route events to a per-thread sink. 40 runs clean, and make ci green three times where it was a coin flip. Verified the assertion still bites by suppressing the alarm. dispatch_mode_e2e and delegation_identity_warning share the pattern but do not manifest it: 0/12 each. Left alone. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
github-project-automation
Bot
moved this from Review
to Done
in Praxis Policy Engine (PPE)
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
2 participants
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.
Description
crates/ppe-core/tests/identity_route_e2e.rsfailed about 1 run in 6 undercargo test, somake ciandmake coveragewere intermittently red. This was meant to go into #72 but that merged first.The failure was always zero alarms collected, never a wrong count. Callsite interest in
tracingis global and cached, whilewith_defaultis thread-local. Other tests in the same binary load a config with no subscriber installed, which registers thosewarn!callsites underNoSubscriber; that cachesInterest::never()for the process, and every later collector sees nothing.Changes
NoSubscribernever registers these callsites.register_callsitereturnsInterest::sometimes(), so no verdict is cached andenabledis asked per event.Testing
Measured before: 0/10 failures with the alarm tests alone, 0/6 with
--test-threads=1, about 2/6 for the full binary concurrent. Addingrebuild_interest_cache()alone cut it to 1/12, which is what identified the cache as the cause and showed a thread-local subscriber could not fix it.After: 0/40 runs on the original branch, 0/20 here, and
make cigreen four consecutive times. Confirmed the test still fails when the alarm is suppressed, so it is not passing vacuously.dispatch_mode_e2e.rsanddelegation_identity_warning.rshand-roll the same pattern but do not manifest it, 0/12 each, because their binaries have no no-subscriber config loads. Left alone; they would start failing if that changes.