Skip to content

Commit a288137

Browse files
amarzialiamarziali
committed
Spring: avoid expensive calls to locate best matching patterns
1 parent 4d1a38c commit a288137

File tree

6 files changed

+74
-147
lines changed

6 files changed

+74
-147
lines changed

‎dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.springframework.context.ApplicationContext;
2626
import org.springframework.web.servlet.HandlerMapping;
2727
import org.springframework.web.servlet.ModelAndView;
28+
import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping;
2829

2930
@AutoService(InstrumenterModule.class)
3031
public final class DispatcherServletInstrumentation extends InstrumenterModule.Tracing
@@ -86,9 +87,16 @@ public static void afterRefresh(
8687
if (springCtx.containsBean("ddDispatcherFilter")) {
8788
final HandlerMappingResourceNameFilter filter =
8889
(HandlerMappingResourceNameFilter) springCtx.getBean("ddDispatcherFilter");
89-
if (handlerMappings != null && filter != null) {
90-
filter.setHandlerMappings(handlerMappings);
90+
boolean found = false;
91+
if (handlerMappings != null) {
92+
for (final HandlerMapping handlerMapping : handlerMappings) {
93+
if (handlerMapping instanceof RequestMappingInfoHandlerMapping) {
94+
found = true;
95+
break;
96+
}
97+
}
9198
}
99+
filter.setHasPatternMatchers(found);
92100
}
93101
}
94102
}

‎dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HandlerMappingResourceNameFilter.java‎

Lines changed: 9 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,22 @@
11
package datadog.trace.instrumentation.springweb;
22

3-
import static datadog.context.Context.root;
43
import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext;
54
import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_CONTEXT_ATTRIBUTE;
6-
import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE;
75

86
import datadog.context.Context;
97
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
108
import java.io.IOException;
11-
import java.util.List;
12-
import java.util.concurrent.CopyOnWriteArrayList;
139
import javax.servlet.FilterChain;
1410
import javax.servlet.ServletException;
1511
import javax.servlet.http.HttpServletRequest;
1612
import javax.servlet.http.HttpServletResponse;
17-
import org.slf4j.Logger;
18-
import org.slf4j.LoggerFactory;
1913
import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition;
2014
import org.springframework.core.Ordered;
2115
import org.springframework.web.filter.OncePerRequestFilter;
22-
import org.springframework.web.servlet.HandlerExecutionChain;
23-
import org.springframework.web.servlet.HandlerMapping;
24-
import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping;
2516

