SpringCloud/spring-cloud-sleuth
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Removed RibbonCommand wrapping

Browse Source 
without this change the instrumentation of RibbonCommand lead to creation of an additional client span
with removal of the instrumentation of the RibbonCommand no longer do we create that span

fixes gh-997
This commit is contained in:
Marcin Grzejszczak
2018-06-05 16:31:49 +02:00
parent fa054850ff
commit 586e3710ec
9 changed files with 21 additions and 402 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java
View File

@@ -383,6 +383,9 @@ class TracingHttpClientInstrumentation {
io.netty.handler.codec.http.HttpHeaders tracedHeaders = req
.requestHeaders();
span.set(this.handler.handleSend(this.injector, tracedHeaders, req));
if (log.isDebugEnabled()) {
log.debug("Handled send of " + span.get());
}
io.netty.handler.codec.http.HttpHeaders addedHeaders = tracedHeaders.copy();
originalHeaders.forEach(header -> addedHeaders.remove(header.getKey()));
try (Tracer.SpanInScope clientInScope = this.tracer.withSpanInScope(span.get())) {

2
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java
View File

@@ -179,7 +179,7 @@ class TraceExchangeFilterFunction implements ExchangeFilterFunction {
Span clientSpan = handler().handleSend(injector(), builder,
request, tracer().nextSpan());
if (log.isDebugEnabled()) {
log.debug("Created a client span for the WebClient " + clientSpan);
log.debug("Handled send of " + clientSpan);
}
if (parent == null) {
c = c.put(Span.class, clientSpan);

11
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java
View File

@@ -34,6 +34,8 @@ import brave.propagation.TraceContext;
import feign.Client;
import feign.Request;
import feign.Response;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
/**
* Feign client wrapper
@@ -42,6 +44,9 @@ import feign.Response;
* @since 2.0.0
*/
final class TracingFeignClient implements Client {
private static final Log log = LogFactory.getLog(TracingFeignClient.class);
static final Propagation.Setter<Map<String, Collection<String>>, String> SETTER =
new Propagation.Setter<Map<String, Collection<String>>, String>() {
@Override public void put(Map<String, Collection<String>> carrier, String key,
@@ -82,6 +87,9 @@ final class TracingFeignClient implements Client {
throws IOException {
Map<String, Collection<String>> headers = new HashMap<>(request.headers());
Span span = this.handler.handleSend(this.injector, headers, request);
if (log.isDebugEnabled()) {
log.debug("Handled send of " + span);
}
Response response = null;
Throwable error = null;
try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span)) {
@@ -93,6 +101,9 @@ final class TracingFeignClient implements Client {
}
finally {
this.handler.handleReceive(response, error, span);
if (log.isDebugEnabled()) {
log.debug("Handled receive of " + span);
}
}
}

7
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java
View File

@@ -18,6 +18,7 @@ package org.springframework.cloud.sleuth.instrument.zuul;
import javax.servlet.http.HttpServletResponse;
import brave.Span;
import brave.Tracer;
import brave.http.HttpServerHandler;
import brave.http.HttpTracing;
@@ -67,7 +68,11 @@ class TracePostZuulFilter extends ZuulFilter {
}
HttpServletResponse response = RequestContext.getCurrentContext().getResponse();
Throwable exception = RequestContext.getCurrentContext().getThrowable();
this.handler.handleSend(response, exception, this.tracer.currentSpan());
Span currentSpan = this.tracer.currentSpan();
this.handler.handleSend(response, exception, currentSpan);
if (log.isDebugEnabled()) {
log.debug("Handled send of " + currentSpan);
}
return null;
}

171
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceRibbonCommandFactory.java
View File

@@ -1,171 +0,0 @@
/*
* Copyright 2013-2018 the original author or authors.
*
* Licensed 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.springframework.cloud.sleuth.instrument.zuul;
import java.io.IOException;
import java.util.Collections;
import java.util.concurrent.Future;
import brave.Span;
import brave.Tracer;
import brave.http.HttpClientHandler;
import brave.http.HttpTracing;
import brave.propagation.Propagation;
import brave.propagation.TraceContext;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.SmartInitializingSingleton;
import org.springframework.cloud.netflix.ribbon.support.RibbonCommandContext;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory;
import org.springframework.http.client.ClientHttpResponse;
import rx.Observable;
/**
* Propagates traces downstream via http headers that contain trace metadata.
*
* @author Spencer Gibb
* @author Marcin Grzejszczak
* @since 1.1.0
*/
class TraceRibbonCommandFactory implements RibbonCommandFactory,
SmartInitializingSingleton {
static final Propagation.Setter<RibbonCommandContext, String> SETTER = new Propagation.Setter<RibbonCommandContext, String>() {
@Override public void put(RibbonCommandContext carrier, String key, String value) {
carrier.getHeaders().put(key, Collections.singletonList(value));
}
@Override public String toString() {
return "RibbonCommandContext::headers::put";
}
};
private static final Log log = LogFactory.getLog(TraceRibbonCommandFactory.class);
HttpTracing tracing;
Tracer tracer;
RibbonCommandFactory delegate;
HttpClientHandler<RibbonCommandContext, ClientHttpResponse> handler;
TraceContext.Injector<RibbonCommandContext> injector;
final BeanFactory beanFactory;
TraceRibbonCommandFactory(RibbonCommandFactory delegate, BeanFactory beanFactory) {
this.delegate = delegate;
this.beanFactory = beanFactory;
}
private void initialize() {
if (this.tracing == null) {
this.tracing = httpTracing();
}
if (this.tracer == null) {
this.tracer = httpTracing().tracing().tracer();
}
if (this.handler == null) {
this.handler = HttpClientHandler
.create(httpTracing(), new TraceRibbonCommandFactory.HttpAdapter());
}
if (this.injector == null) {
this.injector = httpTracing().tracing().propagation().injector(SETTER);
}
}
private HttpTracing httpTracing() {
if (this.tracing == null) {
this.tracing = this.beanFactory.getBean(HttpTracing.class);
}
return this.tracing;
}
@Override
public RibbonCommand create(final RibbonCommandContext context) {
// just in case - everything should be already initialized
initialize();
final RibbonCommand ribbonCommand = this.delegate.create(context);
Span span = this.tracer.currentSpan();
if (log.isDebugEnabled()) {
log.debug("Will set contents of the span " + this.tracer.currentSpan() + " in the ribbon command");
}
return new RibbonCommand() {
@Override public ClientHttpResponse execute() {
Span span = TraceRibbonCommandFactory.this.handler.handleSend(TraceRibbonCommandFactory.this.injector, context);
ClientHttpResponse response = null;
Throwable error = null;
try (Tracer.SpanInScope ws = TraceRibbonCommandFactory.this.tracer.withSpanInScope(span)) {
return response = ribbonCommand.execute();
} catch (RuntimeException | Error e) {
if (log.isDebugEnabled()) {
log.debug("Exception occurred while trying to execute ribbon command", e);
}
error = e;
throw e;
} finally {
TraceRibbonCommandFactory.this.handler.handleReceive(response, error, span);
}
}
// currently only .execute() is used in Zuul
@Override public Future<ClientHttpResponse> queue() {
parseRequest(context, span);
return ribbonCommand.queue();
}
// currently only .execute() is used in Zuul
@Override public Observable<ClientHttpResponse> observe() {
parseRequest(context, span);
return ribbonCommand.observe();
}
};
}
private void parseRequest(RibbonCommandContext context, Span span) {
TraceRibbonCommandFactory.this.tracing.clientParser()
.request(new TraceRibbonCommandFactory.HttpAdapter(), context, span);
}
@Override public void afterSingletonsInstantiated() {
initialize();
}
static final class HttpAdapter
extends brave.http.HttpClientAdapter<RibbonCommandContext, ClientHttpResponse> {
@Override public String method(RibbonCommandContext request) {
return request.getMethod();
}
@Override public String url(RibbonCommandContext request) {
return request.getUri();
}
@Override public String requestHeader(RibbonCommandContext request, String name) {
Object result = request.getHeaders().getFirst(name);
return result != null ? result.toString() : null;
}
@Override public Integer statusCode(ClientHttpResponse response) {
try {
return response.getRawStatusCode();
} catch (IOException e) {
return null;
}
}
}
}

55
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceRibbonCommandFactoryBeanPostProcessor.java
View File

@@ -1,55 +0,0 @@
/*
* Copyright 2013-2018 the original author or authors.
*
* Licensed 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.springframework.cloud.sleuth.instrument.zuul;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory;
/**
* Post processor that wraps a {@link RibbonCommandFactory}
* in its trace representation.
*
* @author Marcin Grzejszczak
*
* @since 2.0.0
*/
final class TraceRibbonCommandFactoryBeanPostProcessor implements BeanPostProcessor {
private final BeanFactory beanFactory;
TraceRibbonCommandFactoryBeanPostProcessor(BeanFactory beanFactory) {
this.beanFactory = beanFactory;
}
@Override
public Object postProcessBeforeInitialization(Object bean, String beanName)
throws BeansException {
return bean;
}
@Override
public Object postProcessAfterInitialization(Object bean, String beanName)
throws BeansException {
if (bean instanceof RibbonCommandFactory
&& !(bean instanceof TraceRibbonCommandFactory)) {
return new TraceRibbonCommandFactory((RibbonCommandFactory) bean, this.beanFactory);
}
return bean;
}
}

9
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java
View File

@@ -23,7 +23,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand;
import org.springframework.cloud.sleuth.instrument.web.TraceWebServletAutoConfiguration;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@@ -48,14 +47,6 @@ public class TraceZuulAutoConfiguration {
return new TracePostZuulFilter(httpTracing);
}
@ConditionalOnClass(RibbonCommand.class)
static class RibbonConfig {
@Bean
static TraceRibbonCommandFactoryBeanPostProcessor traceRibbonCommandFactoryBeanPostProcessor(BeanFactory beanFactory) {
return new TraceRibbonCommandFactoryBeanPostProcessor(beanFactory);
}
}
@Bean
static TraceZuulHandlerMappingBeanPostProcessor traceHandlerMappingBeanPostProcessor(BeanFactory beanFactory) {
return new TraceZuulHandlerMappingBeanPostProcessor(beanFactory);

56
spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TraceRibbonCommandFactoryBeanPostProcessorTests.java
View File

@@ -1,56 +0,0 @@
/*
* Copyright 2013-2018 the original author or authors.
*
* Licensed 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.springframework.cloud.sleuth.instrument.zuul;
import brave.Tracing;
import brave.propagation.StrictCurrentTraceContext;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory;
import org.springframework.cloud.sleuth.util.ArrayListSpanReporter;
import static org.assertj.core.api.BDDAssertions.then;
/**
* @author Marcin Grzejszczak
*/
@RunWith(MockitoJUnitRunner.class)
public class TraceRibbonCommandFactoryBeanPostProcessorTests {
ArrayListSpanReporter reporter = new ArrayListSpanReporter();
Tracing tracing = Tracing.newBuilder()
.currentTraceContext(new StrictCurrentTraceContext())
.spanReporter(this.reporter)
.build();
@Mock RibbonCommandFactory ribbonCommandFactory;
@InjectMocks TraceRibbonCommandFactoryBeanPostProcessor postProcessor;
@Test
public void should_return_a_bean_as_it_is_if_its_not_a_ribbon_command_Factory() {
then(this.postProcessor.postProcessAfterInitialization("", "name")).isEqualTo("");
}
@Test
public void should_wrap_ribbon_command_factory_in_a_trace_representation() {
then(this.postProcessor.postProcessAfterInitialization(this.ribbonCommandFactory, "name")).isInstanceOf(
TraceRibbonCommandFactory.class);
}
}

109
spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TraceRibbonCommandFactoryTest.java
View File

@@ -1,109 +0,0 @@
/*
* Copyright 2013-2018 the original author or authors.
*
* Licensed 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.springframework.cloud.sleuth.instrument.zuul;
import java.util.ArrayList;
import brave.ErrorParser;
import brave.Span;
import brave.Tracer;
import brave.Tracing;
import brave.http.HttpTracing;
import brave.propagation.StrictCurrentTraceContext;
import com.netflix.zuul.context.RequestContext;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.BDDMockito;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.cloud.netflix.ribbon.support.RibbonCommandContext;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory;
import org.springframework.cloud.sleuth.instrument.web.SleuthHttpParserAccessor;
import org.springframework.cloud.sleuth.util.ArrayListSpanReporter;
import org.springframework.http.HttpHeaders;
import org.springframework.util.LinkedMultiValueMap;
import static org.assertj.core.api.BDDAssertions.then;
/**
* @author Marcin Grzejszczak
*/
@RunWith(MockitoJUnitRunner.class)
public class TraceRibbonCommandFactoryTest {
ArrayListSpanReporter reporter = new ArrayListSpanReporter();
Tracing tracing = Tracing.newBuilder()
.currentTraceContext(new StrictCurrentTraceContext())
.spanReporter(this.reporter)
.build();
HttpTracing httpTracing = HttpTracing.newBuilder(this.tracing)
.clientParser(SleuthHttpParserAccessor.getClient())
.serverParser(SleuthHttpParserAccessor.getServer(new ErrorParser()))
.build();
@Mock BeanFactory beanFactory;
@Mock RibbonCommandFactory ribbonCommandFactory;
@Mock RibbonCommand ribbonCommand;
TraceRibbonCommandFactory traceRibbonCommandFactory;
Span span = this.tracing.tracer().nextSpan().name("name");
@Before
@SuppressWarnings({ "deprecation", "unchecked" })
public void setup() {
BDDMockito.given(this.beanFactory.getBean(HttpTracing.class))
.willReturn(this.httpTracing);
this.traceRibbonCommandFactory = new TraceRibbonCommandFactory(
this.ribbonCommandFactory, this.beanFactory);
BDDMockito.given(this.ribbonCommandFactory
.create(BDDMockito.any(RibbonCommandContext.class)))
.willReturn(this.ribbonCommand);
}
@After
public void cleanup() {
RequestContext.getCurrentContext().unset();
this.tracing.close();
}
@Test
public void should_attach_trace_headers_to_the_span() throws Exception {
try (Tracer.SpanInScope ws = this.tracing.tracer().withSpanInScope(this.span)) {
RibbonCommand ribbonCommand = this.traceRibbonCommandFactory
.create(ribbonCommandContext());
ribbonCommand.execute();
} finally {
this.span.finish();
}
then(this.reporter.getSpans()).hasSize(2);
// RPC
zipkin2.Span span = this.reporter.getSpans().get(0);
then(span.tags())
.containsEntry("http.method", "GET")
.containsEntry("http.url", "http://localhost:1234/foo");
zipkin2.Span main = this.reporter.getSpans().get(1);
then(main.name()).isEqualTo("name");
}
private RibbonCommandContext ribbonCommandContext() {
return new RibbonCommandContext("serviceId", "GET", "http://localhost:1234/foo",
false, new HttpHeaders(), new LinkedMultiValueMap<>(), null, new ArrayList<>());
}
}