Skip to content

Commit a3e18b8

Browse files
author
Adrian Cole
committed
Speeds up jersey and double-checks double-slashes
1 parent db3d46e commit a3e18b8

File tree

5 files changed

+290
-13
lines changed

5 files changed

+290
-13
lines changed

‎instrumentation/benchmarks/src/main/java/brave/jersey/server/FakeExtendedUriInfo.java

+139
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package brave.jersey.server;
2+
3+
import java.net.URI;
4+
import java.util.List;
5+
import java.util.regex.MatchResult;
6+
import javax.ws.rs.core.MultivaluedMap;
7+
import javax.ws.rs.core.PathSegment;
8+
import javax.ws.rs.core.UriBuilder;
9+
import org.glassfish.jersey.server.ExtendedUriInfo;
10+
import org.glassfish.jersey.server.model.Resource;
11+
import org.glassfish.jersey.server.model.ResourceMethod;
12+
import org.glassfish.jersey.server.model.RuntimeResource;
13+
import org.glassfish.jersey.uri.UriTemplate;
14+
15+
class FakeExtendedUriInfo implements ExtendedUriInfo {
16+
final URI baseURI;
17+
final List<UriTemplate> matchedTemplates;
18+
19+
FakeExtendedUriInfo(URI baseURI, List<UriTemplate> matchedTemplates) {
20+
this.baseURI = baseURI;
21+
this.matchedTemplates = matchedTemplates;
22+
}
23+
24+
@Override public Throwable getMappedThrowable() {
25+
return null;
26+
}
27+
28+
@Override public List<MatchResult> getMatchedResults() {
29+
return null;
30+
}
31+
32+
@Override public List<UriTemplate> getMatchedTemplates() {
33+
return matchedTemplates;
34+
}
35+
36+
@Override public List<PathSegment> getPathSegments(String name) {
37+
return null;
38+
}
39+
40+
@Override public List<PathSegment> getPathSegments(String name, boolean decode) {
41+
return null;
42+
}
43+
44+
@Override public List<RuntimeResource> getMatchedRuntimeResources() {
45+
return null;
46+
}
47+
48+
@Override public ResourceMethod getMatchedResourceMethod() {
49+
return null;
50+
}
51+
52+
@Override public Resource getMatchedModelResource() {
53+
return null;
54+
}
55+
56+
@Override public List<ResourceMethod> getMatchedResourceLocators() {
57+
return null;
58+
}
59+
60+
@Override public List<Resource> getLocatorSubResources() {
61+
return null;
62+
}
63+
64+
@Override public String getPath() {
65+
return null;
66+
}
67+
68+
@Override public String getPath(boolean decode) {
69+
return null;
70+
}
71+
72+
@Override public List<PathSegment> getPathSegments() {
73+
return null;
74+
}
75+
76+
@Override public List<PathSegment> getPathSegments(boolean decode) {
77+
return null;
78+
}
79+
80+
@Override public URI getRequestUri() {
81+
return null;
82+
}
83+
84+
@Override public UriBuilder getRequestUriBuilder() {
85+
return null;
86+
}
87+
88+
@Override public URI getAbsolutePath() {
89+
return null;
90+
}
91+
92+
@Override public UriBuilder getAbsolutePathBuilder() {
93+
return null;
94+
}
95+
96+
@Override public URI getBaseUri() {
97+
return baseURI;
98+
}
99+
100+
@Override public UriBuilder getBaseUriBuilder() {
101+
return null;
102+
}
103+
104+
@Override public MultivaluedMap<String, String> getPathParameters() {
105+
return null;
106+
}
107+
108+
@Override public MultivaluedMap<String, String> getPathParameters(boolean decode) {
109+
return null;
110+
}
111+
112+
@Override public MultivaluedMap<String, String> getQueryParameters() {
113+
return null;
114+
}
115+
116+
@Override public MultivaluedMap<String, String> getQueryParameters(boolean decode) {
117+
return null;
118+
}
119+
120+
@Override public List<String> getMatchedURIs() {
121+
return null;
122+
}
123+
124+
@Override public List<String> getMatchedURIs(boolean decode) {
125+
return null;
126+
}
127+
128+
@Override public List<Object> getMatchedResources() {
129+
return null;
130+
}
131+
132+
@Override public URI resolve(URI uri) {
133+
return null;
134+
}
135+
136+
@Override public URI relativize(URI uri) {
137+
return null;
138+
}
139+
}

