diff --git a/instrumentation/benchmarks/src/main/java/brave/jersey/server/FakeExtendedUriInfo.java b/instrumentation/benchmarks/src/main/java/brave/jersey/server/FakeExtendedUriInfo.java new file mode 100644 index 0000000000..0b0603c725 --- /dev/null +++ b/instrumentation/benchmarks/src/main/java/brave/jersey/server/FakeExtendedUriInfo.java @@ -0,0 +1,139 @@ +package brave.jersey.server; + +import java.net.URI; +import java.util.List; +import java.util.regex.MatchResult; +import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.core.PathSegment; +import javax.ws.rs.core.UriBuilder; +import org.glassfish.jersey.server.ExtendedUriInfo; +import org.glassfish.jersey.server.model.Resource; +import org.glassfish.jersey.server.model.ResourceMethod; +import org.glassfish.jersey.server.model.RuntimeResource; +import org.glassfish.jersey.uri.UriTemplate; + +class FakeExtendedUriInfo implements ExtendedUriInfo { + final URI baseURI; + final List matchedTemplates; + + FakeExtendedUriInfo(URI baseURI, List matchedTemplates) { + this.baseURI = baseURI; + this.matchedTemplates = matchedTemplates; + } + + @Override public Throwable getMappedThrowable() { + return null; + } + + @Override public List getMatchedResults() { + return null; + } + + @Override public List getMatchedTemplates() { + return matchedTemplates; + } + + @Override public List getPathSegments(String name) { + return null; + } + + @Override public List getPathSegments(String name, boolean decode) { + return null; + } + + @Override public List getMatchedRuntimeResources() { + return null; + } + + @Override public ResourceMethod getMatchedResourceMethod() { + return null; + } + + @Override public Resource getMatchedModelResource() { + return null; + } + + @Override public List getMatchedResourceLocators() { + return null; + } + + @Override public List getLocatorSubResources() { + return null; + } + + @Override public String getPath() { + return null; + } + + @Override public String getPath(boolean decode) { + return null; + } + + @Override public List getPathSegments() { + return null; + } + + @Override public List getPathSegments(boolean decode) { + return null; + } + + @Override public URI getRequestUri() { + return null; + } + + @Override public UriBuilder getRequestUriBuilder() { + return null; + } + + @Override public URI getAbsolutePath() { + return null; + } + + @Override public UriBuilder getAbsolutePathBuilder() { + return null; + } + + @Override public URI getBaseUri() { + return baseURI; + } + + @Override public UriBuilder getBaseUriBuilder() { + return null; + } + + @Override public MultivaluedMap getPathParameters() { + return null; + } + + @Override public MultivaluedMap getPathParameters(boolean decode) { + return null; + } + + @Override public MultivaluedMap getQueryParameters() { + return null; + } + + @Override public MultivaluedMap getQueryParameters(boolean decode) { + return null; + } + + @Override public List getMatchedURIs() { + return null; + } + + @Override public List getMatchedURIs(boolean decode) { + return null; + } + + @Override public List getMatchedResources() { + return null; + } + + @Override public URI resolve(URI uri) { + return null; + } + + @Override public URI relativize(URI uri) { + return null; + } +} diff --git a/instrumentation/benchmarks/src/main/java/brave/jersey/server/TracingApplicationEventListenerAdapterBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/jersey/server/TracingApplicationEventListenerAdapterBenchmarks.java new file mode 100644 index 0000000000..520a437b1b --- /dev/null +++ b/instrumentation/benchmarks/src/main/java/brave/jersey/server/TracingApplicationEventListenerAdapterBenchmarks.java @@ -0,0 +1,77 @@ +/** + * Copyright 2015-2016 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.jersey.server; + +import java.net.URI; +import java.util.Arrays; +import java.util.concurrent.TimeUnit; +import org.glassfish.jersey.internal.MapPropertiesDelegate; +import org.glassfish.jersey.server.ContainerRequest; +import org.glassfish.jersey.server.ContainerResponse; +import org.glassfish.jersey.server.ExtendedUriInfo; +import org.glassfish.jersey.uri.PathTemplate; +import org.jboss.resteasy.core.ServerResponse; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +@Measurement(iterations = 5, time = 1) +@Warmup(iterations = 10, time = 1) +@Fork(3) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +@State(Scope.Thread) +public class TracingApplicationEventListenerAdapterBenchmarks { + FakeExtendedUriInfo uriInfo = new FakeExtendedUriInfo(URI.create("/"), + Arrays.asList( + new PathTemplate("/"), + new PathTemplate("/items/{itemId}"), + new PathTemplate("/"), + new PathTemplate("/nested") + ) + ); + ContainerResponse response = new ContainerResponse( + new ContainerRequest( + URI.create("/"), null, null, null, new MapPropertiesDelegate() + ) { + @Override public ExtendedUriInfo getUriInfo() { + return uriInfo; + } + }, new ServerResponse()); + + TracingApplicationEventListener.Adapter adapter = new TracingApplicationEventListener.Adapter(); + + @Benchmark public String parseRoute() { + return adapter.route(response); + } + + // Convenience main entry-point + public static void main(String[] args) throws RunnerException { + Options opt = new OptionsBuilder() + .include(".*" + TracingApplicationEventListenerAdapterBenchmarks.class.getSimpleName()) + .build(); + + new Runner(opt).run(); + } +} 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 3c632ec54a..26d53ee7e4 100644 --- a/instrumentation/http-tests/src/main/java/brave/http/ITHttpServer.java +++ b/instrumentation/http-tests/src/main/java/brave/http/ITHttpServer.java @@ -264,7 +264,9 @@ private void routeRequestsMatchPrefix(String prefix) throws Exception { Set routes = new LinkedHashSet<>(Arrays.asList(span1.name(), span2.name())); assertThat(routes).hasSize(1); assertThat(routes.iterator().next()) - .startsWith(prefix); + .startsWith(prefix) + .doesNotEndWith("/") // no trailing slashes + .doesNotContain("//"); // no duplicate slashes } final HttpServerParser nameIsRoutePlusUrl = new HttpServerParser() { diff --git a/instrumentation/jersey-server/src/main/java/brave/jersey/server/TracingApplicationEventListener.java b/instrumentation/jersey-server/src/main/java/brave/jersey/server/TracingApplicationEventListener.java index af0a13882d..1c3cac882b 100644 --- a/instrumentation/jersey-server/src/main/java/brave/jersey/server/TracingApplicationEventListener.java +++ b/instrumentation/jersey-server/src/main/java/brave/jersey/server/TracingApplicationEventListener.java @@ -7,8 +7,6 @@ import brave.http.HttpTracing; import brave.propagation.Propagation.Getter; import brave.propagation.TraceContext; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; import javax.inject.Inject; import javax.ws.rs.ext.Provider; @@ -80,19 +78,32 @@ static final class Adapter extends HttpServerAdapterMatched templates are pairs of (resource path, method path). This code skips redundant + * slashes from either source caused by Path("/") or implicitly by Path(""). + */ @Override public String route(ContainerResponse response) { - final ExtendedUriInfo uriInfo = response.getRequestContext().getUriInfo(); - if (uriInfo.getMatchedTemplates().isEmpty()) return ""; - final List matchedTemplates = new ArrayList<>(uriInfo.getMatchedTemplates()); - if (matchedTemplates.size() > 1) { - Collections.reverse(matchedTemplates); + ExtendedUriInfo uriInfo = response.getRequestContext().getUriInfo(); + List templates = uriInfo.getMatchedTemplates(); + int templateCount = templates.size(); + if (templateCount == 0) return ""; + StringBuilder builder = null; // don't allocate unless you need it! + String basePath = uriInfo.getBaseUri().getPath(); + if (!"/".equals(basePath)) { // skip empty base paths + builder = new StringBuilder(basePath); } - final StringBuilder sb = new StringBuilder(); - sb.append(uriInfo.getBaseUri().getPath()); - for (UriTemplate uriTemplate : matchedTemplates) { - sb.append(uriTemplate.getTemplate()); + for (int i = templateCount - 1; i >= 0; i--) { + String template = templates.get(i).getTemplate(); + if ("/".equals(template)) continue; // skip allocation + if (builder == null) { + builder = new StringBuilder(template); + } else { + builder.append(template); + } } - return sb.toString().replaceAll("//+", "/"); + return builder != null ? builder.toString() : ""; } @Override public Integer statusCode(ContainerResponse response) { diff --git a/instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java b/instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java index 2a9f6a52a0..0448c02d17 100644 --- a/instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java +++ b/instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java @@ -2,8 +2,11 @@ import brave.jersey.server.TracingApplicationEventListener.Adapter; import java.net.URI; +import java.util.Arrays; import org.glassfish.jersey.server.ContainerRequest; +import org.glassfish.jersey.server.ContainerResponse; import org.glassfish.jersey.server.ExtendedUriInfo; +import org.glassfish.jersey.uri.PathTemplate; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -17,6 +20,7 @@ public class TracingApplicationEventListenerAdapterTest { Adapter adapter = new Adapter(); @Mock ContainerRequest request; + @Mock ContainerResponse response; @Test public void path_prefixesSlashWhenMissing() { when(request.getPath(false)).thenReturn("bar"); @@ -25,6 +29,50 @@ public class TracingApplicationEventListenerAdapterTest { .isEqualTo("/bar"); } + @Test public void route() { + when(response.getRequestContext()).thenReturn(request); + ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class); + when(request.getUriInfo()).thenReturn(uriInfo); + when(uriInfo.getBaseUri()).thenReturn(URI.create("/")); + when(uriInfo.getMatchedTemplates()).thenReturn(Arrays.asList( + new PathTemplate("/"), + new PathTemplate("/items/{itemId}") + )); + + assertThat(adapter.route(response)) + .isEqualTo("/items/{itemId}"); + } + + @Test public void route_basePath() { + when(response.getRequestContext()).thenReturn(request); + ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class); + when(request.getUriInfo()).thenReturn(uriInfo); + when(uriInfo.getBaseUri()).thenReturn(URI.create("/base")); + when(uriInfo.getMatchedTemplates()).thenReturn(Arrays.asList( + new PathTemplate("/"), + new PathTemplate("/items/{itemId}") + )); + + assertThat(adapter.route(response)) + .isEqualTo("/base/items/{itemId}"); + } + + @Test public void route_nested() { + when(response.getRequestContext()).thenReturn(request); + ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class); + when(request.getUriInfo()).thenReturn(uriInfo); + when(uriInfo.getBaseUri()).thenReturn(URI.create("/")); + when(uriInfo.getMatchedTemplates()).thenReturn(Arrays.asList( + new PathTemplate("/"), + new PathTemplate("/items/{itemId}"), + new PathTemplate("/"), + new PathTemplate("/nested") + )); + + assertThat(adapter.route(response)) + .isEqualTo("/nested/items/{itemId}"); + } + @Test public void url_derivedFromExtendedUriInfo() { ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class); when(request.getUriInfo()).thenReturn(uriInfo);