Committed by
Gerrit Code Review
Resolve some NPEs during collecting system metrics
Change-Id: Id018026676948d732f342d634dff6fba630c1414
Showing
2 changed files
with
69 additions
and
5 deletions
| ... | @@ -15,6 +15,7 @@ | ... | @@ -15,6 +15,7 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.cpman.impl; | 16 | package org.onosproject.cpman.impl; |
| 17 | 17 | ||
| 18 | +import com.google.common.collect.ImmutableMap; | ||
| 18 | import com.google.common.collect.ImmutableSet; | 19 | import com.google.common.collect.ImmutableSet; |
| 19 | import com.google.common.collect.Maps; | 20 | import com.google.common.collect.Maps; |
| 20 | import com.google.common.collect.Sets; | 21 | import com.google.common.collect.Sets; |
| ... | @@ -340,6 +341,9 @@ public class ControlPlaneMonitor implements ControlPlaneMonitorService { | ... | @@ -340,6 +341,9 @@ public class ControlPlaneMonitor implements ControlPlaneMonitorService { |
| 340 | } | 341 | } |
| 341 | 342 | ||
| 342 | private Map convertMap(Map<ControlMetricType, Double> map) { | 343 | private Map convertMap(Map<ControlMetricType, Double> map) { |
| 344 | + if (map == null) { | ||
| 345 | + return ImmutableMap.of(); | ||
| 346 | + } | ||
| 343 | Map newMap = Maps.newConcurrentMap(); | 347 | Map newMap = Maps.newConcurrentMap(); |
| 344 | map.forEach((k, v) -> newMap.putIfAbsent(k.toString(), v)); | 348 | map.forEach((k, v) -> newMap.putIfAbsent(k.toString(), v)); |
| 345 | return newMap; | 349 | return newMap; | ... | ... |
| ... | @@ -18,14 +18,18 @@ package org.onosproject.cpman.rest; | ... | @@ -18,14 +18,18 @@ package org.onosproject.cpman.rest; |
| 18 | import com.fasterxml.jackson.databind.JsonNode; | 18 | import com.fasterxml.jackson.databind.JsonNode; |
| 19 | import com.fasterxml.jackson.databind.node.ArrayNode; | 19 | import com.fasterxml.jackson.databind.node.ArrayNode; |
| 20 | import com.fasterxml.jackson.databind.node.ObjectNode; | 20 | import com.fasterxml.jackson.databind.node.ObjectNode; |
| 21 | +import org.apache.commons.lang3.StringUtils; | ||
| 21 | import org.onosproject.cpman.ControlMetric; | 22 | import org.onosproject.cpman.ControlMetric; |
| 22 | import org.onosproject.cpman.ControlMetricType; | 23 | import org.onosproject.cpman.ControlMetricType; |
| 23 | import org.onosproject.cpman.ControlPlaneMonitorService; | 24 | import org.onosproject.cpman.ControlPlaneMonitorService; |
| 25 | +import org.onosproject.cpman.ControlResource; | ||
| 24 | import org.onosproject.cpman.MetricValue; | 26 | import org.onosproject.cpman.MetricValue; |
| 25 | import org.onosproject.cpman.SystemInfo; | 27 | import org.onosproject.cpman.SystemInfo; |
| 26 | import org.onosproject.cpman.impl.DefaultSystemInfo; | 28 | import org.onosproject.cpman.impl.DefaultSystemInfo; |
| 27 | import org.onosproject.cpman.impl.SystemInfoFactory; | 29 | import org.onosproject.cpman.impl.SystemInfoFactory; |
| 28 | import org.onosproject.rest.AbstractWebResource; | 30 | import org.onosproject.rest.AbstractWebResource; |
| 31 | +import org.slf4j.Logger; | ||
| 32 | +import org.slf4j.LoggerFactory; | ||
| 29 | 33 | ||
| 30 | import javax.ws.rs.Consumes; | 34 | import javax.ws.rs.Consumes; |
| 31 | import javax.ws.rs.POST; | 35 | import javax.ws.rs.POST; |
| ... | @@ -35,7 +39,10 @@ import javax.ws.rs.core.MediaType; | ... | @@ -35,7 +39,10 @@ import javax.ws.rs.core.MediaType; |
| 35 | import javax.ws.rs.core.Response; | 39 | import javax.ws.rs.core.Response; |
| 36 | import java.io.IOException; | 40 | import java.io.IOException; |
| 37 | import java.io.InputStream; | 41 | import java.io.InputStream; |
| 42 | +import java.util.Iterator; | ||
| 38 | import java.util.Optional; | 43 | import java.util.Optional; |
| 44 | +import java.util.Set; | ||
| 45 | +import java.util.stream.Collectors; | ||
| 39 | 46 | ||
| 40 | import static org.onlab.util.Tools.nullIsIllegal; | 47 | import static org.onlab.util.Tools.nullIsIllegal; |
| 41 | 48 | ||
| ... | @@ -45,11 +52,21 @@ import static org.onlab.util.Tools.nullIsIllegal; | ... | @@ -45,11 +52,21 @@ import static org.onlab.util.Tools.nullIsIllegal; |
| 45 | @Path("collector") | 52 | @Path("collector") |
| 46 | public class SystemMetricsCollectorWebResource extends AbstractWebResource { | 53 | public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 47 | 54 | ||
| 55 | + private final Logger log = LoggerFactory.getLogger(getClass()); | ||
| 48 | private final ControlPlaneMonitorService service = get(ControlPlaneMonitorService.class); | 56 | private final ControlPlaneMonitorService service = get(ControlPlaneMonitorService.class); |
| 49 | private static final int UPDATE_INTERVAL_IN_MINUTE = 1; | 57 | private static final int UPDATE_INTERVAL_IN_MINUTE = 1; |
| 50 | private static final String INVALID_SYSTEM_SPECS = "Invalid system specifications"; | 58 | private static final String INVALID_SYSTEM_SPECS = "Invalid system specifications"; |
| 51 | private static final String INVALID_RESOURCE_NAME = "Invalid resource name"; | 59 | private static final String INVALID_RESOURCE_NAME = "Invalid resource name"; |
| 52 | private static final String INVALID_REQUEST = "Invalid request"; | 60 | private static final String INVALID_REQUEST = "Invalid request"; |
| 61 | + private static final int PERCENT_CONSTANT = 100; | ||
| 62 | + | ||
| 63 | + private static final Set<String> MEMORY_FIELD_SET = ControlResource.MEMORY_METRICS | ||
| 64 | + .stream().map(type -> toCamelCase(type.toString(), true)) | ||
| 65 | + .collect(Collectors.toSet()); | ||
| 66 | + | ||
| 67 | + private static final Set<String> CPU_FIELD_SET = ControlResource.CPU_METRICS | ||
| 68 | + .stream().map(type -> toCamelCase(type.toString(), true)) | ||
| 69 | + .collect(Collectors.toSet()); | ||
| 53 | 70 | ||
| 54 | /** | 71 | /** |
| 55 | * Collects CPU metrics. | 72 | * Collects CPU metrics. |
| ... | @@ -67,7 +84,13 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { | ... | @@ -67,7 +84,13 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 67 | ControlMetric cm; | 84 | ControlMetric cm; |
| 68 | try { | 85 | try { |
| 69 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); | 86 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); |
| 70 | - long cpuLoad = nullIsIllegal(jsonTree.get("cpuLoad").asLong(), INVALID_REQUEST); | 87 | + |
| 88 | + if (jsonTree == null || !checkFields(jsonTree, CPU_FIELD_SET)) { | ||
| 89 | + ok(root).build(); | ||
| 90 | + } | ||
| 91 | + | ||
| 92 | + long cpuLoad = nullIsIllegal((long) (jsonTree.get("cpuLoad").asDouble() | ||
| 93 | + * PERCENT_CONSTANT), INVALID_REQUEST); | ||
| 71 | long totalCpuTime = nullIsIllegal(jsonTree.get("totalCpuTime").asLong(), INVALID_REQUEST); | 94 | long totalCpuTime = nullIsIllegal(jsonTree.get("totalCpuTime").asLong(), INVALID_REQUEST); |
| 72 | long sysCpuTime = nullIsIllegal(jsonTree.get("sysCpuTime").asLong(), INVALID_REQUEST); | 95 | long sysCpuTime = nullIsIllegal(jsonTree.get("sysCpuTime").asLong(), INVALID_REQUEST); |
| 73 | long userCpuTime = nullIsIllegal(jsonTree.get("userCpuTime").asLong(), INVALID_REQUEST); | 96 | long userCpuTime = nullIsIllegal(jsonTree.get("userCpuTime").asLong(), INVALID_REQUEST); |
| ... | @@ -95,6 +118,8 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { | ... | @@ -95,6 +118,8 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 95 | 118 | ||
| 96 | } catch (IOException e) { | 119 | } catch (IOException e) { |
| 97 | throw new IllegalArgumentException(e.getMessage()); | 120 | throw new IllegalArgumentException(e.getMessage()); |
| 121 | + } catch (IllegalArgumentException iae) { | ||
| 122 | + log.error("[CPU] Illegal arguments in JSON input, msg: {}", iae.getMessage()); | ||
| 98 | } | 123 | } |
| 99 | return ok(root).build(); | 124 | return ok(root).build(); |
| 100 | } | 125 | } |
| ... | @@ -115,10 +140,16 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { | ... | @@ -115,10 +140,16 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 115 | ControlMetric cm; | 140 | ControlMetric cm; |
| 116 | try { | 141 | try { |
| 117 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); | 142 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); |
| 118 | - long memUsedRatio = nullIsIllegal(jsonTree.get("memoryUsedRatio").asLong(), INVALID_REQUEST); | 143 | + |
| 119 | - long memFreeRatio = nullIsIllegal(jsonTree.get("memoryFreeRatio").asLong(), INVALID_REQUEST); | 144 | + if (jsonTree == null || !checkFields(jsonTree, MEMORY_FIELD_SET)) { |
| 145 | + ok(root).build(); | ||
| 146 | + } | ||
| 147 | + | ||
| 120 | long memUsed = nullIsIllegal(jsonTree.get("memoryUsed").asLong(), INVALID_REQUEST); | 148 | long memUsed = nullIsIllegal(jsonTree.get("memoryUsed").asLong(), INVALID_REQUEST); |
| 121 | long memFree = nullIsIllegal(jsonTree.get("memoryFree").asLong(), INVALID_REQUEST); | 149 | long memFree = nullIsIllegal(jsonTree.get("memoryFree").asLong(), INVALID_REQUEST); |
| 150 | + long memTotal = memUsed + memFree; | ||
| 151 | + long memUsedRatio = memTotal == 0L ? 0L : (memUsed * PERCENT_CONSTANT) / memTotal; | ||
| 152 | + long memFreeRatio = memTotal == 0L ? 0L : (memFree * PERCENT_CONSTANT) / memTotal; | ||
| 122 | 153 | ||
| 123 | cm = new ControlMetric(ControlMetricType.MEMORY_USED_RATIO, | 154 | cm = new ControlMetric(ControlMetricType.MEMORY_USED_RATIO, |
| 124 | new MetricValue.Builder().load(memUsedRatio).add()); | 155 | new MetricValue.Builder().load(memUsedRatio).add()); |
| ... | @@ -138,6 +169,8 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { | ... | @@ -138,6 +169,8 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 138 | 169 | ||
| 139 | } catch (IOException e) { | 170 | } catch (IOException e) { |
| 140 | throw new IllegalArgumentException(e.getMessage()); | 171 | throw new IllegalArgumentException(e.getMessage()); |
| 172 | + } catch (IllegalArgumentException iae) { | ||
| 173 | + log.error("[RAM] Illegal arguments in JSON input, msg: {}", iae.getMessage()); | ||
| 141 | } | 174 | } |
| 142 | return ok(root).build(); | 175 | return ok(root).build(); |
| 143 | } | 176 | } |
| ... | @@ -158,7 +191,9 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { | ... | @@ -158,7 +191,9 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 158 | ControlMetric cm; | 191 | ControlMetric cm; |
| 159 | try { | 192 | try { |
| 160 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); | 193 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); |
| 161 | - ArrayNode diskRes = (ArrayNode) jsonTree.get("disks"); | 194 | + ArrayNode diskRes = |
| 195 | + jsonTree.get("disks") == null ? mapper().createArrayNode() : (ArrayNode) jsonTree.get("disks"); | ||
| 196 | + | ||
| 162 | for (JsonNode node : diskRes) { | 197 | for (JsonNode node : diskRes) { |
| 163 | JsonNode resourceName = node.get("resourceName"); | 198 | JsonNode resourceName = node.get("resourceName"); |
| 164 | nullIsIllegal(resourceName, INVALID_RESOURCE_NAME); | 199 | nullIsIllegal(resourceName, INVALID_RESOURCE_NAME); |
| ... | @@ -176,6 +211,8 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { | ... | @@ -176,6 +211,8 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 176 | } | 211 | } |
| 177 | } catch (IOException e) { | 212 | } catch (IOException e) { |
| 178 | throw new IllegalArgumentException(e.getMessage()); | 213 | throw new IllegalArgumentException(e.getMessage()); |
| 214 | + } catch (IllegalArgumentException iae) { | ||
| 215 | + log.error("[DISK] Illegal arguments in JSON input, msg: {}", iae.getMessage()); | ||
| 179 | } | 216 | } |
| 180 | return ok(root).build(); | 217 | return ok(root).build(); |
| 181 | } | 218 | } |
| ... | @@ -196,7 +233,10 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { | ... | @@ -196,7 +233,10 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 196 | ControlMetric cm; | 233 | ControlMetric cm; |
| 197 | try { | 234 | try { |
| 198 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); | 235 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); |
| 199 | - ArrayNode networkRes = (ArrayNode) jsonTree.get("networks"); | 236 | + |
| 237 | + ArrayNode networkRes = jsonTree.get("networks") == null | ||
| 238 | + ? mapper().createArrayNode() : (ArrayNode) jsonTree.get("networks"); | ||
| 239 | + | ||
| 200 | for (JsonNode node : networkRes) { | 240 | for (JsonNode node : networkRes) { |
| 201 | JsonNode resourceName = node.get("resourceName"); | 241 | JsonNode resourceName = node.get("resourceName"); |
| 202 | nullIsIllegal(resourceName, INVALID_RESOURCE_NAME); | 242 | nullIsIllegal(resourceName, INVALID_RESOURCE_NAME); |
| ... | @@ -271,4 +311,24 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { | ... | @@ -271,4 +311,24 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { |
| 271 | } | 311 | } |
| 272 | return ok(root).build(); | 312 | return ok(root).build(); |
| 273 | } | 313 | } |
| 314 | + | ||
| 315 | + private boolean checkFields(ObjectNode node, Set<String> original) { | ||
| 316 | + Iterator<String> fieldNames = node.fieldNames(); | ||
| 317 | + while (fieldNames.hasNext()) { | ||
| 318 | + String fieldName = fieldNames.next(); | ||
| 319 | + if (!original.contains(fieldName) || node.get(fieldName) == null) { | ||
| 320 | + log.warn("Illegal field name: {}", fieldName); | ||
| 321 | + return false; | ||
| 322 | + } | ||
| 323 | + } | ||
| 324 | + return true; | ||
| 325 | + } | ||
| 326 | + | ||
| 327 | + private static String toCamelCase(String value, boolean startWithLowerCase) { | ||
| 328 | + String[] strings = StringUtils.split(value.toLowerCase(), "_"); | ||
| 329 | + for (int i = startWithLowerCase ? 1 : 0; i < strings.length; i++) { | ||
| 330 | + strings[i] = StringUtils.capitalize(strings[i]); | ||
| 331 | + } | ||
| 332 | + return StringUtils.join(strings); | ||
| 333 | + } | ||
| 274 | } | 334 | } | ... | ... |
-
Please register or login to post a comment