[GitHub] [skywalking] chenmudu commented on a change in pull request #6888: Main include event to alarm

[GitBox <gi...@apache.org>]

chenmudu commented on a change in pull request #6888:
URL: https://github.com/apache/skywalking/pull/6888#discussion_r624622071



##########
File path: oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/AlarmQuery.java
##########
@@ -62,11 +85,166 @@ public Alarms getAlarm(final Duration duration, final Scope scope, final String
         }
         long startSecondTB = 0;
         long endSecondTB = 0;
+        EventQueryCondition condition = new EventQueryCondition();
         if (nonNull(duration)) {
             startSecondTB = duration.getStartTimeBucketInSec();
             endSecondTB = duration.getEndTimeBucketInSec();
+            condition.setTime(duration);
+        }
+        condition.setType(EventType.Error);
+        condition.setName("Alarm");

Review comment:
       Yeah, which means that there are fewer filters to query Events, which means that when an alert occurs, the number of Events associated is no longer the corresponding event, but all related Events.
   
   What I wanted to establish was whether Alarms and Event(s) had a unique connection, rather than all of the events associated with the application in a given time period.
   
   Because this association method may not be accurate and accurate enough in special scenarios, it may produce incorrect association relationship.What do you think? @kezhenxu94  @wu-sheng .
   
   Looking forward to your reply. Thanks a lot.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org