‎instrumentation/benchmarks/src/main/java/brave/jersey/server/TracingApplicationEventListenerAdapterBenchmarks.java

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* Copyright 2015-2016 The OpenZipkin Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package brave.jersey.server;
15+
16+
import java.net.URI;
17+
import java.util.Arrays;
18+
import java.util.concurrent.TimeUnit;
19+
import org.glassfish.jersey.internal.MapPropertiesDelegate;
20+
import org.glassfish.jersey.server.ContainerRequest;
21+
import org.glassfish.jersey.server.ContainerResponse;
22+
import org.glassfish.jersey.server.ExtendedUriInfo;
23+
import org.glassfish.jersey.uri.PathTemplate;
24+
import org.jboss.resteasy.core.ServerResponse;
25+
import org.openjdk.jmh.annotations.Benchmark;
26+
import org.openjdk.jmh.annotations.BenchmarkMode;
27+
import org.openjdk.jmh.annotations.Fork;
28+
import org.openjdk.jmh.annotations.Measurement;
29+
import org.openjdk.jmh.annotations.Mode;
30+
import org.openjdk.jmh.annotations.OutputTimeUnit;
31+
import org.openjdk.jmh.annotations.Scope;
32+
import org.openjdk.jmh.annotations.State;
33+
import org.openjdk.jmh.annotations.Warmup;
34+
import org.openjdk.jmh.runner.Runner;
35+
import org.openjdk.jmh.runner.RunnerException;
36+
import org.openjdk.jmh.runner.options.Options;
37+
import org.openjdk.jmh.runner.options.OptionsBuilder;
38+
39+
@Measurement(iterations = 5, time = 1)
40+
@Warmup(iterations = 10, time = 1)
41+
@Fork(3)
42+
@BenchmarkMode(Mode.Throughput)
43+
@OutputTimeUnit(TimeUnit.MICROSECONDS)
44+
@State(Scope.Thread)
45+
public class TracingApplicationEventListenerAdapterBenchmarks {
46+
FakeExtendedUriInfo uriInfo = new FakeExtendedUriInfo(URI.create("/"),
47+
Arrays.asList(
48+
new PathTemplate("/"),
49+
new PathTemplate("/items/{itemId}"),
50+
new PathTemplate("/"),
51+
new PathTemplate("/nested")
52+
)
53+
);
54+
ContainerResponse response = new ContainerResponse(
55+
new ContainerRequest(
56+
URI.create("/"), null, null, null, new MapPropertiesDelegate()
57+
) {
58+
@Override public ExtendedUriInfo getUriInfo() {
59+
return uriInfo;
60+
}
61+
}, new ServerResponse());
62+
63+
TracingApplicationEventListener.Adapter adapter = new TracingApplicationEventListener.Adapter();
64+
65+
@Benchmark public String parseRoute() {
66+
return adapter.route(response);
67+
}
68+
69+
// Convenience main entry-point
70+
public static void main(String[] args) throws RunnerException {
71+
Options opt = new OptionsBuilder()
72+
.include(".*" + TracingApplicationEventListenerAdapterBenchmarks.class.getSimpleName())
73+
.build();
74+
75+
new Runner(opt).run();
76+
}
77+
}

‎instrumentation/http-tests/src/main/java/brave/http/ITHttpServer.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,9 @@ private void routeRequestsMatchPrefix(String prefix) throws Exception {
264264
Set<String> routes = new LinkedHashSet<>(Arrays.asList(span1.name(), span2.name()));
265265
assertThat(routes).hasSize(1);
266266
assertThat(routes.iterator().next())
267-
.startsWith(prefix);
267+
.startsWith(prefix)
268+
.doesNotEndWith("/") // no trailing slashes
269+
.doesNotContain("//"); // no duplicate slashes
268270
}
269271

270272
final HttpServerParser nameIsRoutePlusUrl = new HttpServerParser() {

‎instrumentation/jersey-server/src/main/java/brave/jersey/server/TracingApplicationEventListener.java

+23-12
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
import brave.http.HttpTracing;
88
import brave.propagation.Propagation.Getter;
99
import brave.propagation.TraceContext;
10-
import java.util.ArrayList;
11-
import java.util.Collections;
1210
import java.util.List;
1311
import javax.inject.Inject;
1412
import javax.ws.rs.ext.Provider;
@@ -80,19 +78,32 @@ static final class Adapter extends HttpServerAdapter<ContainerRequest, Container
8078
return request.getHeaderString(name);
8179
}
8280

