muse-dev[bot] commented on a change in pull request #5872:
URL: https://github.com/apache/skywalking/pull/5872#discussion_r543116934
##########
File path:
oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/MetricServiceGRPCHandler.java
##########
@@ -62,125 +63,110 @@ public MetricServiceGRPCHandler(ModuleManager
moduleManager) {
);
}
- @Override
- public StreamObserver<StreamMetricsMessage>
streamMetrics(StreamObserver<StreamMetricsResponse> responseObserver) {
- return new StreamObserver<StreamMetricsMessage>() {
- private volatile boolean isFirst = true;
- private String serviceName = null;
- private String serviceInstanceName = null;
-
- @Override
- public void onNext(StreamMetricsMessage message) {
- if (log.isDebugEnabled()) {
- log.debug("Received msg {}", message);
- }
+ private volatile boolean isFirst = true;
- if (isFirst) {
- isFirst = false;
- StreamMetricsMessage.Identifier identifier =
message.getIdentifier();
- Node node = identifier.getNode();
- if (node != null) {
- String nodeId = node.getId();
- if (!StringUtil.isEmpty(nodeId)) {
- serviceInstanceName = nodeId;
- }
- String cluster = node.getCluster();
- if (!StringUtil.isEmpty(cluster)) {
- serviceName = cluster;
- if (serviceInstanceName == null) {
- serviceInstanceName = serviceName;
- }
- }
- }
+ private String serviceName = null;
- if (serviceName == null) {
- serviceName = serviceInstanceName;
- }
- }
+ private String serviceInstanceName = null;
- if (log.isDebugEnabled()) {
- log.debug(
- "Envoy metrics reported from service[{}], service
instance[{}]", serviceName,
- serviceInstanceName
- );
+ public void handle(Message message) {
+ if (log.isDebugEnabled()) {
+ log.debug("Received msg {}", message);
+ }
+
+ if (isFirst) {
+ isFirst = false;
+ final String nodeId = findField(message, "identifier.node.id",
null);
+ if (!StringUtil.isEmpty(nodeId)) {
+ serviceInstanceName = nodeId;
+ }
+ final String cluster = findField(message,
"identifier.node.cluster", null);
+ if (!StringUtil.isEmpty(cluster)) {
+ serviceName = cluster;
+ if (serviceInstanceName == null) {
+ serviceInstanceName = serviceName;
}
+ }
- if (StringUtil.isNotEmpty(serviceName) &&
StringUtil.isNotEmpty(serviceInstanceName)) {
- List<Metrics.MetricFamily> list =
message.getEnvoyMetricsList();
- boolean needHeartbeatUpdate = true;
- for (int i = 0; i < list.size(); i++) {
- counter.inc();
-
- final String serviceId =
IDManager.ServiceID.buildId(serviceName, NodeType.Normal);
- final String serviceInstanceId =
IDManager.ServiceInstanceID.buildId(
- serviceId, serviceInstanceName);
-
- HistogramMetrics.Timer timer = histogram.createTimer();
- try {
- Metrics.MetricFamily metricFamily = list.get(i);
- double value = 0;
- long timestamp = 0;
- switch (metricFamily.getType()) {
- case GAUGE:
- for (Metrics.Metric metrics :
metricFamily.getMetricList()) {
- timestamp = metrics.getTimestampMs();
- value = metrics.getGauge().getValue();
-
- if (timestamp > 1000000000000000000L) {
- /**
- * Several versions of envoy in
istio.deps send timestamp in nanoseconds,
- * instead of
milliseconds(protocol says).
- *
- * Sadly, but have to fix it
forcedly.
- *
- * An example of timestamp is
'1552303033488741055', clearly it is not in milliseconds.
- *
- * This should be removed in the
future.
- */
- timestamp /= 1_000_000;
- }
-
- EnvoyInstanceMetric metricSource = new
EnvoyInstanceMetric();
- metricSource.setServiceId(serviceId);
-
metricSource.setServiceName(serviceName);
- metricSource.setId(serviceInstanceId);
-
metricSource.setName(serviceInstanceName);
-
metricSource.setMetricName(metricFamily.getName());
- metricSource.setValue(value);
-
metricSource.setTimeBucket(TimeBucket.getMinuteTimeBucket(timestamp));
- sourceReceiver.receive(metricSource);
- }
- break;
- default:
- continue;
- }
- if (needHeartbeatUpdate) {
- // Send heartbeat
- ServiceInstanceUpdate serviceInstanceUpdate =
new ServiceInstanceUpdate();
-
serviceInstanceUpdate.setName(serviceInstanceName);
- serviceInstanceUpdate.setServiceId(serviceId);
-
serviceInstanceUpdate.setTimeBucket(TimeBucket.getMinuteTimeBucket(timestamp));
- sourceReceiver.receive(serviceInstanceUpdate);
- needHeartbeatUpdate = false;
+ if (serviceName == null) {
+ serviceName = serviceInstanceName;
+ }
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug(
+ "Envoy metrics reported from service[{}], service
instance[{}]", serviceName,
+ serviceInstanceName
+ );
+ }
+
+ if (StringUtil.isNotEmpty(serviceName) &&
StringUtil.isNotEmpty(serviceInstanceName)) {
+ List<Metrics.MetricFamily> list = findField(message,
"envoy_metrics", Collections.emptyList());
+ boolean needHeartbeatUpdate = true;
+ for (Metrics.MetricFamily family : list) {
+ counter.inc();
+
+ final String serviceId =
IDManager.ServiceID.buildId(serviceName, NodeType.Normal);
+ final String serviceInstanceId =
IDManager.ServiceInstanceID.buildId(
+ serviceId, serviceInstanceName);
+
+ HistogramMetrics.Timer timer = histogram.createTimer();
+ try {
+ double value;
+ long timestamp = 0;
+ switch (family.getType()) {
Review comment:
*NULL_DEREFERENCE:* object returned by `family.getType()` could be null
and is dereferenced at line 117.
##########
File path:
oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java
##########
@@ -65,9 +61,8 @@
* @return the {@link ServiceMeshMetric.Builder} adapted from the given
entry.
*/
public ServiceMeshMetric.Builder adaptToDownstreamMetrics() {
- final AccessLogCommon properties = entry.getCommonProperties();
- final long startTime = formatAsLong(properties.getStartTime());
- final long duration =
formatAsLong(properties.getTimeToLastDownstreamTxByte());
+ final long startTime = formatAsLong(findField(logEntry,
"common_properties.start_time", (Timestamp) null));
Review comment:
*NULL_DEREFERENCE:* object returned by
`findField(LogEntry2MetricsAdapter.logEntry,"common_properties.start_time",null)`
could be null and is dereferenced by call to `formatAsLong(...)` at line 64.
##########
File path:
oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java
##########
@@ -65,9 +61,8 @@
* @return the {@link ServiceMeshMetric.Builder} adapted from the given
entry.
*/
public ServiceMeshMetric.Builder adaptToDownstreamMetrics() {
- final AccessLogCommon properties = entry.getCommonProperties();
- final long startTime = formatAsLong(properties.getStartTime());
- final long duration =
formatAsLong(properties.getTimeToLastDownstreamTxByte());
+ final long startTime = formatAsLong(findField(logEntry,
"common_properties.start_time", (Timestamp) null));
+ final long duration = formatAsLong(findField(logEntry,
"common_properties.time_to_last_rx_byte", (Duration) null));
Review comment:
*NULL_DEREFERENCE:* object returned by
`findField(LogEntry2MetricsAdapter.logEntry,"common_properties.time_to_last_rx_byte",null)`
could be null and is dereferenced by call to `formatAsLong(...)` at line 65.
##########
File path:
oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java
##########
@@ -82,10 +77,9 @@
* @return the {@link ServiceMeshMetric.Builder} adapted from the given
entry.
*/
public ServiceMeshMetric.Builder adaptToUpstreamMetrics() {
- final AccessLogCommon properties = entry.getCommonProperties();
- final long startTime = formatAsLong(properties.getStartTime());
- final long outboundStartTime = startTime +
formatAsLong(properties.getTimeToFirstUpstreamTxByte());
- final long outboundEndTime = startTime +
formatAsLong(properties.getTimeToLastUpstreamRxByte());
+ final long startTime = formatAsLong(findField(logEntry,
"common_properties.start_time", (Timestamp) null));
Review comment:
*NULL_DEREFERENCE:* object returned by
`findField(LogEntry2MetricsAdapter.logEntry,"common_properties.start_time",null)`
could be null and is dereferenced by call to `formatAsLong(...)` at line 80.
##########
File path:
oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java
##########
@@ -82,10 +77,9 @@
* @return the {@link ServiceMeshMetric.Builder} adapted from the given
entry.
*/
public ServiceMeshMetric.Builder adaptToUpstreamMetrics() {
- final AccessLogCommon properties = entry.getCommonProperties();
- final long startTime = formatAsLong(properties.getStartTime());
- final long outboundStartTime = startTime +
formatAsLong(properties.getTimeToFirstUpstreamTxByte());
- final long outboundEndTime = startTime +
formatAsLong(properties.getTimeToLastUpstreamRxByte());
+ final long startTime = formatAsLong(findField(logEntry,
"common_properties.start_time", (Timestamp) null));
+ final long outboundStartTime = startTime +
formatAsLong(findField(logEntry,
"common_properties.time_to_first_upstream_tx_byte", (Duration) null));
Review comment:
*NULL_DEREFERENCE:* object returned by
`findField(LogEntry2MetricsAdapter.logEntry,"common_properties.time_to_first_upstream_tx_byte",null)`
could be null and is dereferenced by call to `formatAsLong(...)` at line 81.
##########
File path:
oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java
##########
@@ -82,10 +77,9 @@
* @return the {@link ServiceMeshMetric.Builder} adapted from the given
entry.
*/
public ServiceMeshMetric.Builder adaptToUpstreamMetrics() {
- final AccessLogCommon properties = entry.getCommonProperties();
- final long startTime = formatAsLong(properties.getStartTime());
- final long outboundStartTime = startTime +
formatAsLong(properties.getTimeToFirstUpstreamTxByte());
- final long outboundEndTime = startTime +
formatAsLong(properties.getTimeToLastUpstreamRxByte());
+ final long startTime = formatAsLong(findField(logEntry,
"common_properties.start_time", (Timestamp) null));
+ final long outboundStartTime = startTime +
formatAsLong(findField(logEntry,
"common_properties.time_to_first_upstream_tx_byte", (Duration) null));
+ final long outboundEndTime = startTime +
formatAsLong(findField(logEntry,
"common_properties.time_to_last_upstream_tx_byte", (Duration) null));
Review comment:
*NULL_DEREFERENCE:* object returned by
`findField(LogEntry2MetricsAdapter.logEntry,"common_properties.time_to_last_upstream_tx_byte",null)`
could be null and is dereferenced by call to `formatAsLong(...)` at line 82.
----------------------------------------------------------------
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:
[email protected]
