From 46b1a29bfece2088922211d30ec260cb8053aeae Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Fri, 24 Jan 2020 08:42:05 +0800 Subject: [PATCH] Removes deprecations from server code (#1534) --- .../sleuth/instrument/web/TraceWebFilter.java | 147 +++++++++--------- .../instrument/zuul/TracePostZuulFilter.java | 52 ++++++- 2 files changed, 118 insertions(+), 81 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java index 4d197ba21..d92d8683f 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebFilter.java @@ -16,13 +16,15 @@ package org.springframework.cloud.sleuth.instrument.web; +import java.net.InetSocketAddress; import java.util.concurrent.atomic.AtomicBoolean; import brave.Span; import brave.Tracer; import brave.http.HttpServerHandler; +import brave.http.HttpServerRequest; +import brave.http.HttpServerResponse; import brave.http.HttpTracing; -import brave.propagation.Propagation; import brave.propagation.TraceContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -35,10 +37,8 @@ import reactor.util.context.Context; import org.springframework.beans.factory.BeanFactory; import org.springframework.core.Ordered; -import org.springframework.http.HttpHeaders; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; -import org.springframework.http.server.reactive.ServerHttpResponseDecorator; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.ServerWebExchange; @@ -65,18 +65,6 @@ public final class TraceWebFilter implements WebFilter, Ordered { + ".TRACE"; static final String MVC_CONTROLLER_CLASS_KEY = "mvc.controller.class"; static final String MVC_CONTROLLER_METHOD_KEY = "mvc.controller.method"; - static final Propagation.Getter GETTER = new Propagation.Getter() { - - @Override - public String get(HttpHeaders carrier, String key) { - return carrier.getFirst(key); - } - - @Override - public String toString() { - return "HttpHeaders::getFirst"; - } - }; private static final Log log = LogFactory.getLog(TraceWebFilter.class); @@ -89,9 +77,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { Tracer tracer; - HttpServerHandler handler; - - TraceContext.Extractor extractor; + HttpServerHandler handler; SleuthWebProperties webProperties; @@ -104,11 +90,10 @@ public final class TraceWebFilter implements WebFilter, Ordered { } @SuppressWarnings("unchecked") - HttpServerHandler handler() { + HttpServerHandler handler() { if (this.handler == null) { - this.handler = HttpServerHandler.create( - this.beanFactory.getBean(HttpTracing.class), - new TraceWebFilter.HttpAdapter()); + this.handler = HttpServerHandler + .create(this.beanFactory.getBean(HttpTracing.class)); } return this.handler; } @@ -120,14 +105,6 @@ public final class TraceWebFilter implements WebFilter, Ordered { return this.tracer; } - TraceContext.Extractor extractor() { - if (this.extractor == null) { - this.extractor = this.beanFactory.getBean(HttpTracing.class).tracing() - .propagation().extractor(GETTER); - } - return this.extractor; - } - SleuthWebProperties sleuthWebProperties() { if (this.webProperties == null) { this.webProperties = this.beanFactory.getBean(SleuthWebProperties.class); @@ -163,9 +140,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { final Span attrSpan; - final HttpServerHandler handler; - - final TraceContext.Extractor extractor; + final HttpServerHandler handler; final AtomicBoolean initialSpanAlreadyRemoved = new AtomicBoolean(); @@ -175,7 +150,6 @@ public final class TraceWebFilter implements WebFilter, Ordered { boolean initialTracePresent, TraceWebFilter parent) { super(source); this.tracer = parent.tracer(); - this.extractor = parent.extractor(); this.handler = parent.handler(); this.exchange = exchange; this.attrSpan = exchange.getAttribute(TRACE_REQUEST_ATTR); @@ -214,9 +188,8 @@ public final class TraceWebFilter implements WebFilter, Ordered { } } else { - span = this.handler.handleReceive(this.extractor, - this.exchange.getRequest().getHeaders(), - this.exchange.getRequest()); + span = this.handler.handleReceive( + new WrappedRequest(this.exchange.getRequest())); if (log.isDebugEnabled()) { log.debug("Handled receive of span " + span); } @@ -236,7 +209,7 @@ public final class TraceWebFilter implements WebFilter, Ordered { final ServerWebExchange exchange; - final HttpServerHandler handler; + final HttpServerHandler handler; WebFilterTraceSubscriber(CoreSubscriber actual, Context context, Span span, MonoWebFilterTrace parent) { @@ -284,10 +257,10 @@ public final class TraceWebFilter implements WebFilter, Ordered { String httpRoute = pattern != null ? pattern.toString() : ""; addResponseTagsForSpanWithoutParent(this.exchange, this.exchange.getResponse(), this.span); - DecoratedServerHttpResponse delegate = new DecoratedServerHttpResponse( + WrappedResponse response = new WrappedResponse( this.exchange.getResponse(), this.exchange.getRequest().getMethodValue(), httpRoute); - this.handler.handleSend(delegate, t, this.span); + this.handler.handleSend(response, t, this.span); if (log.isDebugEnabled()) { log.debug("Handled send of " + this.span); } @@ -339,60 +312,84 @@ public final class TraceWebFilter implements WebFilter, Ordered { } - static final class DecoratedServerHttpResponse extends ServerHttpResponseDecorator { + static final class WrappedRequest extends HttpServerRequest { + + final ServerHttpRequest delegate; + + WrappedRequest(ServerHttpRequest delegate) { + this.delegate = delegate; + } + + @Override + public ServerHttpRequest unwrap() { + return delegate; + } + + @Override + public boolean parseClientIpAndPort(Span span) { + InetSocketAddress addr = delegate.getRemoteAddress(); + if (addr == null) { + return false; + } + return span.remoteIpAndPort(addr.getAddress().getHostAddress(), + addr.getPort()); + } + + @Override + public String method() { + return delegate.getMethodValue(); + } + + @Override + public String path() { + return delegate.getPath().toString(); + } + + @Override + public String url() { + return delegate.getURI().toString(); + } + + @Override + public String header(String name) { + return delegate.getHeaders().getFirst(name); + } + + } + + static final class WrappedResponse extends HttpServerResponse { + + final ServerHttpResponse delegate; final String method; final String httpRoute; - DecoratedServerHttpResponse(ServerHttpResponse delegate, String method, - String httpRoute) { - super(delegate); + WrappedResponse(ServerHttpResponse resp, String method, String httpRoute) { + this.delegate = resp; this.method = method; this.httpRoute = httpRoute; } - } - - static final class HttpAdapter - extends brave.http.HttpServerAdapter { - @Override - public String method(ServerHttpRequest request) { - return request.getMethodValue(); + public String method() { + return method; } @Override - public String url(ServerHttpRequest request) { - return request.getURI().toString(); + public String route() { + return httpRoute; } @Override - public String requestHeader(ServerHttpRequest request, String name) { - Object result = request.getHeaders().getFirst(name); - return result != null ? result.toString() : null; + public ServerHttpResponse unwrap() { + return delegate; } @Override - public Integer statusCode(ServerHttpResponse response) { - return response.getStatusCode() != null ? response.getStatusCode().value() - : null; - } - - @Override - public String methodFromResponse(ServerHttpResponse response) { - if (response instanceof DecoratedServerHttpResponse) { - return ((DecoratedServerHttpResponse) response).method; - } - return null; - } - - @Override - public String route(ServerHttpResponse response) { - if (response instanceof DecoratedServerHttpResponse) { - return ((DecoratedServerHttpResponse) response).httpRoute; - } - return null; + public int statusCode() { + return delegate.getStatusCode() != null ? delegate.getStatusCode().value() + : 0; } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java index afb225075..0bd4e30e2 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java @@ -16,13 +16,13 @@ package org.springframework.cloud.sleuth.instrument.zuul; +import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import brave.Span; import brave.Tracer; import brave.http.HttpServerHandler; import brave.http.HttpTracing; -import brave.servlet.HttpServletAdapter; import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.context.RequestContext; import org.apache.commons.logging.Log; @@ -40,12 +40,12 @@ class TracePostZuulFilter extends ZuulFilter { private static final Log log = LogFactory.getLog(TracePostZuulFilter.class); - private final HttpServerHandler handler; + final HttpServerHandler handler; - private final Tracer tracer; + final Tracer tracer; TracePostZuulFilter(HttpTracing httpTracing) { - this.handler = HttpServerHandler.create(httpTracing, new HttpServletAdapter()); + this.handler = HttpServerHandler.create(httpTracing); this.tracer = httpTracing.tracing().tracer(); } @@ -69,10 +69,13 @@ class TracePostZuulFilter extends ZuulFilter { if (log.isDebugEnabled()) { log.debug("Marking current span as handled"); } - HttpServletResponse response = RequestContext.getCurrentContext().getResponse(); + HttpServletRequest req = RequestContext.getCurrentContext().getRequest(); + HttpServletResponse resp = RequestContext.getCurrentContext().getResponse(); + HttpServerResponse request = resp != null ? new HttpServerResponse(req, resp) + : null; Throwable exception = RequestContext.getCurrentContext().getThrowable(); Span currentSpan = this.tracer.currentSpan(); - this.handler.handleSend(response, exception, currentSpan); + this.handler.handleSend(request, exception, currentSpan); if (log.isDebugEnabled()) { log.debug("Handled send of " + currentSpan); } @@ -89,4 +92,41 @@ class TracePostZuulFilter extends ZuulFilter { return 0; } + // copy/paste for now https://github.com/openzipkin/brave/issues/1064 + static final class HttpServerResponse extends brave.http.HttpServerResponse { + + final HttpServletResponse delegate; + + final String method; + + final String httpRoute; + + HttpServerResponse(HttpServletRequest req, HttpServletResponse resp) { + this.delegate = resp; + this.method = req.getMethod(); + this.httpRoute = (String) req.getAttribute("http.route"); + } + + @Override + public String method() { + return method; + } + + @Override + public String route() { + return httpRoute; + } + + @Override + public HttpServletResponse unwrap() { + return delegate; + } + + @Override + public int statusCode() { + return delegate.getStatus(); + } + + } + }