Skip to content

Commit 5b99fbf

Browse files
otelbot[bot]trask
andauthored
Code review sweep (run 24980313637) (open-telemetry#18335)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent 848ea4c commit 5b99fbf

12 files changed

Lines changed: 40 additions & 31 deletions

File tree

instrumentation/jetty-httpclient/jetty-httpclient-12.0/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v12_0/TracingHttpRequest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import io.opentelemetry.instrumentation.jetty.httpclient.v12_0.internal.JettyClientWrapUtil;
1313
import java.net.URI;
1414
import java.nio.ByteBuffer;
15+
import javax.annotation.Nullable;
1516
import org.eclipse.jetty.client.HttpClient;
1617
import org.eclipse.jetty.client.Request;
1718
import org.eclipse.jetty.client.Response;
@@ -21,9 +22,9 @@
2122
class TracingHttpRequest extends HttpRequest {
2223

2324
private final Instrumenter<Request, Response> instrumenter;
24-
private Context parentContext;
25+
@Nullable private Context parentContext;
2526

26-
public TracingHttpRequest(
27+
TracingHttpRequest(
2728
HttpClient client,
2829
HttpConversation conversation,
2930
URI uri,
@@ -40,6 +41,7 @@ public void send(Response.CompleteListener listener) {
4041
super.send(JettyClientWrapUtil.wrapTheListener(listener, parentContext));
4142
}
4243

44+
@Nullable
4345
private Scope openScope() {
4446
return parentContext != null ? parentContext.makeCurrent() : null;
4547
}

instrumentation/jetty-httpclient/jetty-httpclient-12.0/testing/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v12_0/AbstractJettyClient12Test.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ void after() throws Exception {
5959
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
6060
// disable redirect tests
6161
optionsBuilder.disableTestRedirects();
62-
// jetty 12 does not support to reuse request
63-
// use request.send() twice will block the program infinitely
62+
// Jetty 12 does not support reusing requests.
63+
// Calling request.send() twice blocks indefinitely.
6464
optionsBuilder.disableTestReusedRequest();
6565
optionsBuilder.spanEndsAfterBody();
6666
}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/build.gradle.kts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ muzzle {
1212
}
1313

1414
// Jetty client 9.2 is the best starting point, HttpClient.send() is stable there
15-
val jettyVers_base9 = "9.2.0.v20140526"
15+
val jettyVersBase9 = "9.2.0.v20140526"
1616

1717
dependencies {
1818
implementation(project(":instrumentation:jetty-httpclient:jetty-httpclient-9.2:library"))
1919

20-
library("org.eclipse.jetty:jetty-client:$jettyVers_base9")
20+
library("org.eclipse.jetty:jetty-client:$jettyVersBase9")
2121

2222
testInstrumentation(project(":instrumentation:jetty-httpclient:jetty-httpclient-12.0:javaagent"))
2323

instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9InstrumentationModule.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ public List<TypeInstrumentation> typeInstrumentations() {
2828

2929
@Override
3030
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
31-
return hasClassesNamed(
32-
// added in 9.2
33-
"org.eclipse.jetty.client.util.AbstractTypedContentProvider");
31+
// added in 9.2
32+
return hasClassesNamed("org.eclipse.jetty.client.util.AbstractTypedContentProvider");
3433
}
3534
}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientTracingListener.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ private static void wrapRequestListeners(
8484
ListIterator<Request.RequestListener> iterator = requestListeners.listIterator();
8585

8686
while (iterator.hasNext()) {
87-
List<Class<?>> interfaces = new ArrayList<>();
87+
List<Class<?>> interfaces = new ArrayList<>(REQUEST_LISTENER_INTERFACES.length);
8888
Request.RequestListener listener = iterator.next();
8989

9090
Class<?> listenerClass = listener.getClass();

instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientWrapUtil.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal;
77

88
import static java.util.Arrays.asList;
9-
import static java.util.stream.Collectors.toList;
109

1110
import io.opentelemetry.context.Context;
1211
import io.opentelemetry.context.Scope;
@@ -60,10 +59,11 @@ private JettyClientWrapUtil() {}
6059
*/
6160
public static List<Response.ResponseListener> wrapResponseListeners(
6261
Context parentContext, List<Response.ResponseListener> listeners) {
63-
64-
return listeners.stream()
65-
.map(listener -> wrapTheListener(listener, parentContext))
66-
.collect(toList());
62+
List<Response.ResponseListener> wrappedListeners = new ArrayList<>(listeners.size());
63+
for (Response.ResponseListener listener : listeners) {
64+
wrappedListeners.add(wrapTheListener(listener, parentContext));
65+
}
66+
return wrappedListeners;
6767
}
6868

6969
private static Response.ResponseListener wrapTheListener(
@@ -73,7 +73,7 @@ private static Response.ResponseListener wrapTheListener(
7373
}
7474

7575
Class<?> listenerClass = listener.getClass();
76-
List<Class<?>> interfaces = new ArrayList<>();
76+
List<Class<?>> interfaces = new ArrayList<>(LISTENER_INTERFACES.length);
7777
for (Class<?> type : LISTENER_INTERFACES) {
7878
if (type.isInstance(listener)) {
7979
interfaces.add(type);

instrumentation/jetty-httpclient/jetty-httpclient-9.2/metadata.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,19 @@ semantic_conventions:
66
library_link: https://eclipse.dev/jetty/
77
configurations:
88
- name: otel.instrumentation.http.known-methods
9+
declarative_name: java.common.http.known_methods
910
description: >
1011
Configures the instrumentation to recognize an alternative set of HTTP request methods. All
1112
other methods will be treated as `_OTHER`.
1213
type: list
1314
default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
1415
- name: otel.instrumentation.http.client.capture-request-headers
16+
declarative_name: general.http.client.request_captured_headers
1517
description: List of HTTP request headers to capture in HTTP client telemetry.
1618
type: list
1719
default: ""
1820
- name: otel.instrumentation.http.client.capture-response-headers
21+
declarative_name: general.http.client.response_captured_headers
1922
description: List of HTTP response headers to capture in HTTP client telemetry.
2023
type: list
2124
default: ""
@@ -24,13 +27,15 @@ configurations:
2427
type: map
2528
default: ""
2629
- name: otel.instrumentation.http.client.emit-experimental-telemetry
30+
declarative_name: java.common.http.client.emit_experimental_telemetry/development
2731
description: >
2832
Enable the capture of experimental HTTP client telemetry. Adds the `http.request.body.size`
2933
and `http.response.body.size` attributes to spans, and records `http.client.request.size` and
3034
`http.client.response.size` metrics.
3135
type: boolean
3236
default: false
3337
- name: otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters
38+
declarative_name: general.sanitization.url.sensitive_query_parameters/development
3439
description: List of URL query parameter names whose values are redacted in URL attributes. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.
3540
type: list
3641
default: "AWSAccessKeyId,Signature,sig,X-Goog-Signature"

instrumentation/jetty-httpclient/jetty-httpclient-9.2/testing/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/AbstractJettyClient9Test.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import static java.util.concurrent.TimeUnit.MILLISECONDS;
99

10+
import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
1011
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
1112
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
1213
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
@@ -18,13 +19,15 @@
1819
import org.eclipse.jetty.client.api.Request;
1920
import org.eclipse.jetty.client.api.Response;
2021
import org.eclipse.jetty.util.ssl.SslContextFactory;
21-
import org.junit.jupiter.api.AfterEach;
2222
import org.junit.jupiter.api.BeforeEach;
2323
import org.junit.jupiter.api.TestInstance;
24+
import org.junit.jupiter.api.extension.RegisterExtension;
2425

2526
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
2627
public abstract class AbstractJettyClient9Test extends AbstractHttpClientTest<Request> {
2728

29+
@RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
30+
2831
private HttpClient client;
2932
private HttpClient httpsClient;
3033

@@ -34,17 +37,13 @@ void before() throws Exception {
3437
client = createStandardClient();
3538
client.setConnectTimeout(CONNECTION_TIMEOUT.toMillis());
3639
client.start();
40+
cleanup.deferCleanup(client::stop);
3741

3842
SslContextFactory tlsCtx = new SslContextFactory();
3943
httpsClient = createHttpsClient(tlsCtx);
4044
httpsClient.setFollowRedirects(false);
4145
httpsClient.start();
42-
}
43-
44-
@AfterEach
45-
void after() throws Exception {
46-
client.stop();
47-
httpsClient.stop();
46+
cleanup.deferCleanup(httpsClient::stop);
4847
}
4948

5049
@Override
@@ -55,7 +54,7 @@ protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
5554

5655
@Override
5756
public Request buildRequest(String method, URI uri, Map<String, String> headers) {
58-
HttpClient theClient = uri.getScheme().equalsIgnoreCase("https") ? httpsClient : client;
57+
HttpClient theClient = "https".equalsIgnoreCase(uri.getScheme()) ? httpsClient : client;
5958
Request request = theClient.newRequest(uri).method(method).agent("Jetty");
6059
headers.forEach(request::header);
6160

instrumentation/jetty/jetty-11.0/metadata.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,24 @@ semantic_conventions:
66
library_link: https://eclipse.dev/jetty/
77
configurations:
88
- name: otel.instrumentation.http.known-methods
9+
declarative_name: java.common.http.known_methods
910
description: >
1011
Configures the instrumentation to recognize an alternative set of HTTP request methods. All
1112
other methods will be treated as `_OTHER`.
1213
type: list
1314
default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
1415
- name: otel.instrumentation.http.server.capture-request-headers
16+
declarative_name: general.http.server.request_captured_headers
1517
description: List of HTTP request headers to capture in HTTP server telemetry.
1618
type: list
1719
default: ""
1820
- name: otel.instrumentation.http.server.capture-response-headers
21+
declarative_name: general.http.server.response_captured_headers
1922
description: List of HTTP response headers to capture in HTTP server telemetry.
2023
type: list
2124
default: ""
2225
- name: otel.instrumentation.http.server.emit-experimental-telemetry
26+
declarative_name: java.common.http.server.emit_experimental_telemetry/development
2327
description: >
2428
Enable the capture of experimental HTTP server telemetry. Adds the `http.request.body.size`
2529
and `http.response.body.size` attributes to spans, and records `http.server.request.size` and

instrumentation/jetty/jetty-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v12_0/Jetty12Helper.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,18 @@
1414
import org.eclipse.jetty.server.Request;
1515
import org.eclipse.jetty.server.Response;
1616

17-
public class Jetty12Helper {
17+
class Jetty12Helper {
1818
private final Instrumenter<Request, Response> instrumenter;
1919

2020
Jetty12Helper(Instrumenter<Request, Response> instrumenter) {
2121
this.instrumenter = instrumenter;
2222
}
2323

24-
public boolean shouldStart(Context parentContext, Request request) {
24+
boolean shouldStart(Context parentContext, Request request) {
2525
return instrumenter.shouldStart(parentContext, request);
2626
}
2727

28-
public Context start(Context parentContext, Request request, Response response) {
28+
Context start(Context parentContext, Request request, Response response) {
2929
Context context = instrumenter.start(parentContext, request);
3030
request.addFailureListener(throwable -> end(context, request, response, throwable));
3131
// detect request completion
@@ -49,7 +49,7 @@ public void failed(Throwable throwable) {
4949
return context;
5050
}
5151

52-
public void end(Context context, Request request, Response response, @Nullable Throwable error) {
52+
void end(Context context, Request request, Response response, @Nullable Throwable error) {
5353
error = AppServerBridge.getException(context, error);
5454
error = ServletAsyncContext.getAsyncException(context, error);
5555

0 commit comments

Comments
 (0)