Skip to content

Commit 7c39e54

Browse files
trasktrask
authored
Fix NullPointerException when uri is null (#7387)
1 parent 5ede4c8 commit 7c39e54

File tree

3 files changed

+136
-3
lines changed

3 files changed

+136
-3
lines changed

‎instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyHttpClientAttributesGetter.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ public String url(HttpClientConfig request) {
2525

2626
// use the baseUrl if it was configured
2727
String baseUrl = request.baseUrl();
28+
29+
if (uri == null) {
30+
// internally reactor netty appends "/" to the baseUrl
31+
return baseUrl.endsWith("/") ? baseUrl : baseUrl + "/";
32+
}
33+
2834
if (baseUrl != null) {
2935
if (baseUrl.endsWith("/") && uri.startsWith("/")) {
3036
baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
@@ -48,7 +54,7 @@ public String url(HttpClientConfig request) {
4854
}
4955

5056
private static boolean isAbsolute(String uri) {
51-
return uri.startsWith("http://") || uri.startsWith("https://");
57+
return uri != null && !uri.isEmpty() && !uri.startsWith("/");
5258
}
5359

5460
@Nullable

‎instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyNetClientAttributesGetter.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ private static String getHost(HttpClientConfig request) {
5656
String baseUrl = request.baseUrl();
5757
String uri = request.uri();
5858

59-
if (baseUrl != null && uri.startsWith("/")) {
59+
if (baseUrl != null && !isAbsolute(uri)) {
6060
return UrlParser.getHost(baseUrl);
6161
} else {
6262
return UrlParser.getHost(uri);
@@ -68,10 +68,14 @@ private static Integer getPort(HttpClientConfig request) {
6868
String baseUrl = request.baseUrl();
6969
String uri = request.uri();
7070

71-
if (baseUrl != null && uri.startsWith("/")) {
71+
if (baseUrl != null && !isAbsolute(uri)) {
7272
return UrlParser.getPort(baseUrl);
7373
} else {
7474
return UrlParser.getPort(uri);
7575
}
7676
}
77+
78+
private static boolean isAbsolute(String uri) {
79+
return uri != null && !uri.isEmpty() && !uri.startsWith("/");
80+
}
7781
}

‎instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyBaseUrlOnlyTest.java

+123
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0;
7+
8+
import static io.opentelemetry.api.trace.SpanKind.CLIENT;
9+
import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
10+
import static io.opentelemetry.api.trace.SpanKind.SERVER;
11+
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
12+
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
13+
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HttpFlavorValues.HTTP_1_1;
14+
import static org.assertj.core.api.Assertions.assertThat;
15+
16+
import io.opentelemetry.api.GlobalOpenTelemetry;
17+
import io.opentelemetry.api.trace.SpanBuilder;
18+
import io.opentelemetry.context.Context;
19+
import io.opentelemetry.instrumentation.test.server.http.RequestContextGetter;
20+
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
21+
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
22+
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
23+
import io.opentelemetry.testing.internal.armeria.common.HttpData;
24+
import io.opentelemetry.testing.internal.armeria.common.HttpResponse;
25+
import io.opentelemetry.testing.internal.armeria.common.HttpStatus;
26+
import io.opentelemetry.testing.internal.armeria.common.ResponseHeaders;
27+
import io.opentelemetry.testing.internal.armeria.common.ResponseHeadersBuilder;
28+
import io.opentelemetry.testing.internal.armeria.server.ServerBuilder;
29+
import io.opentelemetry.testing.internal.armeria.testing.junit5.server.ServerExtension;
30+
import org.assertj.core.api.AbstractLongAssert;
31+
import org.junit.jupiter.api.AfterAll;
32+
import org.junit.jupiter.api.BeforeAll;
33+
import org.junit.jupiter.api.Test;
34+
import org.junit.jupiter.api.extension.RegisterExtension;
35+
import reactor.netty.http.client.HttpClient;
36+
37+
class ReactorNettyBaseUrlOnlyTest {
38+
39+
@RegisterExtension
40+
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
41+
42+
static ServerExtension server;
43+
44+
@BeforeAll
45+
static void setUp() {
46+
server =
47+
new ServerExtension() {
48+
@Override
49+
protected void configure(ServerBuilder sb) {
50+
sb.service(
51+
"/base/",
52+
(ctx, req) -> {
53+
ResponseHeadersBuilder headers = ResponseHeaders.builder(HttpStatus.OK);
54+
55+
SpanBuilder span =
56+
GlobalOpenTelemetry.getTracer("test")
57+
.spanBuilder("test-http-server")
58+
.setSpanKind(SERVER)
59+
.setParent(
60+
GlobalOpenTelemetry.getPropagators()
61+
.getTextMapPropagator()
62+
.extract(Context.current(), ctx, RequestContextGetter.INSTANCE));
63+
64+
span.startSpan().end();
65+
66+
return HttpResponse.of(headers.build(), HttpData.ofAscii("Hello."));
67+
});
68+
}
69+
};
70+
server.start();
71+
}
72+
73+
@AfterAll
74+
static void tearDown() {
75+
server.stop();
76+
}
77+
78+
@Test
79+
void testSuccessfulRequest() {
80+
HttpClient httpClient = HttpClient.create();
81+
String uri = "http://localhost:" + server.httpPort() + "/base";
82+
83+
int responseCode =
84+
testing.runWithSpan(
85+
"parent",
86+
() ->
87+
httpClient
88+
.baseUrl(uri)
89+
.get()
90+
.responseSingle(
91+
(resp, content) -> {
92+
// Make sure to consume content since that's when we close the span.
93+
return content.map(unused -> resp);
94+
})
95+
.block()
96+
.status()
97+
.code());
98+
99+
assertThat(responseCode).isEqualTo(200);
100+
101+
testing.waitAndAssertTraces(
102+
trace ->
103+
trace.hasSpansSatisfyingExactlyInAnyOrder(
104+
span -> span.hasName("parent").hasKind(INTERNAL).hasNoParent(),
105+
span ->
106+
span.hasName("HTTP GET")
107+
.hasKind(CLIENT)
108+
.hasParent(trace.getSpan(0))
109+
.hasAttributesSatisfyingExactly(
110+
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
111+
equalTo(SemanticAttributes.HTTP_URL, uri + "/"),
112+
equalTo(SemanticAttributes.HTTP_FLAVOR, HTTP_1_1),
113+
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
114+
satisfies(
115+
SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH,
116+
AbstractLongAssert::isNotNegative),
117+
equalTo(SemanticAttributes.NET_PEER_NAME, "localhost"),
118+
equalTo(SemanticAttributes.NET_PEER_PORT, server.httpPort()),
119+
equalTo(SemanticAttributes.NET_SOCK_PEER_ADDR, "127.0.0.1")),
120+
span ->
121+
span.hasName("test-http-server").hasKind(SERVER).hasParent(trace.getSpan(1))));
122+
}
123+
}

0 commit comments

Comments
 (0)