81+
/**
82+
* This returns the matched template as defined by a base URL and path expressions.
83+
*
84+
* <p>Matched templates are pairs of (resource path, method path). This code skips redundant
85+
* slashes from either source caused by Path("/") or implicitly by Path("").
86+
*/
8387
@Override public String route(ContainerResponse response) {
84-
final ExtendedUriInfo uriInfo = response.getRequestContext().getUriInfo();
85-
if (uriInfo.getMatchedTemplates().isEmpty()) return "";
86-
final List<UriTemplate> matchedTemplates = new ArrayList<>(uriInfo.getMatchedTemplates());
87-
if (matchedTemplates.size() > 1) {
88-
Collections.reverse(matchedTemplates);
88+
ExtendedUriInfo uriInfo = response.getRequestContext().getUriInfo();
89+
List<UriTemplate> templates = uriInfo.getMatchedTemplates();
90+
int templateCount = templates.size();
91+
if (templateCount == 0) return "";
92+
StringBuilder builder = null; // don't allocate unless you need it!
93+
String basePath = uriInfo.getBaseUri().getPath();
94+
if (!"/".equals(basePath)) { // skip empty base paths
95+
builder = new StringBuilder(basePath);
8996
}
90-
final StringBuilder sb = new StringBuilder();
91-
sb.append(uriInfo.getBaseUri().getPath());
92-
for (UriTemplate uriTemplate : matchedTemplates) {
93-
sb.append(uriTemplate.getTemplate());
97+
for (int i = templateCount - 1; i >= 0; i--) {
98+
String template = templates.get(i).getTemplate();
99+
if ("/".equals(template)) continue; // skip allocation
100+
if (builder == null) {
101+
builder = new StringBuilder(template);
102+
} else {
103+
builder.append(template);
104+
}
94105
}
95-
return sb.toString().replaceAll("//+", "/");
106+
return builder != null ? builder.toString() : "";
96107
}
97108

98109
@Override public Integer statusCode(ContainerResponse response) {

‎instrumentation/jersey-server/src/test/java/brave/jersey/server/TracingApplicationEventListenerAdapterTest.java

+48
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
import brave.jersey.server.TracingApplicationEventListener.Adapter;
44
import java.net.URI;
5+
import java.util.Arrays;
56
import org.glassfish.jersey.server.ContainerRequest;
7+
import org.glassfish.jersey.server.ContainerResponse;
68
import org.glassfish.jersey.server.ExtendedUriInfo;
9+
import org.glassfish.jersey.uri.PathTemplate;
710
import org.junit.Test;
811
import org.junit.runner.RunWith;
912
import org.mockito.Mock;
@@ -17,6 +20,7 @@
1720
public class TracingApplicationEventListenerAdapterTest {
1821
Adapter adapter = new Adapter();
1922
@Mock ContainerRequest request;
23+
@Mock ContainerResponse response;
2024

2125
@Test public void path_prefixesSlashWhenMissing() {
2226
when(request.getPath(false)).thenReturn("bar");
@@ -25,6 +29,50 @@ public class TracingApplicationEventListenerAdapterTest {
2529
.isEqualTo("/bar");
2630
}
2731

32+
@Test public void route() {
33+
when(response.getRequestContext()).thenReturn(request);
34+
ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class);
35+
when(request.getUriInfo()).thenReturn(uriInfo);
36+
when(uriInfo.getBaseUri()).thenReturn(URI.create("/"));
37+
when(uriInfo.getMatchedTemplates()).thenReturn(Arrays.asList(
38+
new PathTemplate("/"),
39+
new PathTemplate("/items/{itemId}")
40+
));
41+
42+
assertThat(adapter.route(response))
43+
.isEqualTo("/items/{itemId}");
44+
}
45+
46+
@Test public void route_basePath() {
47+
when(response.getRequestContext()).thenReturn(request);
48+
ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class);
49+
when(request.getUriInfo()).thenReturn(uriInfo);
50+
when(uriInfo.getBaseUri()).thenReturn(URI.create("/base"));
51+
when(uriInfo.getMatchedTemplates()).thenReturn(Arrays.asList(
52+
new PathTemplate("/"),
53+
new PathTemplate("/items/{itemId}")
54+
));
55+
56+
assertThat(adapter.route(response))
57+
.isEqualTo("/base/items/{itemId}");
58+
}
59+
60+
@Test public void route_nested() {
61+
when(response.getRequestContext()).thenReturn(request);
62+
ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class);
63+
when(request.getUriInfo()).thenReturn(uriInfo);
64+
when(uriInfo.getBaseUri()).thenReturn(URI.create("/"));
65+
when(uriInfo.getMatchedTemplates()).thenReturn(Arrays.asList(
66+
new PathTemplate("/"),
67+
new PathTemplate("/items/{itemId}"),
68+
new PathTemplate("/"),
69+
new PathTemplate("/nested")
70+
));
71+
72+
assertThat(adapter.route(response))
73+
.isEqualTo("/nested/items/{itemId}");
74+
}
75+
2876
@Test public void url_derivedFromExtendedUriInfo() {
2977
ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class);
3078
when(request.getUriInfo()).thenReturn(uriInfo);

0 commit comments

Comments
 (0)