diff --git a/instrumentation/http-tests/src/main/java/brave/http/ITHttpServer.java b/instrumentation/http-tests/src/main/java/brave/http/ITHttpServer.java index 0b9e2c2a2e..7f05db468e 100644 --- a/instrumentation/http-tests/src/main/java/brave/http/ITHttpServer.java +++ b/instrumentation/http-tests/src/main/java/brave/http/ITHttpServer.java @@ -5,6 +5,8 @@ import brave.sampler.Sampler; import java.io.IOException; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; @@ -219,6 +221,41 @@ public void request(HttpAdapter adapter, Req req, SpanCustomizer c assertReportedTagsInclude("context.visible", "true"); } + @Test + public void supportsHttpTemplate() throws Exception { + // We don't know if we can read the template from the request or the response, so try both + httpTracing = httpTracing.toBuilder().serverParser(new HttpServerParser() { + @Override + public void request(HttpAdapter adapter, Req req, SpanCustomizer customizer) { + super.request(adapter, req, customizer); + String template = adapter.templateFromRequest(req); + if (template != null) customizer.name(template); + } + + @Override public void response(HttpAdapter adapter, Resp res, Throwable error, + SpanCustomizer customizer) { + super.response(adapter, res, error, customizer); + String template = adapter.templateFromResponse(res); + if (template != null) customizer.name(template); + } + }).build(); + init(); + + get("/items/1?foo"); + get("/items/2?bar"); + + assertThat(spans).extracting(s -> s.tags().get("http.path")) + .containsExactly("/items/1", "/items/2"); + + Set templates = spans.stream() + .map(Span::name) + .collect(Collectors.toSet()); + + assertThat(templates).hasSize(1); + assertThat(templates.iterator().next()) + .contains("items"); + } + @Test public void addsStatusCode_badRequest() throws Exception { try { diff --git a/instrumentation/http/src/main/java/brave/http/HttpAdapter.java b/instrumentation/http/src/main/java/brave/http/HttpAdapter.java index 9e0792e4ac..d73eabae45 100644 --- a/instrumentation/http/src/main/java/brave/http/HttpAdapter.java +++ b/instrumentation/http/src/main/java/brave/http/HttpAdapter.java @@ -44,6 +44,26 @@ public abstract class HttpAdapter { */ @Nullable public abstract Integer statusCode(Resp response); + /** + * An expression representing an application endpoint, used to group similar requests together. + * + *

For example, the template "/products/{key}", would match "/products/1" and "/products/2". + * There is no format required for the encoding, as it is sometimes application defined. The + * important part is that the value namespace is low cardinality. + * + *

Conventionally associated with the key "http.template" + */ + // TODO: backfill definition of http.template in the api project + @Nullable public String templateFromRequest(Req request) { + return null; + } + + /** Like {@link #templateFromRequest}, used when the template isn't visible until the response. */ + // NOTE: FromResponse suffix is needed as Req and Resp are generic params which equate to Object + @Nullable public String templateFromResponse(Resp response) { + return null; + } + HttpAdapter() { } } diff --git a/instrumentation/servlet/src/main/java/brave/servlet/HttpServletAdapter.java b/instrumentation/servlet/src/main/java/brave/servlet/HttpServletAdapter.java index edc3106ece..b21db88778 100644 --- a/instrumentation/servlet/src/main/java/brave/servlet/HttpServletAdapter.java +++ b/instrumentation/servlet/src/main/java/brave/servlet/HttpServletAdapter.java @@ -7,8 +7,7 @@ /** This can also parse the remote IP of the client. */ // public for others like sparkjava to use -public final class HttpServletAdapter - extends HttpServerAdapter { +public class HttpServletAdapter extends HttpServerAdapter { final ServletRuntime servlet = ServletRuntime.get(); /** diff --git a/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingAsyncHandlerInterceptor.java b/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingAsyncHandlerInterceptor.java index b140b5e3c6..b4600d4b99 100644 --- a/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingAsyncHandlerInterceptor.java +++ b/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingAsyncHandlerInterceptor.java @@ -22,21 +22,20 @@ public static AsyncHandlerInterceptor create(HttpTracing httpTracing) { return new TracingAsyncHandlerInterceptor(httpTracing); } - final HandlerInterceptor delegate; + final TracingHandlerInterceptor delegate; @Autowired TracingAsyncHandlerInterceptor(HttpTracing httpTracing) { // internal - delegate = TracingHandlerInterceptor.create(httpTracing); + delegate = new TracingHandlerInterceptor(httpTracing); } @Override - public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object o) - throws Exception { + public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object o) { return delegate.preHandle(request, response, o); } @Override - public void afterCompletion(HttpServletRequest request, HttpServletResponse response, - Object o, Exception ex) throws Exception { + public void afterCompletion(HttpServletRequest request, HttpServletResponse response, Object o, + Exception ex) { delegate.afterCompletion(request, response, o, ex); } } diff --git a/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingHandlerInterceptor.java b/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingHandlerInterceptor.java index 5628add5f4..52aca4f2d3 100644 --- a/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingHandlerInterceptor.java +++ b/instrumentation/spring-webmvc/src/main/java/brave/spring/webmvc/TracingHandlerInterceptor.java @@ -8,7 +8,6 @@ import brave.http.HttpTracing; import brave.propagation.Propagation; import brave.propagation.TraceContext; -import brave.servlet.HttpServletAdapter; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.springframework.beans.factory.annotation.Autowired; @@ -42,7 +41,7 @@ public static HandlerInterceptor create(HttpTracing httpTracing) { @Autowired TracingHandlerInterceptor(HttpTracing httpTracing) { // internal tracer = httpTracing.tracing().tracer(); - handler = HttpServerHandler.create(httpTracing, new HttpServletAdapter()); + handler = HttpServerHandler.create(httpTracing, new WebMVCAdapter()); extractor = httpTracing.tracing().propagation().extractor(GETTER); } @@ -59,7 +58,7 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons @Override public void postHandle(HttpServletRequest request, HttpServletResponse response, Object handler, - ModelAndView modelAndView) throws Exception { + ModelAndView modelAndView) { } @Override diff --git a/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingAsyncHandlerInterceptor.java b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingAsyncHandlerInterceptor.java index 1e47599e45..5b17307eb5 100644 --- a/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingAsyncHandlerInterceptor.java +++ b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingAsyncHandlerInterceptor.java @@ -15,6 +15,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.servlet.AsyncHandlerInterceptor; @@ -69,6 +70,11 @@ public Callable> disconnectAsync() { throw new IOException(); }; } + + @RequestMapping(value = "/items/{itemId}") + public ResponseEntity items(@PathVariable String itemId) throws IOException { + return new ResponseEntity<>(HttpStatus.OK); + } } @Configuration diff --git a/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingHandlerInterceptor.java b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingHandlerInterceptor.java index 10670bc310..d51910f8d4 100644 --- a/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingHandlerInterceptor.java +++ b/instrumentation/spring-webmvc/src/test/java/brave/spring/webmvc/ITTracingHandlerInterceptor.java @@ -15,6 +15,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.servlet.DispatcherServlet; @@ -69,6 +70,11 @@ public Callable> disconnectAsync() throws IOException { throw new IOException(); }; } + + @RequestMapping(value = "/items/{itemId}") + public ResponseEntity items(@PathVariable String itemId) throws IOException { + return new ResponseEntity<>(HttpStatus.OK); + } } @Configuration