Copilot commented on code in PR #13570: URL: https://github.com/apache/skywalking/pull/13570#discussion_r2508606735
########## docs/en/status/query_alarm_runtime_status.md: ########## @@ -23,139 +50,211 @@ Return the list of alarm running rules. Return the detailed information of the alarm running rule. -- URL, `http://{core restHost}:{core restPort}/status/alarm/rules/{ruleName}` +- URL, `http://{core restHost}:{core restPort}/status/alarm/rules/{ruleId}` - HTTP GET method. ```json { - "ruleName": "service_resp_time_rule", - "expression": "sum(service_resp_time > baseline(service_resp_time,upper)) >= 1", - "period": 10, - "silentPeriod": 10, - "additonalPeriod": 0, - "includeNames": [ - "mock_a_service", - "mock_b_service", - "mock_c_service" - ], - "excludeNames": [], - "includeNamesRegex": "", - "excludeNamesRegex": "", - "affectedEntities": [ - { - "scope": "SERVICE", - "name": "mock_b_service" - }, - { - "scope": "SERVICE", - "name": "mock_a_service" - }, - { - "scope": "SERVICE", - "name": "mock_c_service" - } - ], - "tags": [ + "oapInstances": [ { - "key": "level", - "value": "WARNING" - } - ], - "hooks": [ - "webhook.default", - "wechat.default" - ], - "includeMetrics": [ - "service_resp_time" - ], - "formattedMessages": [ - { - "mock_b_service": "Response time of service mock_b_service is more than upper baseline in 1 minutes of last 10 minutes." - }, - { - "mock_a_service": "Response time of service mock_a_service is more than upper baseline in 1 minutes of last 10 minutes." + "address": "127.0.0.1_11800", + "status": { + "ruleId": "service_resp_time_rule", + "expression": "sum(service_resp_time > 1000) >= 1", + "period": 10, + "silencePeriod": 10, + "additionalPeriod": 0, + "includeEntityNames": [], + "excludeEntityNames": [], + "includeEntityNamesRegex": "", + "excludeEntityNamesRegex": "", + "runningEntities": [ + { + "scope": "SERVICE", + "name": "mock_b_service", + "formattedMessage": "Response time of mock_b_service is more than upper baseline in 1 minutes of last 10 minutes." + } + ], + "tags": [ + { + "key": "level", + "value": "WARNING" + } + ], + "hooks": [ + "webhook.default", + "wechat.default" + ], + "includeMetrics": [ + "service_resp_time" + ] + } }, { - "mock_c_service": "Response time of service mock_c_service is more than upper baseline in 1 minutes of last 10 minutes." + "address": "127.0.0.1_11801", + "status": { + "ruleId": "service_resp_time_rule", + "expression": "sum(service_resp_time > 1000) >= 1", + "period": 10, + "silencePeriod": 10, + "additionalPeriod": 0, + "includeEntityNames": [], + "excludeEntityNames": [], + "includeEntityNamesRegex": "", + "excludeEntityNamesRegex": "", + "runningEntities": [ + { + "scope": "SERVICE", + "name": "mock_a_service", + "formattedMessage": "Response time of mock_a_serviceis more than upper baseline in 1 minutes of last 10 minutes." Review Comment: Missing space in the formatted message between "mock_a_service" and "is". Should be "Response time of mock_a_service is more than..." instead of "Response time of mock_a_serviceis more than...". ```suggestion "formattedMessage": "Response time of mock_a_service is more than upper baseline in 1 minutes of last 10 minutes." ``` ########## oap-server/server-query-plugin/status-query-plugin/src/main/java/org/apache/skywalking/oap/query/debug/AlarmStatusQueryService.java: ########## @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.query.debug; + +import com.google.gson.Gson; +import java.util.List; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.core.CoreModule; +import org.apache.skywalking.oap.server.core.alarm.AlarmModule; +import org.apache.skywalking.oap.server.core.alarm.AlarmRulesWatcherService; +import org.apache.skywalking.oap.server.core.alarm.AlarmStatusWatcherService; +import org.apache.skywalking.oap.server.core.alarm.provider.AlarmRulesWatcher; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRuleDetail; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRuleList; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRunningContext; +import org.apache.skywalking.oap.server.core.alarm.provider.status.ClusterAlarmStatus; +import org.apache.skywalking.oap.server.core.alarm.provider.status.InstanceAlarmStatus; +import org.apache.skywalking.oap.server.core.remote.client.RemoteClient; +import org.apache.skywalking.oap.server.core.remote.client.RemoteClientManager; +import org.apache.skywalking.oap.server.core.remote.client.SelfRemoteClient; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.AlarmRequest; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.RemoteServiceGrpc; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.StatusRequest; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.StatusResponse; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.apache.skywalking.oap.server.library.module.Service; + +@Slf4j +public class AlarmStatusQueryService implements Service { + private final ModuleManager moduleManager; + private final static Gson GSON = new Gson(); + private AlarmRulesWatcher alarmRulesWatcher; + private AlarmStatusWatcherService alarmStatusWatcher; + private RemoteClientManager remoteClientManager; + + public AlarmStatusQueryService(final ModuleManager manager) { + this.moduleManager = manager; + } + + private AlarmRulesWatcher getAlarmRulesWatcher() { + if (alarmRulesWatcher == null) { + alarmRulesWatcher = (AlarmRulesWatcher) moduleManager.find(AlarmModule.NAME) + .provider().getService(AlarmRulesWatcherService.class); + } + return alarmRulesWatcher; + } + + private AlarmStatusWatcherService getAlarmStatusWatcher() { + if (alarmStatusWatcher == null) { + alarmStatusWatcher = moduleManager.find(AlarmModule.NAME) + .provider().getService(AlarmStatusWatcherService.class); + } + return alarmStatusWatcher; + } + + private RemoteClientManager getRemoteClientManager() { + if (remoteClientManager == null) { + remoteClientManager = moduleManager.find(CoreModule.NAME) + .provider() + .getService(RemoteClientManager.class); + } + return remoteClientManager; + } + + public ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleList>> getAlarmRules() { + ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleList>> result = new ClusterAlarmStatus<>(); + List<RemoteClient> list = getRemoteClientManager().getRemoteClient(); + for (RemoteClient remoteClient : list) { + String rulesInfo = null; + String errorMsg = null; + try { + if (remoteClient instanceof SelfRemoteClient) { + rulesInfo = getAlarmStatusWatcher().getAlarmRules(); + } else { + // get from remote oap in the cluster + RemoteServiceGrpc.RemoteServiceBlockingStub stub = RemoteServiceGrpc.newBlockingStub( + remoteClient.getChannel()); + AlarmRequest alarmRequest = AlarmRequest.newBuilder() + .setRequestType(AlarmRequest.RequestType.GET_ALARM_RULES) + .build(); + StatusResponse statusResponse = stub.syncStatus( + StatusRequest.newBuilder().setAlarmRequest(alarmRequest).build()); + rulesInfo = statusResponse.getAlarmStatus(); + } + } catch (Exception e) { + log.warn("Failed to get alarm rule list.", e); + errorMsg = e.getMessage(); + } Review Comment: Potential null pointer exception: if `rulesInfo` is null when an exception is caught, calling `GSON.fromJson(null, AlarmRuleList.class)` will return null, which is then set to `instanceAlarmStatus.setStatus(null)`. While this may be intentional to indicate an error state, consider explicitly handling the null case or initializing `rulesInfo` to an empty JSON object to avoid potential issues downstream. ```suggestion } if (rulesInfo == null) { rulesInfo = "{}"; } ``` ########## oap-server/server-query-plugin/status-query-plugin/src/main/java/org/apache/skywalking/oap/query/debug/AlarmStatusQueryService.java: ########## @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.query.debug; + +import com.google.gson.Gson; +import java.util.List; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.core.CoreModule; +import org.apache.skywalking.oap.server.core.alarm.AlarmModule; +import org.apache.skywalking.oap.server.core.alarm.AlarmRulesWatcherService; +import org.apache.skywalking.oap.server.core.alarm.AlarmStatusWatcherService; +import org.apache.skywalking.oap.server.core.alarm.provider.AlarmRulesWatcher; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRuleDetail; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRuleList; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRunningContext; +import org.apache.skywalking.oap.server.core.alarm.provider.status.ClusterAlarmStatus; +import org.apache.skywalking.oap.server.core.alarm.provider.status.InstanceAlarmStatus; +import org.apache.skywalking.oap.server.core.remote.client.RemoteClient; +import org.apache.skywalking.oap.server.core.remote.client.RemoteClientManager; +import org.apache.skywalking.oap.server.core.remote.client.SelfRemoteClient; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.AlarmRequest; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.RemoteServiceGrpc; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.StatusRequest; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.StatusResponse; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.apache.skywalking.oap.server.library.module.Service; + +@Slf4j +public class AlarmStatusQueryService implements Service { + private final ModuleManager moduleManager; + private final static Gson GSON = new Gson(); + private AlarmRulesWatcher alarmRulesWatcher; + private AlarmStatusWatcherService alarmStatusWatcher; + private RemoteClientManager remoteClientManager; + + public AlarmStatusQueryService(final ModuleManager manager) { + this.moduleManager = manager; + } + + private AlarmRulesWatcher getAlarmRulesWatcher() { + if (alarmRulesWatcher == null) { + alarmRulesWatcher = (AlarmRulesWatcher) moduleManager.find(AlarmModule.NAME) + .provider().getService(AlarmRulesWatcherService.class); + } + return alarmRulesWatcher; + } + + private AlarmStatusWatcherService getAlarmStatusWatcher() { + if (alarmStatusWatcher == null) { + alarmStatusWatcher = moduleManager.find(AlarmModule.NAME) + .provider().getService(AlarmStatusWatcherService.class); + } + return alarmStatusWatcher; + } + + private RemoteClientManager getRemoteClientManager() { + if (remoteClientManager == null) { + remoteClientManager = moduleManager.find(CoreModule.NAME) + .provider() + .getService(RemoteClientManager.class); + } + return remoteClientManager; + } + + public ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleList>> getAlarmRules() { + ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleList>> result = new ClusterAlarmStatus<>(); + List<RemoteClient> list = getRemoteClientManager().getRemoteClient(); + for (RemoteClient remoteClient : list) { + String rulesInfo = null; + String errorMsg = null; + try { + if (remoteClient instanceof SelfRemoteClient) { + rulesInfo = getAlarmStatusWatcher().getAlarmRules(); + } else { + // get from remote oap in the cluster + RemoteServiceGrpc.RemoteServiceBlockingStub stub = RemoteServiceGrpc.newBlockingStub( + remoteClient.getChannel()); + AlarmRequest alarmRequest = AlarmRequest.newBuilder() + .setRequestType(AlarmRequest.RequestType.GET_ALARM_RULES) + .build(); + StatusResponse statusResponse = stub.syncStatus( + StatusRequest.newBuilder().setAlarmRequest(alarmRequest).build()); + rulesInfo = statusResponse.getAlarmStatus(); + } + } catch (Exception e) { + log.warn("Failed to get alarm rule list.", e); + errorMsg = e.getMessage(); + } + AlarmRuleList alarmRuleList = GSON.fromJson(rulesInfo, AlarmRuleList.class); + InstanceAlarmStatus<AlarmRuleList> instanceAlarmStatus = new InstanceAlarmStatus<>(); + instanceAlarmStatus.setStatus(alarmRuleList); + instanceAlarmStatus.setAddress(remoteClient.getAddress().toString()); + instanceAlarmStatus.setErrorMsg(errorMsg); + result.getOapInstances().add(instanceAlarmStatus); + } + return result; + } + + public ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleDetail>> getAlarmRuleById(String ruleId) { + ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleDetail>> result = new ClusterAlarmStatus<>(); + List<RemoteClient> list = getRemoteClientManager().getRemoteClient(); + for (RemoteClient remoteClient : list) { + String ruleDetail = null; + String errorMsg = null; + try { + if (remoteClient instanceof SelfRemoteClient) { + ruleDetail = getAlarmStatusWatcher().getAlarmRuleById(ruleId); + } else { + // get from remote oap in the cluster + RemoteServiceGrpc.RemoteServiceBlockingStub stub = RemoteServiceGrpc.newBlockingStub( + remoteClient.getChannel()); + AlarmRequest alarmRequest = AlarmRequest.newBuilder() + .setRequestType( + AlarmRequest.RequestType.GET_ALARM_RULE_BY_ID) + .setRuleId(ruleId) + .build(); + StatusResponse statusResponse = stub.syncStatus( + StatusRequest.newBuilder().setAlarmRequest(alarmRequest).build()); + ruleDetail = statusResponse.getAlarmStatus(); + } + } catch (Exception e) { + log.warn("Failed to get alarm rule detail by ID: {}.", ruleId, e); + errorMsg = e.getMessage(); + } + + AlarmRuleDetail alarmRuleDetail = GSON.fromJson(ruleDetail, AlarmRuleDetail.class); Review Comment: Potential null pointer exception: if `ruleDetail` is null when an exception is caught, calling `GSON.fromJson(null, AlarmRuleDetail.class)` will return null, which is then set to `instanceAlarmRuleDetail.setStatus(null)`. Consider handling the null case explicitly or initializing `ruleDetail` to an empty JSON object. ########## oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/status/AlarmRunningContext.java: ########## @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.server.core.alarm.provider.status; + +import com.google.gson.JsonObject; +import java.util.ArrayList; +import java.util.List; +import lombok.Data; + +@Data +public class AlarmRunningContext { + private String ruleId; + private String expression; + private String endTime; + private int additionalPeriod; + private int size; + private int silenceCountdown; + private String entityName; + private List<WindowValue> windowValues = new ArrayList<>(); + private JsonObject mqeMetricsSnapshot; + + @Data + public static class Metric { + private String name; + private long timeBucket; + private String value; + } + + @Data + public static class WindowValue { + private int index; + private List<Metric> metrics = new ArrayList<>(); Review Comment: getMetrics exposes the internal representation stored in field metrics. The value may be modified [after this call to getMetrics](1). ########## oap-server/server-query-plugin/status-query-plugin/src/main/java/org/apache/skywalking/oap/query/debug/AlarmStatusQueryService.java: ########## @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.query.debug; + +import com.google.gson.Gson; +import java.util.List; +import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.core.CoreModule; +import org.apache.skywalking.oap.server.core.alarm.AlarmModule; +import org.apache.skywalking.oap.server.core.alarm.AlarmRulesWatcherService; +import org.apache.skywalking.oap.server.core.alarm.AlarmStatusWatcherService; +import org.apache.skywalking.oap.server.core.alarm.provider.AlarmRulesWatcher; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRuleDetail; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRuleList; +import org.apache.skywalking.oap.server.core.alarm.provider.status.AlarmRunningContext; +import org.apache.skywalking.oap.server.core.alarm.provider.status.ClusterAlarmStatus; +import org.apache.skywalking.oap.server.core.alarm.provider.status.InstanceAlarmStatus; +import org.apache.skywalking.oap.server.core.remote.client.RemoteClient; +import org.apache.skywalking.oap.server.core.remote.client.RemoteClientManager; +import org.apache.skywalking.oap.server.core.remote.client.SelfRemoteClient; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.AlarmRequest; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.RemoteServiceGrpc; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.StatusRequest; +import org.apache.skywalking.oap.server.core.remote.grpc.proto.StatusResponse; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.apache.skywalking.oap.server.library.module.Service; + +@Slf4j +public class AlarmStatusQueryService implements Service { + private final ModuleManager moduleManager; + private final static Gson GSON = new Gson(); + private AlarmRulesWatcher alarmRulesWatcher; + private AlarmStatusWatcherService alarmStatusWatcher; + private RemoteClientManager remoteClientManager; + + public AlarmStatusQueryService(final ModuleManager manager) { + this.moduleManager = manager; + } + + private AlarmRulesWatcher getAlarmRulesWatcher() { + if (alarmRulesWatcher == null) { + alarmRulesWatcher = (AlarmRulesWatcher) moduleManager.find(AlarmModule.NAME) + .provider().getService(AlarmRulesWatcherService.class); + } + return alarmRulesWatcher; + } + + private AlarmStatusWatcherService getAlarmStatusWatcher() { + if (alarmStatusWatcher == null) { + alarmStatusWatcher = moduleManager.find(AlarmModule.NAME) + .provider().getService(AlarmStatusWatcherService.class); + } + return alarmStatusWatcher; + } + + private RemoteClientManager getRemoteClientManager() { + if (remoteClientManager == null) { + remoteClientManager = moduleManager.find(CoreModule.NAME) + .provider() + .getService(RemoteClientManager.class); + } + return remoteClientManager; + } + + public ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleList>> getAlarmRules() { + ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleList>> result = new ClusterAlarmStatus<>(); + List<RemoteClient> list = getRemoteClientManager().getRemoteClient(); + for (RemoteClient remoteClient : list) { + String rulesInfo = null; + String errorMsg = null; + try { + if (remoteClient instanceof SelfRemoteClient) { + rulesInfo = getAlarmStatusWatcher().getAlarmRules(); + } else { + // get from remote oap in the cluster + RemoteServiceGrpc.RemoteServiceBlockingStub stub = RemoteServiceGrpc.newBlockingStub( + remoteClient.getChannel()); + AlarmRequest alarmRequest = AlarmRequest.newBuilder() + .setRequestType(AlarmRequest.RequestType.GET_ALARM_RULES) + .build(); + StatusResponse statusResponse = stub.syncStatus( + StatusRequest.newBuilder().setAlarmRequest(alarmRequest).build()); + rulesInfo = statusResponse.getAlarmStatus(); + } + } catch (Exception e) { + log.warn("Failed to get alarm rule list.", e); + errorMsg = e.getMessage(); + } + AlarmRuleList alarmRuleList = GSON.fromJson(rulesInfo, AlarmRuleList.class); + InstanceAlarmStatus<AlarmRuleList> instanceAlarmStatus = new InstanceAlarmStatus<>(); + instanceAlarmStatus.setStatus(alarmRuleList); + instanceAlarmStatus.setAddress(remoteClient.getAddress().toString()); + instanceAlarmStatus.setErrorMsg(errorMsg); + result.getOapInstances().add(instanceAlarmStatus); + } + return result; + } + + public ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleDetail>> getAlarmRuleById(String ruleId) { + ClusterAlarmStatus<InstanceAlarmStatus<AlarmRuleDetail>> result = new ClusterAlarmStatus<>(); + List<RemoteClient> list = getRemoteClientManager().getRemoteClient(); + for (RemoteClient remoteClient : list) { + String ruleDetail = null; + String errorMsg = null; + try { + if (remoteClient instanceof SelfRemoteClient) { + ruleDetail = getAlarmStatusWatcher().getAlarmRuleById(ruleId); + } else { + // get from remote oap in the cluster + RemoteServiceGrpc.RemoteServiceBlockingStub stub = RemoteServiceGrpc.newBlockingStub( + remoteClient.getChannel()); + AlarmRequest alarmRequest = AlarmRequest.newBuilder() + .setRequestType( + AlarmRequest.RequestType.GET_ALARM_RULE_BY_ID) + .setRuleId(ruleId) + .build(); + StatusResponse statusResponse = stub.syncStatus( + StatusRequest.newBuilder().setAlarmRequest(alarmRequest).build()); + ruleDetail = statusResponse.getAlarmStatus(); + } + } catch (Exception e) { + log.warn("Failed to get alarm rule detail by ID: {}.", ruleId, e); + errorMsg = e.getMessage(); + } + + AlarmRuleDetail alarmRuleDetail = GSON.fromJson(ruleDetail, AlarmRuleDetail.class); + InstanceAlarmStatus<AlarmRuleDetail> instanceAlarmRuleDetail = new InstanceAlarmStatus<>(); + instanceAlarmRuleDetail.setStatus(alarmRuleDetail); + instanceAlarmRuleDetail.setAddress(remoteClient.getAddress().toString()); + instanceAlarmRuleDetail.setErrorMsg(errorMsg); + result.getOapInstances().add(instanceAlarmRuleDetail); + } + return result; + } + + public ClusterAlarmStatus<InstanceAlarmStatus<AlarmRunningContext>> getAlarmRuleContext(String ruleId, String entityName) { + ClusterAlarmStatus<InstanceAlarmStatus<AlarmRunningContext>> result = new ClusterAlarmStatus<>(); + List<RemoteClient> list = getRemoteClientManager().getRemoteClient(); + for (RemoteClient remoteClient : list) { + String context = null; + String errorMsg = null; + try { + if (remoteClient instanceof SelfRemoteClient) { + context = getAlarmStatusWatcher().getAlarmRuleContext(ruleId, entityName); + } else { + // get from remote oap in the cluster + RemoteServiceGrpc.RemoteServiceBlockingStub stub = RemoteServiceGrpc.newBlockingStub( + remoteClient.getChannel()); + AlarmRequest alarmRequest = AlarmRequest.newBuilder() + .setRequestType( + AlarmRequest.RequestType.GET_ALARM_RULE_CONTEXT) + .setRuleId(ruleId) + .setEntityName(entityName) + .build(); + StatusResponse statusResponse = stub.syncStatus( + StatusRequest.newBuilder().setAlarmRequest(alarmRequest).build()); + context = statusResponse.getAlarmStatus(); + } + } catch (Exception e) { + log.warn("Failed to get alarm running context by ruleId: {} and entityName: {}.", ruleId, entityName, e); + errorMsg = e.getMessage(); + } Review Comment: Potential null pointer exception: if `context` is null when an exception is caught, calling `GSON.fromJson(null, AlarmRunningContext.class)` will return null, which is then set to `runningContext.setStatus(null)`. Consider handling the null case explicitly or initializing `context` to an empty JSON object. ```suggestion } if (context == null) { context = "{}"; } ``` ########## oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/status/AlarmRuleList.java: ########## @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.server.core.alarm.provider.status; + +import java.util.ArrayList; +import java.util.List; +import lombok.Data; + +@Data +public class AlarmRuleList { + private List<RuleInfo> ruleList = new ArrayList<>(); + + @Data Review Comment: getRuleList exposes the internal representation stored in field ruleList. The value may be modified [after this call to getRuleList](1). ```suggestion import java.util.Collections; public class AlarmRuleList { private final List<RuleInfo> ruleList = new ArrayList<>(); /** * Returns an unmodifiable view of the rule list to prevent external modification. */ public List<RuleInfo> getRuleList() { return Collections.unmodifiableList(ruleList); } // If you need to add rules, provide controlled methods, e.g.: public void addRule(RuleInfo rule) { ruleList.add(rule); } @lombok.Data ``` ########## oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/status/AlarmRuleDetail.java: ########## @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.server.core.alarm.provider.status; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import lombok.Data; +import org.apache.skywalking.oap.server.core.analysis.manual.searchtag.Tag; + +@Data +public class AlarmRuleDetail { + private String ruleId; + private String expression; + private int period; + private int silencePeriod; + private int additionalPeriod; + private List<String> includeEntityNames = new ArrayList<>(); + private List<String> excludeEntityNames = new ArrayList<>(); + private String includeEntityNamesRegex; + private String excludeEntityNamesRegex; + private List<RunningEntity> runningEntities = new ArrayList<>(); Review Comment: getRunningEntities exposes the internal representation stored in field runningEntities. The value may be modified [after this call to getRunningEntities](1). ########## oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/status/ClusterAlarmStatus.java: ########## @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.server.core.alarm.provider.status; + +import java.util.ArrayList; +import java.util.List; +import lombok.Data; + +@Data +public class ClusterAlarmStatus<T> { + private List<T> oapInstances = new ArrayList<>(); Review Comment: getOapInstances exposes the internal representation stored in field oapInstances. The value may be modified [after this call to getOapInstances](1). getOapInstances exposes the internal representation stored in field oapInstances. The value may be modified [after this call to getOapInstances](2). getOapInstances exposes the internal representation stored in field oapInstances. The value may be modified [after this call to getOapInstances](3). ```suggestion import java.util.List; import java.util.Collections; import lombok.Data; import lombok.Getter; import lombok.Setter; import lombok.AccessLevel; @Data public class ClusterAlarmStatus<T> { @Getter(AccessLevel.NONE) @Setter(AccessLevel.NONE) private List<T> oapInstances = new ArrayList<>(); public List<T> getOapInstances() { return Collections.unmodifiableList(oapInstances); } ``` ########## oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/status/AlarmRunningContext.java: ########## @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.server.core.alarm.provider.status; + +import com.google.gson.JsonObject; +import java.util.ArrayList; +import java.util.List; +import lombok.Data; + +@Data +public class AlarmRunningContext { + private String ruleId; + private String expression; + private String endTime; + private int additionalPeriod; + private int size; + private int silenceCountdown; + private String entityName; + private List<WindowValue> windowValues = new ArrayList<>(); Review Comment: getWindowValues exposes the internal representation stored in field windowValues. The value may be modified [after this call to getWindowValues](1). ########## oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/remote/client/GRPCRemoteClient.java: ########## @@ -110,7 +110,7 @@ public void connect() { * * @return a channel when the state to be ready */ Review Comment: This method overrides [RemoteClient.getChannel](1); it is advisable to add an Override annotation. ```suggestion */ @Override ``` -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