2617
public class HandlerMappingResourceNameFilter extends OncePerRequestFilter implements Ordered {
2718

28-
private static final Logger log = LoggerFactory.getLogger(HandlerMappingResourceNameFilter.class);
29-
private final List<HandlerMapping> handlerMappings = new CopyOnWriteArrayList<>();
19+
private boolean hasPatternMatchers = true;
3020

3121
@Override
3222
protected void doFilterInternal(
@@ -35,55 +25,22 @@ protected void doFilterInternal(
3525
final FilterChain filterChain)
3626
throws ServletException, IOException {
3727

38-
final Object contextObj = request.getAttribute(DD_CONTEXT_ATTRIBUTE);
39-
if (contextObj instanceof Context) {
28+
final Object contextObj;
29+
HttpServletRequest requestToUse = request;
30+
if (hasPatternMatchers
31+
&& (contextObj = request.getAttribute(DD_CONTEXT_ATTRIBUTE)) instanceof Context) {
4032
Context context = (Context) contextObj;
4133
AgentSpan parentSpan = spanFromContext(context);
4234
if (parentSpan != null) {
43-
PathMatchingHttpServletRequestWrapper wrappedRequest =
44-
new PathMatchingHttpServletRequestWrapper(request);
45-
try {
46-
if (findMapping(wrappedRequest)) {
47-
// Name the parent span based on the matching pattern
48-
// Let the parent span resource name be set with the attribute set in findMapping.
49-
DECORATE.onRequest(parentSpan, wrappedRequest, wrappedRequest, root());
50-
}
51-
} catch (final Exception ignored) {
52-
// mapping.getHandler() threw exception. Ignore
53-
}
35+
requestToUse = new PathMatchingHttpServletRequestWrapper(request, parentSpan);
5436
}
5537
}
5638

57-
filterChain.doFilter(request, response);
39+
filterChain.doFilter(requestToUse, response);
5840
}
5941

60-
/**
61-
* When a HandlerMapping matches a request, it sets HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE
62-
* as an attribute on the request. This attribute is read by
63-
* SpringWebHttpServerDecorator.onRequest and set as the resource name.
64-
*/
65-
private boolean findMapping(final HttpServletRequest request) throws Exception {
66-
for (final HandlerMapping mapping : handlerMappings) {
67-
final HandlerExecutionChain handler = mapping.getHandler(request);
68-
if (handler != null) {
69-
return true;
70-
}
71-
}
72-
return false;
73-
}
74-
75-
public void setHandlerMappings(final List<HandlerMapping> handlerMappings) {
76-
for (HandlerMapping handlerMapping : handlerMappings) {
77-
if (handlerMapping instanceof RequestMappingInfoHandlerMapping) {
78-
if (!this.handlerMappings.contains(handlerMapping)) {
79-
this.handlerMappings.add(handlerMapping);
80-
}
81-
} else {
82-
log.debug(
83-
"discarding handler mapping {} which won't set BEST_MATCHING_PATTERN_ATTRIBUTE",
84-
handlerMapping);
85-
}
86-
}
42+
public void setHasPatternMatchers(boolean hasPatternMatchers) {
43+
this.hasPatternMatchers = hasPatternMatchers;
8744
}
8845

8946
@Override

‎dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/PathMatchingHttpServletRequestWrapper.java‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,32 @@
11
package datadog.trace.instrumentation.springweb;
22

3-
import java.util.HashMap;
4-
import java.util.Map;
3+
import static datadog.context.Context.root;
4+
import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE;
5+
6+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
57
import javax.servlet.http.HttpServletRequest;
68
import javax.servlet.http.HttpServletRequestWrapper;
9+
import org.springframework.web.servlet.HandlerMapping;
710

811
class PathMatchingHttpServletRequestWrapper extends HttpServletRequestWrapper {
9-
private final Map<String, Object> localAttributes = new HashMap<>();
1012

11-
public PathMatchingHttpServletRequestWrapper(HttpServletRequest request) {
12-
super(request);
13-
}
13+
private final AgentSpan requestSpan;
14+
private boolean applied = false;
1415

15-
@Override
16-
public Object getAttribute(String name) {
17-
final Object ret = localAttributes.get(name);
18-
if (ret == null) {
19-
return super.getAttribute(name);
20-
}
21-
return ret;
16+
public PathMatchingHttpServletRequestWrapper(HttpServletRequest request, AgentSpan requestSpan) {
17+
super(request);
18+
this.requestSpan = requestSpan;
2219
}
2320

2421
@Override
2522
public void setAttribute(String name, Object o) {
26-
localAttributes.put(name, o);
27-
}
28-
29-
@Override
30-
public void removeAttribute(String name) {
31-
localAttributes.remove(name);
23+
super.setAttribute(name, o);
24+
try {
25+
if (!applied && o != null && HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE.equals(name)) {
26+
applied = true;
27+
DECORATE.onRequest(requestSpan, this, this, root());
28+
}
29+
} catch (Throwable ignored) {
30+
}
3231
}
3332
}

‎dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,18 @@ public static void afterRefresh(
1717
@Advice.Argument(0) final ApplicationContext springCtx,
1818
@Advice.FieldValue("handlerMappings") final List<HandlerMapping> handlerMappings) {
1919
if (springCtx.containsBean("ddDispatcherFilter")) {
20-
final datadog.trace.instrumentation.springweb6.HandlerMappingResourceNameFilter filter =
20+
final HandlerMappingResourceNameFilter filter =
2121
(HandlerMappingResourceNameFilter) springCtx.getBean("ddDispatcherFilter");
22-
if (handlerMappings != null && filter != null) {
23-
filter.setHandlerMappings(handlerMappings);
22+
boolean found = false;
23+
if (handlerMappings != null) {
24+
for (final HandlerMapping handlerMapping : handlerMappings) {
25+
if (handlerMapping.usesPathPatterns()) {
26+
found = true;
27+
break;
28+
}
29+
}
2430
}
31+
filter.setHasPatternMatchers(found);
2532
}
2633
}
2734
}

‎dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java‎

Lines changed: 9 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
package datadog.trace.instrumentation.springweb6;
22

3-
import static datadog.context.Context.root;
43
import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext;
54
import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_CONTEXT_ATTRIBUTE;
6-
import static datadog.trace.instrumentation.springweb6.SpringWebHttpServerDecorator.DECORATE;
75

86
import datadog.context.Context;
97
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
@@ -12,21 +10,13 @@
1210
import jakarta.servlet.http.HttpServletRequest;
1311
import jakarta.servlet.http.HttpServletResponse;
1412
import java.io.IOException;
15-
import java.util.List;
16-
import java.util.concurrent.CopyOnWriteArrayList;
17-
import org.slf4j.Logger;
18-
import org.slf4j.LoggerFactory;
1913
import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition;
2014
import org.springframework.core.Ordered;
2115
import org.springframework.web.filter.OncePerRequestFilter;
22-
import org.springframework.web.servlet.HandlerExecutionChain;
23-
import org.springframework.web.servlet.HandlerMapping;
24-
import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping;
2516

2617
public class HandlerMappingResourceNameFilter extends OncePerRequestFilter implements Ordered {
2718

28-
private static final Logger log = LoggerFactory.getLogger(HandlerMappingResourceNameFilter.class);
29-
private final List<HandlerMapping> handlerMappings = new CopyOnWriteArrayList<>();
19+
private boolean hasPatternMatchers = true;
3020

3121
@Override
3222
protected void doFilterInternal(
@@ -35,55 +25,22 @@ protected void doFilterInternal(
3525
final FilterChain filterChain)
3626
throws ServletException, IOException {
3727

38-
final Object contextObj = request.getAttribute(DD_CONTEXT_ATTRIBUTE);
39-
if (contextObj instanceof Context) {
28+
final Object contextObj;
29+
HttpServletRequest requestToUse = request;
30+
if (hasPatternMatchers
31+
&& (contextObj = request.getAttribute(DD_CONTEXT_ATTRIBUTE)) instanceof Context) {
4032
Context context = (Context) contextObj;
4133
AgentSpan parentSpan = spanFromContext(context);
4234
if (parentSpan != null) {
43-
PathMatchingHttpServletRequestWrapper wrappedRequest =
44-
new PathMatchingHttpServletRequestWrapper(request);
45-
try {
46-
if (findMapping(wrappedRequest)) {
47-
// Name the parent span based on the matching pattern
48-
// Let the parent span resource name be set with the attribute set in findMapping.
49-
DECORATE.onRequest(parentSpan, wrappedRequest, wrappedRequest, root());
50-
}
51-
} catch (final Exception ignored) {
52-
// mapping.getHandler() threw exception. Ignore
53-
}
35+
requestToUse = new PathMatchingHttpServletRequestWrapper(request, parentSpan);
5436
}
5537
}
5638

57-
filterChain.doFilter(request, response);
39+
filterChain.doFilter(requestToUse, response);
5840
}
5941

60-
/**
61-
* When a HandlerMapping matches a request, it sets HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE
62-
* as an attribute on the request. This attribute is read by
63-
* SpringWebHttpServerDecorator.onRequest and set as the resource name.
64-
*/
65-
private boolean findMapping(final HttpServletRequest request) throws Exception {
66-
for (final HandlerMapping mapping : handlerMappings) {
67-
final HandlerExecutionChain handler = mapping.getHandler(request);
68-
if (handler != null) {
69-
return true;
70-
}
71-
}
72-
return false;
73-
}
74-
75-
public void setHandlerMappings(final List<HandlerMapping> handlerMappings) {
76-
for (HandlerMapping handlerMapping : handlerMappings) {
77-
if (handlerMapping instanceof RequestMappingInfoHandlerMapping) {
78-
if (!this.handlerMappings.contains(handlerMapping)) {
79-
this.handlerMappings.add(handlerMapping);
80-
}
81-
} else {
82-
log.debug(
83-
"discarding handler mapping {} which won't set BEST_MATCHING_PATTERN_ATTRIBUTE",
84-
handlerMapping);
85-
}
86-
}
42+
public void setHasPatternMatchers(boolean hasPatternMatchers) {
43+
this.hasPatternMatchers = hasPatternMatchers;
8744
}
8845

8946
@Override

‎dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,32 @@
11
package datadog.trace.instrumentation.springweb6;
22

3+
import static datadog.context.Context.root;
4+
import static datadog.trace.instrumentation.springweb6.SpringWebHttpServerDecorator.DECORATE;
5+
6+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
37
import jakarta.servlet.http.HttpServletRequest;
48
import jakarta.servlet.http.HttpServletRequestWrapper;
5-
import java.util.HashMap;
6-
import java.util.Map;
9+
import org.springframework.web.servlet.HandlerMapping;
710

811
class PathMatchingHttpServletRequestWrapper extends HttpServletRequestWrapper {
9-
private final Map<String, Object> localAttributes = new HashMap<>();
1012

11-
public PathMatchingHttpServletRequestWrapper(HttpServletRequest request) {
12-
super(request);
13-
}
13+
private final AgentSpan requestSpan;
14+
private boolean applied = false;
1415

15-
@Override
16-
public Object getAttribute(String name) {
17-
final Object ret = localAttributes.get(name);
18-
if (ret == null) {
19-
return super.getAttribute(name);
20-
}
21-
return ret;
16+
public PathMatchingHttpServletRequestWrapper(HttpServletRequest request, AgentSpan requestSpan) {
17+
super(request);
18+
this.requestSpan = requestSpan;
2219
}
2320

2421
@Override
2522
public void setAttribute(String name, Object o) {
26-
localAttributes.put(name, o);
27-
}
28-
29-
@Override
30-
public void removeAttribute(String name) {
31-
localAttributes.remove(name);
23+
super.setAttribute(name, o);
24+
try {
25+
if (!applied && o != null && HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE.equals(name)) {
26+
applied = true;
27+
DECORATE.onRequest(requestSpan, this, this, root());
28+
}
29+
} catch (Throwable ignored) {
30+
}
3231
}
3332
}

0 commit comments

Comments
 (0)