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 2020/12/15 07:47:06 UTC
[GitHub] [skywalking] muse-dev[bot] commented on a change in pull request #5872: Support Envoy {AccessLog,Metrics}Service API V3
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:
users@infra.apache.org