feat:Enhance the alarm kernel with recovered status notification capability for alarm rules #13539
Conversation
|
Apologies for the oversight. While merging the latest master code, the @BanyanDB.Group annotation in the AlarmRecoveryRecordclass was accidentally missed, which caused the e2e test failure @wankai123 @wu-sheng |
|
Take your time. |
| Long recoveryTime = getAlarmRecoveryTime(alarmRecord.getUuid(), duration); | ||
| AlarmMessage alarmMessage = buildAlarmMessage(alarmRecord, recoveryTime); |
There was a problem hiding this comment.
I have concerns about the way you are doing this. Querying status from a list usually results a bad performance.
There was a problem hiding this comment.
You should at least get the alarm list first. Then use the UUID list to retrieve the recovery list.
There was a problem hiding this comment.
Thank you for the helpful feedback. I've pushed new commits to address the points you raised. Please take another look when you have a moment, and let me know if anything else needs adjustment.
There was a problem hiding this comment.
It appears that the e2e test job on the GitHub Actions workflow was blocked and then got canceled. I'm not entirely sure if this is an issue on my end. I've sampled all the alarm e2e tests and some other tests that were not marked as completed; they all seemed to have passed verification. Is there anything I need to do on my side to allow them to run to completion?
|
Please fix CI. |
It appears that the e2e test job on the GitHub Actions workflow was blocked and then got canceled. I've sampled all the alarm e2e tests and some other tests that were not marked as completed; they all seemed to have passed verification. Could you please spare a moment to guide me on what I need to do to get them to run to completion? |
|
Are you setting the recovery quickly enough? They are running for over one hour, and be cancelled due to preset timeout |
It seems unrelated to the test cases. I observed that some test cases had been verified successfully before the 18-minute mark, but the test did not continue execute. like [E2E test (Alarm ES, test/e2e-v2/cases/alarm/es/e2e.yaml)] (https://github.com/apache/skywalking/actions/runs/18516094658/job/52781047577#logs) which just cost 10minute to detect recovery. |
|
Another PR just passed all the tests and merged. I assume if there is anything wrong, it is in this change. |
2 participants
Submodule PR:
skywalking-booster-ui#505
skywalking-query-protocol#153
If this is non-trivial feature, paste the links/URLs to the design doc.
Update the documentation to include this new feature.
Tests(including UT, IT, E2E) are added to verify the new feature.
If it's UI related, attach the screenshots below.
If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes [Feature] Enhance the alarm kernel with recovered status notification capability for alarm rules. #13492.
Update the
CHANGESlog.