This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch 2.8.x
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/2.8.x by this push:
new 8fb8f0da4 [#4035] fix invocation context loss issue (#4110)
8fb8f0da4 is described below
commit 8fb8f0da487bfde94da14eb91f90a964121e6b71
Author: yanghao <[email protected]>
AuthorDate: Mon Dec 11 09:32:26 2023 +0800
[#4035] fix invocation context loss issue (#4110)
---
.../tracing/zipkin/ZipkinConsumerDelegate.java | 2 +-
.../zipkin/ZipkinConsumerTracingHandler.java | 5 +---
.../tracing/zipkin/ZipkinProviderDelegate.java | 2 +-
.../zipkin/ZipkinProviderTracingHandler.java | 6 +----
.../tracing/zipkin/ZipkinTracingFilter.java | 15 ++++--------
.../tracing/zipkin/ZipkinTracingHandler.java | 28 ++++++++++++++--------
.../tracing/zipkin/ZipkinTracingHandlerTest.java | 14 +++--------
7 files changed, 29 insertions(+), 43 deletions(-)
diff --git
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinConsumerDelegate.java
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinConsumerDelegate.java
index 84505d6d2..149dc8c6e 100644
---
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinConsumerDelegate.java
+++
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinConsumerDelegate.java
@@ -50,7 +50,7 @@ class ZipkinConsumerDelegate implements ZipkinTracingDelegate
{
}
@Override
- public synchronized Span createSpan(Invocation invocation) {
+ public Span createSpan(Invocation invocation) {
return handler.handleSend(requestWrapper.invocation(invocation));
}
diff --git
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinConsumerTracingHandler.java
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinConsumerTracingHandler.java
index 788313413..8d823ffb9 100644
---
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinConsumerTracingHandler.java
+++
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinConsumerTracingHandler.java
@@ -24,10 +24,7 @@ import brave.http.HttpTracing;
public class ZipkinConsumerTracingHandler extends ZipkinTracingHandler {
public ZipkinConsumerTracingHandler() {
- this(new
ZipkinConsumerDelegate(BeanUtils.getContext().getBean(HttpTracing.class)));
+ super(BeanUtils.getContext().getBean(HttpTracing.class));
}
- private ZipkinConsumerTracingHandler(ZipkinTracingDelegate tracingHandler) {
- super(tracingHandler);
- }
}
diff --git
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinProviderDelegate.java
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinProviderDelegate.java
index 48156c57d..0820f0c8c 100644
---
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinProviderDelegate.java
+++
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinProviderDelegate.java
@@ -80,7 +80,7 @@ class ZipkinProviderDelegate implements ZipkinTracingDelegate
{
}
@Override
- public synchronized Span createSpan(Invocation invocation) {
+ public Span createSpan(Invocation invocation) {
return handler.handleReceive(requestWrapper.invocation(invocation));
}
diff --git
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinProviderTracingHandler.java
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinProviderTracingHandler.java
index abec4f330..c66459c0a 100644
---
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinProviderTracingHandler.java
+++
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinProviderTracingHandler.java
@@ -24,10 +24,6 @@ import brave.http.HttpTracing;
public class ZipkinProviderTracingHandler extends ZipkinTracingHandler {
public ZipkinProviderTracingHandler() {
- this(new
ZipkinProviderDelegate(BeanUtils.getContext().getBean(HttpTracing.class)));
- }
-
- private ZipkinProviderTracingHandler(ZipkinProviderDelegate tracingHandler) {
- super(tracingHandler);
+ super(BeanUtils.getContext().getBean(HttpTracing.class));
}
}
diff --git
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingFilter.java
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingFilter.java
index f896004eb..6dd8d5442 100644
---
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingFilter.java
+++
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingFilter.java
@@ -39,9 +39,8 @@ import brave.http.HttpTracing;
public class ZipkinTracingFilter implements Filter {
public static final String NAME = "zipkin";
- private ZipkinConsumerDelegate consumer;
-
- private ZipkinProviderDelegate producer;
+ @Autowired
+ private HttpTracing httpTracing;
@Nonnull
@Override
@@ -49,12 +48,6 @@ public class ZipkinTracingFilter implements Filter {
return NAME;
}
- @Autowired
- public void setHttpTracing(HttpTracing httpTracing) {
- this.consumer = new ZipkinConsumerDelegate(httpTracing);
- this.producer = new ZipkinProviderDelegate(httpTracing);
- }
-
@SuppressWarnings({"try", "unused"})
@Override
public CompletableFuture<Response> onFilter(Invocation invocation,
FilterNode nextNode) {
@@ -69,9 +62,9 @@ public class ZipkinTracingFilter implements Filter {
private ZipkinTracingDelegate collectTracing(Invocation invocation) {
if (PRODUCER.equals(invocation.getInvocationType())) {
- return producer;
+ return new ZipkinProviderDelegate(httpTracing);
}
- return consumer;
+ return new ZipkinConsumerDelegate(httpTracing);
}
}
diff --git
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingHandler.java
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingHandler.java
index 7990d0dfa..5d6593663 100644
---
a/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingHandler.java
+++
b/handlers/handler-tracing-zipkin/src/main/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingHandler.java
@@ -17,6 +17,7 @@
package org.apache.servicecomb.tracing.zipkin;
+import brave.http.HttpTracing;
import org.apache.servicecomb.core.Handler;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.swagger.invocation.AsyncResponse;
@@ -25,31 +26,30 @@ import org.slf4j.LoggerFactory;
import brave.Span;
import brave.Tracer.SpanInScope;
-import brave.Tracing;
+
+import static
org.apache.servicecomb.swagger.invocation.InvocationType.PRODUCER;
class ZipkinTracingHandler implements Handler {
private static final Logger LOGGER =
LoggerFactory.getLogger(ZipkinTracingHandler.class);
- private final Tracing tracer;
-
- private final ZipkinTracingDelegate tracingDelegate;
+ private final HttpTracing httpTracing;
- ZipkinTracingHandler(ZipkinTracingDelegate tracingDelegate) {
- this.tracer = tracingDelegate.tracer();
- this.tracingDelegate = tracingDelegate;
+ ZipkinTracingHandler(HttpTracing httpTracing) {
+ this.httpTracing = httpTracing;
}
@SuppressWarnings({"try", "unused"})
@Override
public void handle(Invocation invocation, AsyncResponse asyncResp) throws
Exception {
+ ZipkinTracingDelegate tracingDelegate = collectTracing(invocation);
Span span = tracingDelegate.createSpan(invocation);
- try (SpanInScope scope = tracer.tracer().withSpanInScope(span)) {
+ try (SpanInScope scope =
tracingDelegate.tracer().tracer().withSpanInScope(span)) {
LOGGER.debug("{}: Generated tracing span for {}",
tracingDelegate.name(),
invocation.getOperationName());
- invocation.next(onResponse(invocation, asyncResp, span));
+ invocation.next(onResponse(invocation, asyncResp, span,
tracingDelegate));
} catch (Exception e) {
LOGGER.debug("{}: Failed invocation on {}",
tracingDelegate.name(),
@@ -61,7 +61,7 @@ class ZipkinTracingHandler implements Handler {
}
}
- private AsyncResponse onResponse(Invocation invocation, AsyncResponse
asyncResp, Span span) {
+ private AsyncResponse onResponse(Invocation invocation, AsyncResponse
asyncResp, Span span, ZipkinTracingDelegate tracingDelegate) {
return response -> {
Throwable error = response.isFailed() ? response.getResult() : null;
tracingDelegate.onResponse(span, response, error);
@@ -74,4 +74,12 @@ class ZipkinTracingHandler implements Handler {
asyncResp.handle(response);
};
}
+
+ private ZipkinTracingDelegate collectTracing(Invocation invocation) {
+ if (PRODUCER.equals(invocation.getInvocationType())) {
+ return new ZipkinProviderDelegate(httpTracing);
+ }
+
+ return new ZipkinConsumerDelegate(httpTracing);
+ }
}
diff --git
a/handlers/handler-tracing-zipkin/src/test/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingHandlerTest.java
b/handlers/handler-tracing-zipkin/src/test/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingHandlerTest.java
index 7773e98a2..f32032060 100644
---
a/handlers/handler-tracing-zipkin/src/test/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingHandlerTest.java
+++
b/handlers/handler-tracing-zipkin/src/test/java/org/apache/servicecomb/tracing/zipkin/ZipkinTracingHandlerTest.java
@@ -22,6 +22,7 @@ import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import brave.http.HttpTracing;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.swagger.invocation.AsyncResponse;
import org.apache.servicecomb.swagger.invocation.Response;
@@ -43,8 +44,6 @@ public class ZipkinTracingHandlerTest {
private final AsyncResponse asyncResponse =
Mockito.mock(AsyncResponse.class);
- private final ZipkinTracingDelegate delegate =
Mockito.mock(ZipkinTracingDelegate.class);
-
private final Tracing tracing = Tracing.newBuilder().build();
private final Span span = Mockito.mock(Span.class);
@@ -55,11 +54,8 @@ public class ZipkinTracingHandlerTest {
@BeforeEach
public void setUp() throws Exception {
- when(delegate.tracer()).thenReturn(tracing);
- when(delegate.createSpan(invocation)).thenReturn(span);
- when(delegate.name()).thenReturn("delegate");
-
- tracingHandler = new ZipkinTracingHandler(delegate);
+ HttpTracing httpTracing = HttpTracing.create(tracing);
+ tracingHandler = new ZipkinTracingHandler(httpTracing);
}
@Test
@@ -73,7 +69,6 @@ public class ZipkinTracingHandlerTest {
argumentCaptor.getValue().handle(response);
verify(asyncResponse).handle(response);
- verify(delegate).onResponse(span, response, null);
}
@Test
@@ -90,7 +85,6 @@ public class ZipkinTracingHandlerTest {
argumentCaptor.getValue().handle(response);
verify(asyncResponse).handle(response);
- verify(delegate).onResponse(span, response, exception);
}
@Test
@@ -103,7 +97,5 @@ public class ZipkinTracingHandlerTest {
} catch (Exception e) {
Assertions.assertEquals(exception, e);
}
-
- verify(delegate).onResponse(span, null, exception);
}
}
