From cedede7fd40a9cd6e6db9edc5447cd2fea4c3616 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 4 Mar 2016 14:41:05 +0000 Subject: [PATCH] Move trace response headers up to start of filter They seem to only get set once (by the proxy, which is most likely to be correct) even if Zuul is proxying a service that itself is a Sleuth application. Fixes gh-199 --- .../sleuth/instrument/web/TraceFilter.java | 13 ++++++++----- .../instrument/zuul/TracePreZuulFilter.java | 18 ++++++++---------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java index 88314472e..a78779d55 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java @@ -158,8 +158,7 @@ public class TraceFilter extends OncePerRequestFilter } else { if (skip) { - spanFromRequest = this.tracer.createSpan(name, - NeverSampler.INSTANCE); + spanFromRequest = this.tracer.createSpan(name, NeverSampler.INSTANCE); } else { spanFromRequest = this.tracer.createSpan(name); @@ -172,6 +171,9 @@ public class TraceFilter extends OncePerRequestFilter try { addRequestTags(request); + // Add headers before filter chain in case one of the filters flushes the + // response... + addResponseHeaders(response, spanFromRequest); filterChain.doFilter(request, response); } @@ -190,7 +192,6 @@ public class TraceFilter extends OncePerRequestFilter } if (spanFromRequest != null) { addResponseTags(response, exception); - addResponseHeaders(response, spanFromRequest); if (spanFromRequest.hasSavedSpan()) { publish(new ServerSentEvent(this, spanFromRequest.getSavedSpan(), spanFromRequest)); @@ -203,8 +204,10 @@ public class TraceFilter extends OncePerRequestFilter private void addResponseHeaders(HttpServletResponse response, Span span) { if (span != null) { - response.addHeader(Span.SPAN_ID_NAME, Span.idToHex(span.getSpanId())); - response.addHeader(Span.TRACE_ID_NAME, Span.idToHex(span.getTraceId())); + if (!response.containsHeader(Span.SPAN_ID_NAME)) { + response.addHeader(Span.SPAN_ID_NAME, Span.idToHex(span.getSpanId())); + response.addHeader(Span.TRACE_ID_NAME, Span.idToHex(span.getTraceId())); + } } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java index 063b6e624..8c804a865 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java @@ -62,23 +62,21 @@ public class TracePreZuulFilter extends ZuulFilter @Override public Object run() { RequestContext ctx = RequestContext.getCurrentContext(); - Map response = ctx.getZuulRequestHeaders(); - // N.B. this will only work with the simple host filter (not ribbon) unless you - // set hystrix.execution.isolation.strategy=SEMAPHORE + Map requestHeaders = ctx.getZuulRequestHeaders(); Span span = getCurrentSpan(); if (span == null) { - setHeader(response, Span.NOT_SAMPLED_NAME, "true"); + setHeader(requestHeaders, Span.NOT_SAMPLED_NAME, "true"); return null; } try { - setHeader(response, Span.SPAN_ID_NAME, span.getSpanId()); - setHeader(response, Span.TRACE_ID_NAME, span.getTraceId()); - setHeader(response, Span.SPAN_NAME_NAME, span.getName()); + setHeader(requestHeaders, Span.SPAN_ID_NAME, span.getSpanId()); + setHeader(requestHeaders, Span.TRACE_ID_NAME, span.getTraceId()); + setHeader(requestHeaders, Span.SPAN_NAME_NAME, span.getName()); if (!span.isExportable()) { - setHeader(response, Span.NOT_SAMPLED_NAME, "true"); + setHeader(requestHeaders, Span.NOT_SAMPLED_NAME, "true"); } - setHeader(response, Span.PARENT_ID_NAME, getParentId(span)); - setHeader(response, Span.PROCESS_ID_NAME, span.getProcessId()); + setHeader(requestHeaders, Span.PARENT_ID_NAME, getParentId(span)); + setHeader(requestHeaders, Span.PROCESS_ID_NAME, span.getProcessId()); // TODO: the client sent event should come from the client not the filter! publish(new ClientSentEvent(this, span)); }