From 4238992baf80a1f8d476f04c8313078d6203c5bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=90=B4=E6=99=9F=20Wu=20Sheng?= Date: Mon, 2 Nov 2020 14:29:14 +0800 Subject: [PATCH] Polish the context and optimize the tag analyze (#5767) * Polish the context and optimize the tag analyze * Fix bug of new context codes. --- CHANGES.md | 2 ++ .../agent/core/context/ContextCarrier.java | 18 +++++----- .../core/context/CorrelationContext.java | 18 +++++++--- .../agent/core/context/ExtensionContext.java | 10 +++++- .../agent/core/context/TracingContext.java | 11 +++--- .../listener/SegmentAnalysisListener.java | 9 ++++- .../core/analysis/manual/segment/SpanTag.java | 16 ++++----- .../analysis/manual/segment/SpanTagTest.java | 35 +++++++++++++++++++ 8 files changed, 90 insertions(+), 29 deletions(-) create mode 100644 oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/manual/segment/SpanTagTest.java diff --git a/CHANGES.md b/CHANGES.md index 9df5062658..8a3bbac3a6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,7 @@ Release Notes. * Make HttpClient 3.x, 4.x, and HttpAsyncClient 3.x plugins to support collecting HTTP parameters. * Make the Feign plugin to support Java 14 * Make the okhttp3 plugin to support Java 14 +* Polish tracing context related codes. #### OAP-Backend * Add the `@SuperDataset` annotation for BrowserErrorLog. @@ -20,6 +21,7 @@ Release Notes. * Support keeping collecting the slowly segments in the sampling mechanism. * Support choose files to active the meter analyzer. * Improve Kubernetes service registry for ALS analysis. +* Improve the queryable tags generation. Remove the duplicated tags to reduce the storage payload. #### UI diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ContextCarrier.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ContextCarrier.java index 3c185cb73d..65fa3b8c18 100644 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ContextCarrier.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ContextCarrier.java @@ -24,7 +24,6 @@ import lombok.Getter; import lombok.Setter; import org.apache.skywalking.apm.agent.core.base64.Base64; import org.apache.skywalking.apm.agent.core.conf.Constants; -import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; import org.apache.skywalking.apm.util.StringUtil; /** @@ -90,18 +89,21 @@ public class ContextCarrier implements Serializable { } /** - * Extract the extension context to the given span + * Extract the extension context to tracing context */ - void extractExtensionTo(AbstractSpan span) { - this.extensionContext.handle(span); - + void extractExtensionTo(TracingContext tracingContext) { + tracingContext.getExtensionContext().extract(this); + // The extension context could have field not to propagate further, so, must use the this.* to process. + this.extensionContext.handle(tracingContext.activeSpan()); } /** - * Extract the correlation context to the given span + * Extract the correlation context to tracing context */ - void extractCorrelationTo(AbstractSpan span) { - this.correlationContext.handle(span); + void extractCorrelationTo(TracingContext tracingContext) { + tracingContext.getCorrelationContext().extract(this); + // The correlation context could have field not to propagate further, so, must use the this.* to process. + this.correlationContext.handle(tracingContext.activeSpan()); } /** diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/CorrelationContext.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/CorrelationContext.java index 41f4164536..5290df8e1f 100644 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/CorrelationContext.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/CorrelationContext.java @@ -164,6 +164,15 @@ public class CorrelationContext { } } + /** + * Process the active span + * + * 1. Inject the tags with auto-tag flag into the span + */ + void handle(AbstractSpan span) { + AUTO_TAG_KEYS.forEach(key -> this.get(key).ifPresent(val -> span.tag(new StringTag(key), val))); + } + /** * Clone the context data, work for capture to cross-thread. */ @@ -173,14 +182,15 @@ public class CorrelationContext { return context; } + /** + * Continue the correlation context in another thread. + * + * @param snapshot holds the context. + */ void continued(ContextSnapshot snapshot) { this.data.putAll(snapshot.getCorrelationContext().data); } - void handle(AbstractSpan span) { - AUTO_TAG_KEYS.forEach(key -> this.get(key).ifPresent(val -> span.tag(new StringTag(key), val))); - } - @Override public boolean equals(Object o) { if (this == o) diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ExtensionContext.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ExtensionContext.java index 6dfdaf1086..88af06dc82 100644 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ExtensionContext.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ExtensionContext.java @@ -109,7 +109,11 @@ public class ExtensionContext { } /** - * Handle the tracing span. + * Process the active span + * + * 1. Set the `skipAnalysis` flag. + * 2. Tag the {@link Tags#TRANSMISSION_LATENCY} if the context includes `sendingTimestamp`, + * which is set by the client side. */ void handle(AbstractSpan span) { if (this.skipAnalysis) { @@ -131,6 +135,10 @@ public class ExtensionContext { return context; } + /** + * Continue the context in another thread. + * @param snapshot holds the context + */ void continued(ContextSnapshot snapshot) { this.skipAnalysis = snapshot.getExtensionContext().skipAnalysis; this.sendingTimestamp = snapshot.getExtensionContext().sendingTimestamp; diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java index ca444b59f9..9026d035d1 100644 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java @@ -22,6 +22,8 @@ import java.util.LinkedList; import java.util.List; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.concurrent.locks.ReentrantLock; +import lombok.AccessLevel; +import lombok.Getter; import org.apache.skywalking.apm.agent.core.boot.ServiceManager; import org.apache.skywalking.apm.agent.core.conf.Config; import org.apache.skywalking.apm.agent.core.context.ids.DistributedTraceId; @@ -103,8 +105,9 @@ public class TracingContext implements AbstractTracerContext { * profile status */ private final ProfileStatusReference profileStatus; - + @Getter(AccessLevel.PACKAGE) private final CorrelationContext correlationContext; + @Getter(AccessLevel.PACKAGE) private final ExtensionContext extensionContext; /** @@ -187,10 +190,8 @@ public class TracingContext implements AbstractTracerContext { span.ref(ref); } - carrier.extractExtensionTo(span); - carrier.extractCorrelationTo(span); - this.correlationContext.extract(carrier); - this.extensionContext.extract(carrier); + carrier.extractExtensionTo(this); + carrier.extractCorrelationTo(this); } /** diff --git a/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/parser/listener/SegmentAnalysisListener.java b/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/parser/listener/SegmentAnalysisListener.java index 51e4c68202..bb7ee333d1 100644 --- a/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/parser/listener/SegmentAnalysisListener.java +++ b/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/parser/listener/SegmentAnalysisListener.java @@ -19,6 +19,7 @@ package org.apache.skywalking.oap.server.analyzer.provider.trace.parser.listener; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -159,11 +160,17 @@ public class SegmentAnalysisListener implements FirstAnalysisListener, EntryAnal } private void appendSearchableTags(SpanObject span) { + HashSet segmentTags = new HashSet<>(); span.getTagsList().forEach(tag -> { if (searchableTagKeys.contains(tag.getKey())) { - segment.getTags().add(new SpanTag(tag.getKey(), tag.getValue())); + final SpanTag spanTag = new SpanTag(tag.getKey(), tag.getValue()); + if (!segmentTags.contains(spanTag)) { + segmentTags.add(spanTag); + } + } }); + segment.getTags().addAll(segmentTags); } @Override diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/manual/segment/SpanTag.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/manual/segment/SpanTag.java index 4dc2514c1f..976bf57379 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/manual/segment/SpanTag.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/manual/segment/SpanTag.java @@ -21,23 +21,19 @@ package org.apache.skywalking.oap.server.core.analysis.manual.segment; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import lombok.EqualsAndHashCode; import lombok.Getter; +import lombok.RequiredArgsConstructor; import lombok.Setter; import org.apache.skywalking.oap.server.library.util.CollectionUtils; @Getter @Setter +@EqualsAndHashCode +@RequiredArgsConstructor public class SpanTag { - private String key; - private String value; - - public SpanTag() { - } - - public SpanTag(String key, String value) { - this.key = key; - this.value = value; - } + private final String key; + private final String value; @Override public String toString() { diff --git a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/manual/segment/SpanTagTest.java b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/manual/segment/SpanTagTest.java new file mode 100644 index 0000000000..fab8748a50 --- /dev/null +++ b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/manual/segment/SpanTagTest.java @@ -0,0 +1,35 @@ +/* + * 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.analysis.manual.segment; + +import org.junit.Assert; +import org.junit.Test; + +public class SpanTagTest { + @Test + public void testEqual() { + final SpanTag spanTag = new SpanTag("tag1", "value1"); + final SpanTag spanTag1 = new SpanTag("tag1", "value2"); + final SpanTag spanTag2 = new SpanTag("tag2", "value3"); + final SpanTag spanTag3 = new SpanTag("tag1", "value1"); + Assert.assertEquals(spanTag, spanTag3); + Assert.assertNotEquals(spanTag, spanTag1); + Assert.assertNotEquals(spanTag, spanTag2); + } +} -- GitLab