You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ke...@apache.org on 2021/04/12 13:41:40 UTC
[skywalking] branch bugfix/envoy-als updated (5e1f018 -> d66785a)
This is an automated email from the ASF dual-hosted git repository.
kezhenxu94 pushed a change to branch bugfix/envoy-als
in repository https://gitbox.apache.org/repos/asf/skywalking.git.
discard 5e1f018 add some defensive codes of new protobuffers' semantics
new d66785a add some defensive codes of new protobuffers' semantics
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (5e1f018)
\
N -- N -- N refs/heads/bugfix/envoy-als (d66785a)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGES.md | 1 +
1 file changed, 1 insertion(+)
[skywalking] 01/01: add some defensive codes of new protobuffers'
semantics
Posted by ke...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kezhenxu94 pushed a commit to branch bugfix/envoy-als
in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit d66785a679e5a1f8ecf7f85598a7378c36e4eae1
Author: kezhenxu94 <ke...@apache.org>
AuthorDate: Mon Apr 12 21:38:22 2021 +0800
add some defensive codes of new protobuffers' semantics
---
CHANGES.md | 1 +
.../envoy/als/LogEntry2MetricsAdapter.java | 15 ++--------
.../server/receiver/envoy/als/k8s/Addresses.java | 34 ++++++++++++++++++++++
.../receiver/envoy/als/k8s/K8SServiceRegistry.java | 2 +-
.../als/k8s/K8sALSServiceMeshHTTPAnalysis.java | 20 +++++++++----
5 files changed, 53 insertions(+), 19 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 2fa2e84..8f818b2 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -12,6 +12,7 @@ Release Notes.
#### OAP-Backend
+* BugFix: filter invalid Envoy access logs whose socket address is empty.
#### UI
diff --git a/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java b/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java
index accdf23..a37888e 100644
--- a/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java
+++ b/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/LogEntry2MetricsAdapter.java
@@ -20,11 +20,9 @@ package org.apache.skywalking.oap.server.receiver.envoy.als;
import com.google.protobuf.Duration;
import com.google.protobuf.Timestamp;
-import com.google.protobuf.UInt32Value;
import io.envoyproxy.envoy.data.accesslog.v3.AccessLogCommon;
import io.envoyproxy.envoy.data.accesslog.v3.HTTPAccessLogEntry;
import io.envoyproxy.envoy.data.accesslog.v3.HTTPRequestProperties;
-import io.envoyproxy.envoy.data.accesslog.v3.HTTPResponseProperties;
import io.envoyproxy.envoy.data.accesslog.v3.ResponseFlags;
import io.envoyproxy.envoy.data.accesslog.v3.TLSProperties;
import java.time.Instant;
@@ -36,7 +34,6 @@ import org.apache.skywalking.apm.network.servicemesh.v3.Protocol;
import org.apache.skywalking.apm.network.servicemesh.v3.ServiceMeshMetric;
import static com.google.common.base.Strings.isNullOrEmpty;
-import static java.util.Optional.ofNullable;
/**
* Adapt {@link HTTPAccessLogEntry} objects to {@link ServiceMeshMetric} builders.
@@ -97,9 +94,7 @@ public class LogEntry2MetricsAdapter {
protected ServiceMeshMetric.Builder adaptCommonPart() {
final AccessLogCommon properties = entry.getCommonProperties();
final String endpoint = endpoint();
- final int responseCode = ofNullable(entry.getResponse()).map(HTTPResponseProperties::getResponseCode)
- .map(UInt32Value::getValue)
- .orElse(200);
+ final int responseCode = entry.getResponse().getResponseCode().getValue();
final boolean status = responseCode >= 200 && responseCode < 400;
final Protocol protocol = requestProtocol(entry.getRequest());
final String tlsMode = parseTLS(properties.getTlsProperties());
@@ -162,15 +157,11 @@ public class LogEntry2MetricsAdapter {
if (properties == null) {
return NON_TLS;
}
- TLSProperties.CertificateProperties lp = Optional
- .ofNullable(properties.getLocalCertificateProperties())
- .orElse(TLSProperties.CertificateProperties.newBuilder().build());
+ TLSProperties.CertificateProperties lp = properties.getLocalCertificateProperties();
if (isNullOrEmpty(lp.getSubject()) && !hasSAN(lp.getSubjectAltNameList())) {
return NON_TLS;
}
- TLSProperties.CertificateProperties pp = Optional
- .ofNullable(properties.getPeerCertificateProperties())
- .orElse(TLSProperties.CertificateProperties.newBuilder().build());
+ TLSProperties.CertificateProperties pp = properties.getPeerCertificateProperties();
if (isNullOrEmpty(pp.getSubject()) && !hasSAN(pp.getSubjectAltNameList())) {
return TLS;
}
diff --git a/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/Addresses.java b/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/Addresses.java
new file mode 100644
index 0000000..d17cec5
--- /dev/null
+++ b/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/Addresses.java
@@ -0,0 +1,34 @@
+/*
+ * 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.receiver.envoy.als.k8s;
+
+import io.envoyproxy.envoy.config.core.v3.Address;
+import lombok.experimental.UtilityClass;
+
+import static java.util.Objects.nonNull;
+import static org.apache.skywalking.apm.util.StringUtil.isNotBlank;
+
+@UtilityClass
+public class Addresses {
+ public boolean isValid(final Address address) {
+ return nonNull(address)
+ && address.hasSocketAddress()
+ && isNotBlank(address.getSocketAddress().getAddress());
+ }
+}
diff --git a/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/K8SServiceRegistry.java b/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/K8SServiceRegistry.java
index 158cd20..a1cd1a6 100644
--- a/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/K8SServiceRegistry.java
+++ b/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/K8SServiceRegistry.java
@@ -267,7 +267,7 @@ public class K8SServiceRegistry {
protected ServiceMetaInfo findService(final String ip) {
final ServiceMetaInfo service = ipServiceMetaInfoMap.get(ip);
if (isNull(service)) {
- log.debug("Unknown ip {}, ip -> service is null", ip);
+ log.warn("Unknown ip {}, ip -> service is null", ip);
return ServiceMetaInfo.UNKNOWN;
}
return service;
diff --git a/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/K8sALSServiceMeshHTTPAnalysis.java b/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/K8sALSServiceMeshHTTPAnalysis.java
index 2b99075..e761ff7 100644
--- a/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/K8sALSServiceMeshHTTPAnalysis.java
+++ b/oap-server/server-receiver-plugin/envoy-metrics-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/envoy/als/k8s/K8sALSServiceMeshHTTPAnalysis.java
@@ -35,8 +35,10 @@ import org.apache.skywalking.oap.server.receiver.envoy.als.AbstractALSAnalyzer;
import org.apache.skywalking.oap.server.receiver.envoy.als.Role;
import org.apache.skywalking.oap.server.receiver.envoy.als.ServiceMetaInfo;
+import static org.apache.skywalking.apm.util.StringUtil.isBlank;
import static org.apache.skywalking.oap.server.library.util.CollectionUtils.isNotEmpty;
import static org.apache.skywalking.oap.server.receiver.envoy.als.LogEntry2MetricsAdapter.NON_TLS;
+import static org.apache.skywalking.oap.server.receiver.envoy.als.k8s.Addresses.isValid;
/**
* Analysis log based on ingress and mesh scenarios.
@@ -81,12 +83,12 @@ public class K8sALSServiceMeshHTTPAnalysis extends AbstractALSAnalyzer {
}
protected List<ServiceMeshMetric.Builder> analyzeSideCar(final HTTPAccessLogEntry entry) {
- final AccessLogCommon properties = entry.getCommonProperties();
- if (properties == null) {
+ if (!entry.hasCommonProperties()) {
return Collections.emptyList();
}
+ final AccessLogCommon properties = entry.getCommonProperties();
final String cluster = properties.getUpstreamCluster();
- if (cluster == null) {
+ if (isBlank(cluster)) {
return Collections.emptyList();
}
@@ -98,6 +100,9 @@ public class K8sALSServiceMeshHTTPAnalysis extends AbstractALSAnalyzer {
: properties.getDownstreamRemoteAddress();
final ServiceMetaInfo downstreamService = find(downstreamRemoteAddress.getSocketAddress().getAddress());
final Address downstreamLocalAddress = properties.getDownstreamLocalAddress();
+ if (!isValid(downstreamRemoteAddress) || !isValid(downstreamLocalAddress)) {
+ return Collections.emptyList();
+ }
final ServiceMetaInfo localService = find(downstreamLocalAddress.getSocketAddress().getAddress());
if (cluster.startsWith("inbound|")) {
@@ -119,6 +124,9 @@ public class K8sALSServiceMeshHTTPAnalysis extends AbstractALSAnalyzer {
} else if (cluster.startsWith("outbound|")) {
// sidecar(client side) -> sidecar
final Address upstreamRemoteAddress = properties.getUpstreamRemoteAddress();
+ if (!isValid(upstreamRemoteAddress)) {
+ return sources;
+ }
final ServiceMetaInfo destService = find(upstreamRemoteAddress.getSocketAddress().getAddress());
final ServiceMeshMetric.Builder metric = newAdapter(entry, downstreamService, destService).adaptToUpstreamMetrics();
@@ -131,15 +139,15 @@ public class K8sALSServiceMeshHTTPAnalysis extends AbstractALSAnalyzer {
}
protected List<ServiceMeshMetric.Builder> analyzeProxy(final HTTPAccessLogEntry entry) {
- final AccessLogCommon properties = entry.getCommonProperties();
- if (properties == null) {
+ if (!entry.hasCommonProperties()) {
return Collections.emptyList();
}
+ final AccessLogCommon properties = entry.getCommonProperties();
final Address downstreamLocalAddress = properties.getDownstreamLocalAddress();
final Address downstreamRemoteAddress = properties.hasDownstreamDirectRemoteAddress() ?
properties.getDownstreamDirectRemoteAddress() : properties.getDownstreamRemoteAddress();
final Address upstreamRemoteAddress = properties.getUpstreamRemoteAddress();
- if (downstreamLocalAddress == null || downstreamRemoteAddress == null || upstreamRemoteAddress == null) {
+ if (!isValid(downstreamLocalAddress) || !isValid(downstreamRemoteAddress) || !isValid(upstreamRemoteAddress)) {
return Collections.emptyList();
}