Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed sending the tracing information back in the response #329

Merged
merged 1 commit into from
Jul 11, 2016
Merged

Removed sending the tracing information back in the response #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 0 additions & 44 deletions ...ain/java/org/springframework/cloud/sleuth/instrument/web/HttpServletResponseInjector.java
View file
Open in desktop

This file was deleted.

21 changes: 1 addition & 20 deletions ...leuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.SpanExtractor;
import org.springframework.cloud.sleuth.SpanInjector;
import org.springframework.cloud.sleuth.SpanReporter;
import org.springframework.cloud.sleuth.TraceKeys;
import org.springframework.cloud.sleuth.Tracer;
Expand All @@ -45,8 +44,6 @@
import org.springframework.web.filter.GenericFilterBean;
import org.springframework.web.util.UrlPathHelper;

import static org.springframework.util.StringUtils.hasText;

/**
* Filter that takes the value of the {@link Span#SPAN_ID_NAME} and
* {@link Span#TRACE_ID_NAME} header from either request or response and uses them to
Expand Down Expand Up @@ -89,29 +86,25 @@ public class TraceFilter extends GenericFilterBean {
private final Pattern skipPattern;
private final SpanReporter spanReporter;
private final SpanExtractor<HttpServletRequest> spanExtractor;
private final SpanInjector<HttpServletResponse> spanInjector;
private final HttpTraceKeysInjector httpTraceKeysInjector;

private UrlPathHelper urlPathHelper = new UrlPathHelper();

public TraceFilter(Tracer tracer, TraceKeys traceKeys, SpanReporter spanReporter,
SpanExtractor<HttpServletRequest> spanExtractor,
SpanInjector<HttpServletResponse> spanInjector,
HttpTraceKeysInjector httpTraceKeysInjector) {
this(tracer, traceKeys, Pattern.compile(DEFAULT_SKIP_PATTERN), spanReporter,
spanExtractor, spanInjector, httpTraceKeysInjector);
spanExtractor, httpTraceKeysInjector);
}

public TraceFilter(Tracer tracer, TraceKeys traceKeys, Pattern skipPattern,
SpanReporter spanReporter, SpanExtractor<HttpServletRequest> spanExtractor,
SpanInjector<HttpServletResponse> spanInjector,
HttpTraceKeysInjector httpTraceKeysInjector) {
this.tracer = tracer;
this.traceKeys = traceKeys;
this.skipPattern = skipPattern;
this.spanReporter = spanReporter;
this.spanExtractor = spanExtractor;
this.spanInjector = spanInjector;
this.httpTraceKeysInjector = httpTraceKeysInjector;
}

Expand All @@ -138,7 +131,6 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
processErrorRequest(filterChain, request, response, spanFromRequest);
return;
}
addToResponseIfNotPresent(response, Span.SAMPLED_NAME, skip ? Span.SPAN_NOT_SAMPLED : Span.SPAN_SAMPLED);
String name = HTTP_COMPONENT + ":" + uri;
try {
spanFromRequest = createSpan(request, skip, spanFromRequest, name);
Expand All @@ -150,9 +142,6 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
}
Throwable exception = null;
try {
this.spanInjector.inject(spanFromRequest, response);
// Add headers before filter chain in case one of the filters flushes the
// response...
filterChain.doFilter(request, response);
} catch (Throwable e) {
exception = e;
Expand All @@ -166,7 +155,6 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
return;
}
spanFromRequest = createSpanIfRequestNotHandled(request, spanFromRequest, name, skip);
addToResponseIfNotPresent(response, Span.SAMPLED_NAME, skip ? Span.SPAN_NOT_SAMPLED : Span.SPAN_SAMPLED);
detachOrCloseSpans(request, response, spanFromRequest, exception);
}
}
Expand Down Expand Up @@ -364,13 +352,6 @@ else if (httpStatus >= 100 && (httpStatus < 200) || (httpStatus > 399)) {
}
}

private void addToResponseIfNotPresent(HttpServletResponse response, String name,
String value) {
if (!hasText(response.getHeader(name))) {
response.addHeader(name, value);
}
}

protected boolean isAsyncStarted(HttpServletRequest request) {
return WebAsyncUtils.getAsyncManager(request).isConcurrentHandlingStarted();
}
Expand Down
11 changes: 1 addition & 10 deletions .../main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package org.springframework.cloud.sleuth.instrument.web;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.regex.Pattern;

import org.springframework.beans.factory.BeanFactory;
Expand All @@ -32,7 +31,6 @@
import org.springframework.boot.context.embedded.FilterRegistrationBean;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.cloud.sleuth.SpanExtractor;
import org.springframework.cloud.sleuth.SpanInjector;
import org.springframework.cloud.sleuth.SpanNamer;
import org.springframework.cloud.sleuth.SpanReporter;
import org.springframework.cloud.sleuth.TraceKeys;
Expand Down Expand Up @@ -89,7 +87,6 @@ public TraceSpringDataBeanPostProcessor traceSpringDataBeanPostProcessor(BeanFac
public FilterRegistrationBean traceWebFilter(Tracer tracer, TraceKeys traceKeys,
SkipPatternProvider skipPatternProvider, SpanReporter spanReporter,
SpanExtractor<HttpServletRequest> spanExtractor,
SpanInjector<HttpServletResponse> spanInjector,
HttpTraceKeysInjector httpTraceKeysInjector, TraceFilter traceFilter) {
FilterRegistrationBean filterRegistrationBean = new FilterRegistrationBean(traceFilter);
filterRegistrationBean.setDispatcherTypes(ASYNC, ERROR, FORWARD, INCLUDE, REQUEST);
Expand All @@ -100,10 +97,9 @@ public FilterRegistrationBean traceWebFilter(Tracer tracer, TraceKeys traceKeys,
public TraceFilter traceFilter(Tracer tracer, TraceKeys traceKeys,
SkipPatternProvider skipPatternProvider, SpanReporter spanReporter,
SpanExtractor<HttpServletRequest> spanExtractor,
SpanInjector<HttpServletResponse> spanInjector,
HttpTraceKeysInjector httpTraceKeysInjector) {
return new TraceFilter(tracer, traceKeys, skipPatternProvider.skipPattern(),
spanReporter, spanExtractor, spanInjector, httpTraceKeysInjector);
spanReporter, spanExtractor, httpTraceKeysInjector);
}

@Bean
Expand All @@ -112,11 +108,6 @@ public SpanExtractor<HttpServletRequest> httpServletRequestSpanExtractor(
return new HttpServletRequestExtractor(skipPatternProvider.skipPattern());
}

@Bean
public SpanInjector<HttpServletResponse> httpServletResponseSpanInjector() {
return new HttpServletResponseInjector();
}

@Configuration
@ConditionalOnClass(ManagementServerProperties.class)
@ConditionalOnMissingBean(SkipPatternProvider.class)
Expand Down
8 changes: 1 addition & 7 deletions ...springframework/cloud/sleuth/instrument/web/TraceFilterAlwaysSamplerIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public void when_always_sampler_is_used_span_is_exportable() throws Exception {

MvcResult mvcResult = whenSentPingWithTraceId(expectedTraceId);

then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId);
then(span.isExportable());
}

Expand All @@ -52,14 +51,13 @@ public void when_not_sampling_header_present_span_is_not_exportable() throws Exc

MvcResult mvcResult = whenSentPingWithTraceIdAndNotSampling(expectedTraceId);

then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId);
then(span.isExportable()).isFalse();
}

@Override
protected void configureMockMvcBuilder(DefaultMockMvcBuilder mockMvcBuilder) {
mockMvcBuilder.addFilters(new TraceFilter(this.tracer, this.traceKeys,
new NoOpSpanReporter(), this.spanExtractor, this.spanInjector,
new NoOpSpanReporter(), this.spanExtractor,
this.httpTraceKeysInjector));
}

Expand Down Expand Up @@ -87,10 +85,6 @@ private MvcResult sendPingWithTraceId(String headerName, Long correlationId,
return this.mockMvc.perform(request).andReturn();
}

private Long tracingHeaderFrom(MvcResult mvcResult) {
return Span.hexToId(mvcResult.getResponse().getHeader(Span.TRACE_ID_NAME));
}

@DefaultTestAutoConfiguration
@RestController
@Configuration
Expand Down
9 changes: 2 additions & 7 deletions ...java/org/springframework/cloud/sleuth/instrument/web/TraceFilterCustomExtractorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.net.URI;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Random;
Expand Down Expand Up @@ -84,20 +83,16 @@ public void should_create_a_valid_span_from_custom_headers() {
.header("mySpanId", Span.idToHex(spanId)).build();

@SuppressWarnings("rawtypes")
ResponseEntity<Map> requestHeaders = this.restTemplate.exchange(requestEntity,
ResponseEntity<Map> responseHeaders = this.restTemplate.exchange(requestEntity,
Map.class);

await().until(() -> then(this.accumulator.getSpans().stream().filter(
span -> span.getSpanId() == spanId).findFirst().get())
.hasTraceIdEqualTo(traceId));
then(requestHeaders.getBody())
then(responseHeaders.getBody())
.containsEntry("correlationid", Span.idToHex(traceId))
.containsEntry("myspanid", Span.idToHex(spanId))
.as("input request headers");
then(requestHeaders.getHeaders())
.containsEntry("correlationId",
Collections.singletonList(Span.idToHex(traceId)))
.containsKey("mySpanId").as("response headers");
}

@Configuration
Expand Down
19 changes: 5 additions & 14 deletions ...est/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,8 @@ public void clearSpans() {

@Test
public void should_create_and_return_trace_in_HTTP_header() throws Exception {
MvcResult mvcResult = whenSentPingWithoutTracingData();
whenSentPingWithoutTracingData();

then(tracingHeaderFrom(mvcResult)).isNotNull();
Span parentSpan = this.spanAccumulator.getSpans().stream().filter(
span -> span.getSpanId() == TraceFilterIntegrationTests.span.getParents().get(0))
.findFirst().get();
Expand All @@ -75,7 +74,9 @@ public void should_ignore_sampling_the_span_if_uri_matches_management_properties
throws Exception {
MvcResult mvcResult = whenSentInfoWithTraceId(new Random().nextLong());

then(notSampledHeaderIsPresent(mvcResult)).isEqualTo(true);
// https://github.com/spring-cloud/spring-cloud-sleuth/issues/327
// we don't want to respond with any tracing data
then(notSampledHeaderIsPresent(mvcResult)).isEqualTo(false);
then(ExceptionUtils.getLastException()).isNull();
}

Expand All @@ -86,7 +87,6 @@ public void when_traceId_is_sent_should_not_create_a_new_one_but_return_the_exis

MvcResult mvcResult = whenSentPingWithTraceId(expectedTraceId);

then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId);
then(ExceptionUtils.getLastException()).isNull();
}

Expand All @@ -108,7 +108,6 @@ public void when_traceId_is_sent_to_async_endpoint_span_is_joined() throws Excep
mvcResult = this.mockMvc.perform(asyncDispatch(mvcResult))
.andExpect(status().isOk()).andReturn();

then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId);
then(this.tracer.getCurrentSpan()).isNull();
then(ExceptionUtils.getLastException()).isNull();
}
Expand All @@ -121,7 +120,6 @@ public void should_add_a_custom_tag_to_the_span_created_in_controller() throws E
mvcResult = this.mockMvc.perform(asyncDispatch(mvcResult))
.andExpect(status().isOk()).andReturn();

then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId);
Optional<Span> taggedSpan = this.spanAccumulator.getSpans().stream()
.filter(span -> span.tags().containsKey("tag")).findFirst();
then(taggedSpan.isPresent()).isTrue();
Expand All @@ -137,7 +135,6 @@ public void should_log_tracing_information_when_exception_was_thrown() throws Ex

MvcResult mvcResult = whenSentToNonExistentEndpointWithTraceId(expectedTraceId);

then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId);
then(this.tracer.getCurrentSpan()).isNull();
then(ExceptionUtils.getLastException()).isNull();
}
Expand Down Expand Up @@ -208,12 +205,10 @@ private MvcResult sendRequestWithTraceId(String path, String headerName, Long tr

private MvcResult whenSentRequestWithTraceIdAndNoSpanId(Long traceId)
throws Exception {
MvcResult mvcResult = this.mockMvc
return this.mockMvc
.perform(MockMvcRequestBuilders.get("/ping").accept(MediaType.TEXT_PLAIN)
.header(Span.TRACE_ID_NAME, Span.idToHex(traceId)))
.andReturn();
then(tracingHeaderFrom(mvcResult)).isEqualTo(traceId);
return mvcResult;
}

private MvcResult sendRequestWithTraceId(String path, String headerName, Long traceId, HttpStatus status)
Expand All @@ -226,10 +221,6 @@ private MvcResult sendRequestWithTraceId(String path, String headerName, Long tr
.andReturn();
}

private Long tracingHeaderFrom(MvcResult mvcResult) {
return Span.hexToId(mvcResult.getResponse().getHeader(Span.TRACE_ID_NAME));
}

private boolean notSampledHeaderIsPresent(MvcResult mvcResult) {
return Span.SPAN_NOT_SAMPLED
.equals(mvcResult.getResponse().getHeader(Span.SAMPLED_NAME));
Expand Down
4 changes: 2 additions & 2 deletions ...org/springframework/cloud/sleuth/instrument/web/TraceFilterMockChainIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public MockHttpServletRequestBuilder builder() {
public void startsNewTrace() throws Exception {
TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys, new NoOpSpanReporter(),
new HttpServletRequestExtractor(Pattern.compile(TraceFilter.DEFAULT_SKIP_PATTERN)),
new HttpServletResponseInjector(), keysInjector);
keysInjector);
filter.doFilter(this.request, this.response, this.filterChain);
assertNull(TestSpanContextHolder.getCurrentSpan());
}
Expand All @@ -86,7 +86,7 @@ public void continuesSpanFromHeaders() throws Exception {
.header(Span.TRACE_ID_NAME, generator.nextLong()).buildRequest(new MockServletContext());
TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys, new NoOpSpanReporter(),
new HttpServletRequestExtractor(Pattern.compile(TraceFilter.DEFAULT_SKIP_PATTERN)),
new HttpServletResponseInjector(), keysInjector);
keysInjector);
filter.doFilter(this.request, this.response, this.filterChain);
assertNull(TestSpanContextHolder.getCurrentSpan());
}
Expand Down
Loading