Skip to content

Commit

Permalink
WIP Begins support of http.template tag
Browse files Browse the repository at this point in the history
  • Loading branch information
Adrian Cole committed Jan 31, 2018
1 parent 79755d1 commit 8c6a024
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -219,6 +221,41 @@ public <Req> void request(HttpAdapter<Req, ?> 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 <Req> void request(HttpAdapter<Req, ?> adapter, Req req, SpanCustomizer customizer) {
super.request(adapter, req, customizer);
String template = adapter.templateFromRequest(req);
if (template != null) customizer.name(template);
}

@Override public <Resp> void response(HttpAdapter<?, Resp> 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<String> 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 {
Expand Down
20 changes: 20 additions & 0 deletions instrumentation/http/src/main/java/brave/http/HttpAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,26 @@ public abstract class HttpAdapter<Req, Resp> {
*/
@Nullable public abstract Integer statusCode(Resp response);

/**
* An expression representing an application endpoint, used to group similar requests together.
*
* <p>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.
*
* <p>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() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<HttpServletRequest, HttpServletResponse> {
public class HttpServletAdapter extends HttpServerAdapter<HttpServletRequest, HttpServletResponse> {
final ServletRuntime servlet = ServletRuntime.get();

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -69,6 +70,11 @@ public Callable<ResponseEntity<Void>> disconnectAsync() {
throw new IOException();
};
}

@RequestMapping(value = "/items/{itemId}")
public ResponseEntity<Void> items(@PathVariable String itemId) throws IOException {
return new ResponseEntity<>(HttpStatus.OK);
}
}

@Configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -69,6 +70,11 @@ public Callable<ResponseEntity<Void>> disconnectAsync() throws IOException {
throw new IOException();
};
}

@RequestMapping(value = "/items/{itemId}")
public ResponseEntity<Void> items(@PathVariable String itemId) throws IOException {
return new ResponseEntity<>(HttpStatus.OK);
}
}

@Configuration
Expand Down

0 comments on commit 8c6a024

Please sign in to comment.