Revert #1166: un-mute the Advisor check now that #1165 is fixed - #1170
Open
bjagg wants to merge 1 commit into
Open
Revert #1166: un-mute the Advisor check now that #1165 is fixed#1170bjagg wants to merge 1 commit into
bjagg wants to merge 1 commit into
Conversation
…e its data defect is open" This reverts commit 69d60e6. The mute existed because the Advisor check failed 100% of the time on a known, tracked data defect, and a check that always fails trains people to filter the alert topic. LIF-Initiative#1167 fixed the defect and LIF-Initiative#1169 promoted it to demo, so the reason is gone. Verified before reverting, on demo: - the Advisor happy path passes on its own, unmuted - the query the Advisor actually issues (replayed from the semantic-search log) returns 10,386 bytes, down from 6,271,948 -- about 2,600 tokens against a 1,047,576 limit it used to blow by 50% - learner 100001 collapsed to its true fragment set: Name 2, Contact 2, Identifier 3, from 1,294 / 1,296 / 1,876 - CourseLearningExperience 5,096 -> 8 items, CredentialAward 5,733 -> 9 Leaving the mute in place after the fix is the worse failure: it would keep a working check from paging, and a green monitor would go on meaning "MDR/LDE is healthy" rather than "the demo works". Alerting still requires two consecutive failures before it pages, so a single flake after this lands stays silent -- un-muting does not reintroduce the noise the mute was written to stop.
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
1 participant
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 of Change
Reverts #1166. The Advisor check pages again.
The mute existed because the check failed 100% of the time on a known, tracked data defect, and a check that always fails just trains people to filter the alert topic. #1167 fixed the defect and #1169 promoted it to demo, so the reason is gone. Dereck called this revert out as item 2 in his #1167 review.
Verified on demo before reverting — the Advisor happy path passes unmuted, and the payload that broke it is gone:
Name/Contact/Identifier(learner 100001)CourseLearningExperienceCredentialAwardThe query is the exact GraphQL the semantic-search service logged, replayed against demo org1. The record collapsed to its true fragment set on its own after the first cache refresh — no data surgery, exactly as #1167 predicted.
Why not leave the mute in. It would keep a working check from paging, and a green monitor would go on meaning "MDR/LDE is healthy" rather than "the demo works" — which is the misreading the mute's own PR body warned about.
Why this doesn't reintroduce the noise. Alerting still requires two consecutive failures before it pages (#1157), so a single flake after this lands stays silent.
Related Issues
Refs #1165
Type of Change
Project Area(s) Affected
Checklist
Testing
Monitor run after the demo deploy:
success, with the Advisor step genuinely passing (Advisor happy path: success— not thecontinue-on-errorartifact, which is what the mute's summary line existed to disambiguate). Workflow YAML re-validated after the revert: 10 steps, nocontinue-on-error, everyif:condition intact.Additional Notes
Follow-up worth its own issue, found during the promotion:
.github/workflows/lif_query_cache_api.ymllistscomponents/lif/query_cache_service/**in its path filter but notcomponents/lif/composer/**, even thoughprojects/lif_query_cache_apiships that brick. #1167 rebuilt only because it happened to also touch the call site — a composer-only change would merge and silently never rebuild the image.