You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/05/03 09:57:19 UTC
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #6888: Include event(s) to alarms.
kezhenxu94 commented on a change in pull request #6888:
URL: https://github.com/apache/skywalking/pull/6888#discussion_r624980069
##########
File path: oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/AlarmQuery.java
##########
@@ -62,11 +83,111 @@ 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);
+ }
+ Alarms alarms = getQueryService().getAlarm(
+ scopeId, keyword, paging, startSecondTB, endSecondTB, tags);
+ Events events = null;
+ try {
+ events = getEventQueryService().queryEvents(condition);
Review comment:
Also, I don't think we want to include the `Alarm` events in the alarm messages, it seems to be pointless to me.
##########
File path: oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/AlarmQuery.java
##########
@@ -62,11 +83,111 @@ 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);
+ }
+ Alarms alarms = getQueryService().getAlarm(
+ scopeId, keyword, paging, startSecondTB, endSecondTB, tags);
+ Events events = null;
+ try {
+ events = getEventQueryService().queryEvents(condition);
+ } catch (Throwable e) {
+ LOGGER.error(e.getMessage(), e);
+ return alarms;
+ }
+ return includeEvents2Alarms(alarms, events);
+ }
+
+ private Alarms includeEvents2Alarms(Alarms alarms, Events events) {
+ if (alarms.getTotal() < 1 || events.getTotal() < 1) {
+ return alarms;
}
- return getQueryService().getAlarm(
- scopeId, keyword, paging, startSecondTB, endSecondTB, tags);
+ Map<String, List<Event>> mappingMap = events.getEvents().stream().collect(Collectors.groupingBy(Event::getServiceInSource));
+ alarms.getMsgs().forEach(a -> {
+ switch (a.getScopeId()) {
+ case DefaultScopeDefine.SERVICE :
+ List<Event> serviceEvent = mappingMap.get(IDManager.ServiceID.analysisId(a.getId()).getName());
+ if (CollectionUtils.isNotEmpty(serviceEvent)) {
+ a.setEvents(serviceEvent);
+ }
+ break;
+ case DefaultScopeDefine.SERVICE_RELATION :
+ List<Event> sourceServiceEvent = mappingMap.get(IDManager.ServiceID.analysisId(a.getId()));
+ List<Event> destServiceEvent = mappingMap.get(IDManager.ServiceID.analysisId(a.getId1()));
+ if (CollectionUtils.isNotEmpty(sourceServiceEvent)) {
+ a.setEvents(sourceServiceEvent);
+ }
+ if (CollectionUtils.isNotEmpty(destServiceEvent)) {
+ a.getEvents().addAll(destServiceEvent);
+ }
+ break;
+ case DefaultScopeDefine.SERVICE_INSTANCE :
+ IDManager.ServiceInstanceID.InstanceIDDefinition instanceIDDefinition = IDManager.ServiceInstanceID.analysisId(a.getId());
+ String serviceInstanceName = instanceIDDefinition.getName();
+ String serviceName = IDManager.ServiceID.analysisId(instanceIDDefinition.getServiceId()).getName();
+ List<Event> serviceInstanceEvent = mappingMap.get(serviceName);
+ if (CollectionUtils.isNotEmpty(serviceInstanceEvent)) {
+ List<Event> filterEvents = serviceInstanceEvent.stream().filter(e -> StringUtils.equals(e.getSource().getServiceInstance(), serviceInstanceName)).collect(Collectors.toList());
+ a.setEvents(filterEvents);
+ }
+ break;
+ case DefaultScopeDefine.SERVICE_INSTANCE_RELATION :
+ IDManager.ServiceInstanceID.InstanceIDDefinition sourceInstanceIDDefinition = IDManager.ServiceInstanceID.analysisId(a.getId());
+ String sourceServiceInstanceName = sourceInstanceIDDefinition.getName();
+ String sourceServiceName = IDManager.ServiceID.analysisId(sourceInstanceIDDefinition.getServiceId()).getName();
+
+ IDManager.ServiceInstanceID.InstanceIDDefinition destInstanceIDDefinition = IDManager.ServiceInstanceID.analysisId(a.getId1());
+ String destServiceInstanceName = destInstanceIDDefinition.getName();
+ String destServiceName = IDManager.ServiceID.analysisId(destInstanceIDDefinition.getServiceId()).getName();
+
+ List<Event> sourceInstanceEvent = mappingMap.get(sourceServiceName);
+ List<Event> destInstanceEvent = mappingMap.get(destServiceName);
+
+ if (CollectionUtils.isNotEmpty(sourceInstanceEvent)) {
+ List<Event> filterEvents = sourceInstanceEvent.stream().filter(e -> StringUtils.equals(e.getSource().getServiceInstance(), sourceServiceInstanceName)).collect(Collectors.toList());
+ a.setEvents(filterEvents);
+ }
+ if (CollectionUtils.isNotEmpty(destInstanceEvent)) {
+ List<Event> filterEvents = destInstanceEvent.stream().filter(e -> StringUtils.equals(e.getSource().getServiceInstance(), destServiceInstanceName)).collect(Collectors.toList());
+ a.getEvents().addAll(filterEvents);
+ }
+ break;
+ case DefaultScopeDefine.ENDPOINT :
Review comment:
As for the `endpoint` scope, I'm wondering what do you think about the idea that we simply fallback to the service instance level events, because there are rarely endpoint events in my opinion, but endpoint alarms are highly possible triggered by instance events. Associating endpoint alarm with endpoint events ONLY may be correct, but it's highly possible to miss the relationship between them once there missed the endpoint events.
##########
File path: oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/resolver/AlarmQuery.java
##########
@@ -62,11 +83,111 @@ 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);
+ }
+ Alarms alarms = getQueryService().getAlarm(
+ scopeId, keyword, paging, startSecondTB, endSecondTB, tags);
+ Events events = null;
+ try {
+ events = getEventQueryService().queryEvents(condition);
Review comment:
> I think we should the alarm list first, and query the event accordingly, @kezhenxu94 What do you think?
> If you have concerns about performance, try concurrently query.
+1, please narrow the `condition` as much as possible, querying events by time range may cause critical performance problem
--
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