Skip to content

Commit 4faca0c

Browse files
committed
Merge branch 'master' of github.com:DataDog/dd-trace-java into labbati/rebrand-priority-sampling
2 parents 19e9fb8 + 799db65 commit 4faca0c

32 files changed

Lines changed: 490 additions & 264 deletions

File tree

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/HttpServerDecorator.java

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,26 @@
44
import datadog.trace.api.DDSpanTypes;
55
import io.opentracing.Span;
66
import io.opentracing.tag.Tags;
7+
import java.net.URI;
8+
import java.util.regex.Pattern;
9+
import lombok.extern.slf4j.Slf4j;
710

8-
public abstract class HttpServerDecorator<REQUEST, RESPONSE> extends ServerDecorator {
11+
@Slf4j
12+
public abstract class HttpServerDecorator<REQUEST, CONNECTION, RESPONSE> extends ServerDecorator {
13+
// Source: https://www.regextester.com/22
14+
private static final Pattern VALID_IPV4_ADDRESS =
15+
Pattern.compile(
16+
"^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$");
917

1018
protected abstract String method(REQUEST request);
1119

12-
protected abstract String url(REQUEST request);
20+
protected abstract URI url(REQUEST request);
1321

14-
protected abstract String hostname(REQUEST request);
22+
protected abstract String peerHostname(CONNECTION connection);
1523

16-
protected abstract Integer port(REQUEST request);
24+
protected abstract String peerHostIP(CONNECTION connection);
25+
26+
protected abstract Integer peerPort(CONNECTION connection);
1727

1828
protected abstract Integer status(RESPONSE response);
1929

@@ -31,14 +41,49 @@ public Span onRequest(final Span span, final REQUEST request) {
3141
assert span != null;
3242
if (request != null) {
3343
Tags.HTTP_METHOD.set(span, method(request));
34-
Tags.HTTP_URL.set(span, url(request));
35-
Tags.PEER_HOSTNAME.set(span, hostname(request));
36-
Tags.PEER_PORT.set(span, port(request));
44+
45+
try {
46+
final URI url = url(request);
47+
final StringBuilder urlNoParams = new StringBuilder(url.getScheme());
48+
urlNoParams.append("://");
49+
urlNoParams.append(url.getHost());
50+
if (url.getPort() > 0 && url.getPort() != 80 && url.getPort() != 443) {
51+
urlNoParams.append(":");
52+
urlNoParams.append(url.getPort());
53+
}
54+
final String path = url.getPath();
55+
if (path.isEmpty()) {
56+
urlNoParams.append("/");
57+
} else {
58+
urlNoParams.append(path);
59+
}
60+
61+
Tags.HTTP_URL.set(span, urlNoParams.toString());
62+
} catch (final Exception e) {
63+
log.debug("Error tagging url", e);
64+
}
3765
// TODO set resource name from URL.
3866
}
3967
return span;
4068
}
4169

70+
public Span onConnection(final Span span, final CONNECTION connection) {
71+
assert span != null;
72+
if (connection != null) {
73+
Tags.PEER_HOSTNAME.set(span, peerHostname(connection));
74+
final String ip = peerHostIP(connection);
75+
if (ip != null) {
76+
if (VALID_IPV4_ADDRESS.matcher(ip).matches()) {
77+
Tags.PEER_HOST_IPV4.set(span, ip);
78+
} else if (ip.contains(":")) {
79+
Tags.PEER_HOST_IPV6.set(span, ip);
80+
}
81+
}
82+
Tags.PEER_PORT.set(span, peerPort(connection));
83+
}
84+
return span;
85+
}
86+
4287
public Span onResponse(final Span span, final RESPONSE response) {
4388
assert span != null;
4489
if (response != null) {

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpServerDecoratorTest.groovy

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package datadog.trace.agent.decorator
22

3+
import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
4+
35
import datadog.trace.api.Config
46
import io.opentracing.Span
57
import io.opentracing.tag.Tags
68

7-
import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
8-
99
class HttpServerDecoratorTest extends ServerDecoratorTest {
1010

1111
def span = Mock(Span)
@@ -20,14 +20,45 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
2020
then:
2121
if (req) {
2222
1 * span.setTag(Tags.HTTP_METHOD.key, "test-method")
23-
1 * span.setTag(Tags.HTTP_URL.key, "test-url")
23+
1 * span.setTag(Tags.HTTP_URL.key, url)
24+
}
25+
0 * _
26+
27+
where:
28+
req | url
29+
null | _
30+
[method: "test-method", url: URI.create("http://test-url?some=query")] | "http://test-url/"
31+
[method: "test-method", url: URI.create("http://a:80/")] | "http://a/"
32+
[method: "test-method", url: URI.create("https://10.0.0.1:443")] | "https://10.0.0.1/"
33+
[method: "test-method", url: URI.create("https://localhost:0/1/")] | "https://localhost/1/"
34+
[method: "test-method", url: URI.create("http://123:8080/some/path")] | "http://123:8080/some/path"
35+
}
36+
37+
def "test onConnection"() {
38+
setup:
39+
def decorator = newDecorator()
40+
41+
when:
42+
decorator.onConnection(span, conn)
43+
44+
then:
45+
if (conn) {
2446
1 * span.setTag(Tags.PEER_HOSTNAME.key, "test-host")
2547
1 * span.setTag(Tags.PEER_PORT.key, 555)
48+
if (ipv4) {
49+
1 * span.setTag(Tags.PEER_HOST_IPV4.key, "10.0.0.1")
50+
} else if (ipv4 != null) {
51+
1 * span.setTag(Tags.PEER_HOST_IPV6.key, "3ffe:1900:4545:3:200:f8ff:fe21:67cf")
52+
}
2653
}
2754
0 * _
2855

2956
where:
30-
req << [null, [method: "test-method", url: "test-url", host: "test-host", port: 555]]
57+
ipv4 | conn
58+
null | null
59+
null | [host: "test-host", ip: null, port: 555]
60+
true | [host: "test-host", ip: "10.0.0.1", port: 555]
61+
false | [host: "test-host", ip: "3ffe:1900:4545:3:200:f8ff:fe21:67cf", port: 555]
3162
}
3263

3364
def "test onResponse"() {
@@ -81,7 +112,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
81112

82113
@Override
83114
def newDecorator() {
84-
return new HttpServerDecorator<Map, Map>() {
115+
return new HttpServerDecorator<Map, Map, Map>() {
85116
@Override
86117
protected String[] instrumentationNames() {
87118
return ["test1", "test2"]
@@ -98,17 +129,22 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
98129
}
99130

100131
@Override
101-
protected String url(Map m) {
132+
protected URI url(Map m) {
102133
return m.url
103134
}
104135

105136
@Override
106-
protected String hostname(Map m) {
137+
protected String peerHostname(Map m) {
107138
return m.host
108139
}
109140

110141
@Override
111-
protected Integer port(Map m) {
142+
protected String peerHostIP(Map m) {
143+
return m.ip
144+
}
145+
146+
@Override
147+
protected Integer peerPort(Map m) {
112148
return m.port
113149
}
114150

dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,6 @@ class LagomTest extends AgentTestRunner {
6969
"$Tags.HTTP_STATUS.key" 101
7070
"$Tags.HTTP_URL.key" "ws://localhost:${server.port()}/echo"
7171
"$Tags.HTTP_METHOD.key" "GET"
72-
"$Tags.PEER_HOSTNAME.key" "localhost"
73-
"$Tags.PEER_PORT.key" server.port()
7472
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
7573
"$Tags.COMPONENT.key" "akka-http-server"
7674
}
@@ -109,8 +107,6 @@ class LagomTest extends AgentTestRunner {
109107
"$Tags.HTTP_STATUS.key" 500
110108
"$Tags.HTTP_URL.key" "ws://localhost:${server.port()}/error"
111109
"$Tags.HTTP_METHOD.key" "GET"
112-
"$Tags.PEER_HOSTNAME.key" "localhost"
113-
"$Tags.PEER_PORT.key" server.port()
114110
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
115111
"$Tags.COMPONENT.key" "akka-http-server"
116112
"$Tags.ERROR.key" true

dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
import akka.http.scaladsl.model.HttpRequest;
44
import akka.http.scaladsl.model.HttpResponse;
55
import datadog.trace.agent.decorator.HttpServerDecorator;
6+
import java.net.URI;
67

7-
public class AkkaHttpServerDecorator extends HttpServerDecorator<HttpRequest, HttpResponse> {
8+
public class AkkaHttpServerDecorator
9+
extends HttpServerDecorator<HttpRequest, HttpRequest, HttpResponse> {
810
public static final AkkaHttpServerDecorator DECORATE = new AkkaHttpServerDecorator();
911

1012
@Override
@@ -23,18 +25,23 @@ protected String method(final HttpRequest httpRequest) {
2325
}
2426

2527
@Override
26-
protected String url(final HttpRequest httpRequest) {
27-
return httpRequest.uri().toString();
28+
protected URI url(final HttpRequest httpRequest) {
29+
return URI.create(httpRequest.uri().toString());
2830
}
2931

3032
@Override
31-
protected String hostname(final HttpRequest httpRequest) {
32-
return httpRequest.getUri().host().address();
33+
protected String peerHostname(final HttpRequest httpRequest) {
34+
return null;
3335
}
3436

3537
@Override
36-
protected Integer port(final HttpRequest httpRequest) {
37-
return httpRequest.getUri().port();
38+
protected String peerHostIP(final HttpRequest httpRequest) {
39+
return null;
40+
}
41+
42+
@Override
43+
protected Integer peerPort(final HttpRequest httpRequest) {
44+
return null;
3845
}
3946

4047
@Override

dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ public static Scope createSpan(final HttpRequest request) {
108108
.startActive(false);
109109

110110
DECORATE.afterStart(scope.span());
111+
DECORATE.onConnection(scope.span(), request);
111112
DECORATE.onRequest(scope.span(), request);
112113

113114
if (scope instanceof TraceScope) {

dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ class AkkaHttpServerInstrumentationTest extends AgentTestRunner {
5555
"$Tags.HTTP_STATUS.key" 200
5656
"$Tags.HTTP_URL.key" "http://localhost:$port/test"
5757
"$Tags.HTTP_METHOD.key" "GET"
58-
"$Tags.PEER_HOSTNAME.key" "localhost"
59-
"$Tags.PEER_PORT.key" port
6058
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
6159
"$Tags.COMPONENT.key" "akka-http-server"
6260
}
@@ -98,8 +96,6 @@ class AkkaHttpServerInstrumentationTest extends AgentTestRunner {
9896
"$Tags.HTTP_STATUS.key" 500
9997
"$Tags.HTTP_URL.key" "http://localhost:$port/$endpoint"
10098
"$Tags.HTTP_METHOD.key" "GET"
101-
"$Tags.PEER_HOSTNAME.key" "localhost"
102-
"$Tags.PEER_PORT.key" port
10399
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
104100
"$Tags.COMPONENT.key" "akka-http-server"
105101
errorTags RuntimeException, errorMessage
@@ -139,8 +135,6 @@ class AkkaHttpServerInstrumentationTest extends AgentTestRunner {
139135
"$Tags.HTTP_STATUS.key" 500
140136
"$Tags.HTTP_URL.key" "http://localhost:$port/server-error"
141137
"$Tags.HTTP_METHOD.key" "GET"
142-
"$Tags.PEER_HOSTNAME.key" "localhost"
143-
"$Tags.PEER_PORT.key" port
144138
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
145139
"$Tags.COMPONENT.key" "akka-http-server"
146140
"$Tags.ERROR.key" true
@@ -179,8 +173,6 @@ class AkkaHttpServerInstrumentationTest extends AgentTestRunner {
179173
"$Tags.HTTP_STATUS.key" 404
180174
"$Tags.HTTP_URL.key" "http://localhost:$port/not-found"
181175
"$Tags.HTTP_METHOD.key" "GET"
182-
"$Tags.PEER_HOSTNAME.key" "localhost"
183-
"$Tags.PEER_PORT.key" port
184176
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
185177
"$Tags.COMPONENT.key" "akka-http-server"
186178
}

dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyDecorator.java

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22

33
import datadog.trace.agent.decorator.HttpServerDecorator;
44
import io.opentracing.Span;
5+
import java.net.URI;
56
import javax.servlet.http.HttpServletRequest;
67
import javax.servlet.http.HttpServletResponse;
78

8-
public class JettyDecorator extends HttpServerDecorator<HttpServletRequest, HttpServletResponse> {
9+
public class JettyDecorator
10+
extends HttpServerDecorator<HttpServletRequest, HttpServletRequest, HttpServletResponse> {
911
public static final JettyDecorator DECORATE = new JettyDecorator();
1012

1113
@Override
@@ -24,18 +26,23 @@ protected String method(final HttpServletRequest httpServletRequest) {
2426
}
2527

2628
@Override
27-
protected String url(final HttpServletRequest httpServletRequest) {
28-
return httpServletRequest.getRequestURL().toString();
29+
protected URI url(final HttpServletRequest httpServletRequest) {
30+
return URI.create(httpServletRequest.getRequestURL().toString());
2931
}
3032

3133
@Override
32-
protected String hostname(final HttpServletRequest httpServletRequest) {
33-
return httpServletRequest.getServerName();
34+
protected String peerHostname(final HttpServletRequest httpServletRequest) {
35+
return httpServletRequest.getRemoteHost();
3436
}
3537

3638
@Override
37-
protected Integer port(final HttpServletRequest httpServletRequest) {
38-
return httpServletRequest.getServerPort();
39+
protected String peerHostIP(final HttpServletRequest httpServletRequest) {
40+
return httpServletRequest.getRemoteAddr();
41+
}
42+
43+
@Override
44+
protected Integer peerPort(final HttpServletRequest httpServletRequest) {
45+
return httpServletRequest.getRemotePort();
3946
}
4047

4148
@Override

dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyHandlerAdvice.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ public static Scope startSpan(
3636
.withTag("span.origin.type", source.getClass().getName())
3737
.startActive(false);
3838

39-
DECORATE.afterStart(scope.span());
40-
DECORATE.onRequest(scope.span(), req);
39+
final Span span = scope.span();
40+
DECORATE.afterStart(span);
41+
DECORATE.onConnection(span, req);
42+
DECORATE.onRequest(span, req);
4143
final String resourceName = req.getMethod() + " " + source.getClass().getName();
42-
scope.span().setTag(DDTags.RESOURCE_NAME, resourceName);
44+
span.setTag(DDTags.RESOURCE_NAME, resourceName);
4345

4446
if (scope instanceof TraceScope) {
4547
((TraceScope) scope).setAsyncPropagation(true);

dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyHandlerTest.groovy

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,9 @@ class JettyHandlerTest extends AgentTestRunner {
6969
"component" "jetty-handler"
7070
"span.origin.type" handler.class.name
7171
"http.status_code" 200
72-
"peer.hostname" "localhost"
73-
"peer.port" port
72+
"peer.hostname" "127.0.0.1"
73+
"peer.ipv4" "127.0.0.1"
74+
"peer.port" Integer
7475
defaultTags()
7576
}
7677
}
@@ -165,14 +166,16 @@ class JettyHandlerTest extends AgentTestRunner {
165166
"component" "jetty-handler"
166167
"span.origin.type" handler.class.name
167168
"http.status_code" 500
168-
"peer.hostname" "localhost"
169-
"peer.port" port
169+
"peer.hostname" "127.0.0.1"
170+
"peer.ipv4" "127.0.0.1"
171+
"peer.port" Integer
170172
errorTags RuntimeException
171173
defaultTags()
172174
}
173175
}
174176
}
175177
if (errorHandlerCalled.get()) {
178+
// FIXME: This doesn't ever seem to be called.
176179
trace(1, 1) {
177180
span(0) {
178181
serviceName "unnamed-java-app"
@@ -188,8 +191,9 @@ class JettyHandlerTest extends AgentTestRunner {
188191
"component" "jetty-handler"
189192
"span.origin.type" handler.class.name
190193
"http.status_code" 500
191-
"peer.hostname" "localhost"
192-
"peer.port" port
194+
"peer.hostname" "127.0.0.1"
195+
"peer.ipv4" "127.0.0.1"
196+
"peer.port" Integer
193197
"error" true
194198
defaultTags()
195199
}

0 commit comments

Comments
 (0)