Reusing input headers for Gateway instrumentation
fixes gh-1469
This commit is contained in:
@@ -10,7 +10,6 @@ jobs:
|
||||
branches:
|
||||
ignore:
|
||||
- gh-pages # list of branches to ignore
|
||||
resource_class: large
|
||||
steps:
|
||||
- checkout
|
||||
- restore_cache:
|
||||
|
||||
@@ -29,6 +29,7 @@ import org.springframework.cloud.gateway.filter.headers.HttpHeadersFilter;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.server.reactive.ServerHttpRequest;
|
||||
import org.springframework.http.server.reactive.ServerHttpResponse;
|
||||
import org.springframework.lang.NonNull;
|
||||
import org.springframework.web.server.ServerWebExchange;
|
||||
|
||||
final class TraceRequestHttpHeadersFilter extends AbstractHttpHeadersFilter {
|
||||
@@ -48,17 +49,16 @@ final class TraceRequestHttpHeadersFilter extends AbstractHttpHeadersFilter {
|
||||
if (log.isDebugEnabled()) {
|
||||
log.debug("Will instrument the HTTP request headers");
|
||||
}
|
||||
ServerHttpRequest.Builder builder = exchange.getRequest().mutate();
|
||||
Span span = this.handler.handleSend(this.injector, builder);
|
||||
TraceCarrier carrier = new TraceCarrier(exchange.getRequest(), input);
|
||||
Span span = this.handler.handleSend(this.injector, carrier);
|
||||
if (log.isDebugEnabled()) {
|
||||
log.debug(
|
||||
"Client span " + span + " created for the request. New headers are "
|
||||
+ builder.build().getHeaders().toSingleValueMap());
|
||||
log.debug("Client span " + span + " created for the request. New headers are "
|
||||
+ carrier.filteredHeaders.toSingleValueMap());
|
||||
}
|
||||
exchange.getAttributes().put(SPAN_ATTRIBUTE, span);
|
||||
HttpHeaders headersWithInput = new HttpHeaders();
|
||||
headersWithInput.addAll(input);
|
||||
builder.build().getHeaders().forEach(headersWithInput::put);
|
||||
carrier.filteredHeaders.forEach(headersWithInput::put);
|
||||
return headersWithInput;
|
||||
}
|
||||
|
||||
@@ -69,10 +69,22 @@ final class TraceRequestHttpHeadersFilter extends AbstractHttpHeadersFilter {
|
||||
|
||||
}
|
||||
|
||||
class TraceCarrier {
|
||||
|
||||
final ServerHttpRequest originalRequest;
|
||||
|
||||
final HttpHeaders filteredHeaders;
|
||||
|
||||
TraceCarrier(@NonNull ServerHttpRequest originalRequest, @NonNull HttpHeaders filteredHeaders) {
|
||||
this.originalRequest = originalRequest;
|
||||
this.filteredHeaders = filteredHeaders;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
final class TraceResponseHttpHeadersFilter extends AbstractHttpHeadersFilter {
|
||||
|
||||
private static final Log log = LogFactory
|
||||
.getLog(TraceResponseHttpHeadersFilter.class);
|
||||
private static final Log log = LogFactory.getLog(TraceResponseHttpHeadersFilter.class);
|
||||
|
||||
private TraceResponseHttpHeadersFilter(HttpTracing httpTracing) {
|
||||
super(httpTracing);
|
||||
@@ -109,23 +121,23 @@ abstract class AbstractHttpHeadersFilter implements HttpHeadersFilter {
|
||||
|
||||
static final String SPAN_ATTRIBUTE = Span.class.getName();
|
||||
|
||||
private static final Propagation.Setter<ServerHttpRequest.Builder, String> SETTER = new Propagation.Setter<ServerHttpRequest.Builder, String>() {
|
||||
private static final Propagation.Setter<TraceCarrier, String> SETTER = new Propagation.Setter<TraceCarrier, String>() {
|
||||
@Override
|
||||
public void put(ServerHttpRequest.Builder carrier, String key, String value) {
|
||||
carrier.headers(httpHeaders -> httpHeaders.set(key, value));
|
||||
public void put(TraceCarrier carrier, String key, String value) {
|
||||
carrier.filteredHeaders.set(key, value);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return "ServerHttpRequest.Builder::header";
|
||||
return "TraceCarrier::httpHeaders::set";
|
||||
}
|
||||
};
|
||||
|
||||
final Tracer tracer;
|
||||
|
||||
final HttpClientHandler<ServerHttpRequest.Builder, ServerHttpResponse> handler;
|
||||
final HttpClientHandler<TraceCarrier, ServerHttpResponse> handler;
|
||||
|
||||
final TraceContext.Injector<ServerHttpRequest.Builder> injector;
|
||||
final TraceContext.Injector<TraceCarrier> injector;
|
||||
|
||||
final HttpTracing httpTracing;
|
||||
|
||||
@@ -136,29 +148,27 @@ abstract class AbstractHttpHeadersFilter implements HttpHeadersFilter {
|
||||
this.httpTracing = httpTracing;
|
||||
}
|
||||
|
||||
private static class ServerHttpAdapter extends
|
||||
brave.http.HttpClientAdapter<ServerHttpRequest.Builder, ServerHttpResponse> {
|
||||
private static class ServerHttpAdapter extends brave.http.HttpClientAdapter<TraceCarrier, ServerHttpResponse> {
|
||||
|
||||
@Override
|
||||
public String method(ServerHttpRequest.Builder request) {
|
||||
return request.build().getMethodValue();
|
||||
public String method(TraceCarrier request) {
|
||||
return request.originalRequest.getMethodValue();
|
||||
}
|
||||
|
||||
@Override
|
||||
public String url(ServerHttpRequest.Builder request) {
|
||||
return request.build().getURI().toString();
|
||||
public String url(TraceCarrier request) {
|
||||
return request.originalRequest.getURI().toString();
|
||||
}
|
||||
|
||||
@Override
|
||||
public String requestHeader(ServerHttpRequest.Builder request, String name) {
|
||||
Object result = request.build().getHeaders().get(name);
|
||||
public String requestHeader(TraceCarrier request, String name) {
|
||||
Object result = request.filteredHeaders.get(name);
|
||||
return result != null ? result.toString() : "";
|
||||
}
|
||||
|
||||
@Override
|
||||
public Integer statusCode(ServerHttpResponse response) {
|
||||
return response.getStatusCode() != null ? response.getStatusCode().value()
|
||||
: null;
|
||||
return response.getStatusCode() != null ? response.getStatusCode().value() : null;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -48,14 +48,13 @@ public class TraceRequestHttpHeadersFilterTests {
|
||||
HttpHeadersFilter filter = TraceRequestHttpHeadersFilter.create(this.httpTracing);
|
||||
HttpHeaders httpHeaders = new HttpHeaders();
|
||||
httpHeaders.set("X-Hello", "World");
|
||||
httpHeaders.set("X-Auth-User", "aaaa");
|
||||
httpHeaders.set("X-B3-TraceId", "52f112af7472aff0");
|
||||
httpHeaders.set("X-B3-SpanId", "53e6ab6fc5dfee58");
|
||||
MockServerHttpRequest request = MockServerHttpRequest.post("foo/bar")
|
||||
.headers(httpHeaders).build();
|
||||
MockServerWebExchange exchange = MockServerWebExchange.builder(request).build();
|
||||
|
||||
HttpHeaders filteredHeaders = filter.filter(requestHeaders(), exchange);
|
||||
HttpHeaders filteredHeaders = filter.filter(requestHeaders(httpHeaders), exchange);
|
||||
|
||||
BDDAssertions.then(filteredHeaders.get("X-B3-TraceId"))
|
||||
.isNotEqualTo(httpHeaders.get("X-B3-TraceId"));
|
||||
@@ -81,7 +80,7 @@ public class TraceRequestHttpHeadersFilterTests {
|
||||
.headers(httpHeaders).build();
|
||||
MockServerWebExchange exchange = MockServerWebExchange.builder(request).build();
|
||||
|
||||
HttpHeaders filteredHeaders = filter.filter(requestHeaders(), exchange);
|
||||
HttpHeaders filteredHeaders = filter.filter(requestHeaders(httpHeaders), exchange);
|
||||
|
||||
BDDAssertions.then(filteredHeaders.get("X-B3-TraceId")).isNotEmpty();
|
||||
BDDAssertions.then(filteredHeaders.get("X-B3-SpanId")).isNotEmpty();
|
||||
@@ -95,18 +94,40 @@ public class TraceRequestHttpHeadersFilterTests {
|
||||
.isNotNull();
|
||||
}
|
||||
|
||||
// #1352
|
||||
// #1469
|
||||
@Test
|
||||
public void should_set_tracing_headers_with_multiple_values() {
|
||||
public void should_reuse_headers_only_from_input_since_exchange_may_contain_already_ignored_headers() {
|
||||
HttpHeadersFilter filter = TraceRequestHttpHeadersFilter.create(this.httpTracing);
|
||||
HttpHeaders httpHeaders = new HttpHeaders();
|
||||
httpHeaders.addAll("X-Hello", Arrays.asList("World1", "World2"));
|
||||
httpHeaders.set("X-Hello", "World");
|
||||
MockServerHttpRequest request = MockServerHttpRequest.post("foo/bar")
|
||||
.headers(httpHeaders).build();
|
||||
MockServerWebExchange exchange = MockServerWebExchange.builder(request).build();
|
||||
|
||||
HttpHeaders filteredHeaders = filter.filter(requestHeaders(), exchange);
|
||||
|
||||
BDDAssertions.then(filteredHeaders.get("X-B3-TraceId")).isNotEmpty();
|
||||
BDDAssertions.then(filteredHeaders.get("X-B3-SpanId")).isNotEmpty();
|
||||
BDDAssertions.then(filteredHeaders.get("X-Hello")).isNullOrEmpty();
|
||||
BDDAssertions
|
||||
.then((Object) exchange
|
||||
.getAttribute(TraceRequestHttpHeadersFilter.SPAN_ATTRIBUTE))
|
||||
.isNotNull();
|
||||
}
|
||||
|
||||
// #1352
|
||||
@Test
|
||||
public void should_set_tracing_headers_with_multiple_values() {
|
||||
HttpHeadersFilter filter = TraceRequestHttpHeadersFilter.create(this.httpTracing);
|
||||
HttpHeaders httpHeaders = new HttpHeaders();
|
||||
httpHeaders.add("X-Hello-Request", "Request World");
|
||||
httpHeaders.addAll("X-Hello", Arrays.asList("World1", "World2"));
|
||||
MockServerHttpRequest request = MockServerHttpRequest.post("foo/bar")
|
||||
.headers(httpHeaders).build();
|
||||
MockServerWebExchange exchange = MockServerWebExchange.builder(request).build();
|
||||
|
||||
HttpHeaders filteredHeaders = filter.filter(httpHeaders, exchange);
|
||||
|
||||
BDDAssertions.then(filteredHeaders.get("X-B3-TraceId")).isNotEmpty();
|
||||
BDDAssertions.then(filteredHeaders.get("X-B3-SpanId")).isNotEmpty();
|
||||
BDDAssertions.then(filteredHeaders.get("X-Hello"))
|
||||
@@ -126,4 +147,12 @@ public class TraceRequestHttpHeadersFilterTests {
|
||||
return headers;
|
||||
}
|
||||
|
||||
private HttpHeaders requestHeaders(HttpHeaders originalHeaders) {
|
||||
HttpHeaders headers = new HttpHeaders();
|
||||
headers.putAll(originalHeaders);
|
||||
headers.add("X-Hello-Request", "Request World");
|
||||
headers.add("X-Auth-User", "aaaa");
|
||||
return headers;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user