Skip to content

Commit 785c8d8

Browse files
otelbot[bot]trask
andauthored
Code review sweep (run 24906382919) (#18268)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent 3aa41ae commit 785c8d8

11 files changed

Lines changed: 38 additions & 22 deletions

File tree

instrumentation/servlet/servlet-2.2/metadata.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,22 @@ features:
88
- HTTP_ROUTE
99
configurations:
1010
- name: otel.instrumentation.servlet.experimental-span-attributes
11+
declarative_name: java.servlet.experimental_span_attributes/development
1112
description: Enables capturing the experimental `servlet.timeout` span attribute.
1213
type: boolean
1314
default: false
1415
- name: otel.instrumentation.servlet.experimental.capture-request-parameters
16+
declarative_name: java.servlet.capture_request_parameters/development
1517
description: List of request parameter names to capture as span attributes.
1618
type: list
1719
default: ""
1820
- name: otel.instrumentation.servlet.experimental.trace-id-request-attribute.enabled
21+
declarative_name: java.servlet.trace_id_request_attribute/development.enabled
1922
description: Enables adding the trace ID and span ID as request attributes for downstream servlet access.
2023
type: boolean
2124
default: true
25+
- name: otel.experimental.javascript-snippet
26+
declarative_name: java.servlet.javascript_snippet/development
27+
description: Experimental setting to inject a JavaScript snippet into HTML responses after the opening `<head>` tag.
28+
type: string
29+
default: ""

instrumentation/servlet/servlet-3.0/metadata.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ configurations:
2424
default: true
2525
- name: otel.experimental.javascript-snippet
2626
declarative_name: java.servlet.javascript_snippet/development
27-
description: HTML snippet to inject into `text/html` servlet responses.
27+
description: Experimental setting to inject a JavaScript snippet into HTML responses after the opening `<head>` tag.
2828
type: string
2929
default: ""
3030
examples:

instrumentation/servlet/servlet-5.0/metadata.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,22 @@ features:
88
- HTTP_ROUTE
99
configurations:
1010
- name: otel.instrumentation.servlet.experimental-span-attributes
11+
declarative_name: java.servlet.experimental_span_attributes/development
1112
description: Enables capturing the experimental `servlet.timeout` span attribute.
1213
type: boolean
1314
default: false
1415
- name: otel.instrumentation.servlet.experimental.capture-request-parameters
16+
declarative_name: java.servlet.capture_request_parameters/development
1517
description: List of request parameter names to capture as span attributes.
1618
type: list
1719
default: ""
1820
- name: otel.instrumentation.servlet.experimental.trace-id-request-attribute.enabled
21+
declarative_name: java.servlet.trace_id_request_attribute/development.enabled
1922
description: Enables adding the trace ID and span ID as request attributes for downstream servlet access.
2023
type: boolean
2124
default: true
25+
- name: otel.experimental.javascript-snippet
26+
declarative_name: java.servlet.javascript_snippet/development
27+
description: Experimental setting to inject a JavaScript snippet into HTML responses after the opening `<head>` tag.
28+
type: string
29+
default: ""

instrumentation/servlet/servlet-5.0/tomcat-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/tomcat/TomcatDispatchTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111

1212
class TomcatDispatchTest extends BaseTomcatDispatchTest {
1313
@RegisterExtension
14-
protected static final InstrumentationExtension testing =
14+
private static final InstrumentationExtension testing =
1515
HttpServerInstrumentationExtension.forAgent();
1616
}

instrumentation/servlet/servlet-5.0/tomcat-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/tomcat/TomcatServlet5Test.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111

1212
class TomcatServlet5Test extends BaseTomcatServlet5Test {
1313
@RegisterExtension
14-
protected static final InstrumentationExtension testing =
14+
private static final InstrumentationExtension testing =
1515
HttpServerInstrumentationExtension.forAgent();
1616
}

instrumentation/servlet/servlet-5.0/tomcat-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/tomcat/TomcatServlet5WebXmlFilterTest.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
1414
import java.io.ByteArrayOutputStream;
1515
import java.io.File;
16+
import java.io.IOException;
1617
import java.io.InputStream;
1718
import java.net.HttpURLConnection;
1819
import java.net.URI;
@@ -42,12 +43,12 @@
4243
class TomcatServlet5WebXmlFilterTest {
4344

4445
@RegisterExtension
45-
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
46+
private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
4647

47-
@TempDir static File tempDir;
48+
@TempDir private static File tempDir;
4849

49-
static Tomcat tomcat;
50-
static int port;
50+
private static Tomcat tomcat;
51+
private static int port;
5152

5253
@BeforeAll
5354
static void setup() throws Exception {
@@ -119,7 +120,7 @@ static void cleanup() throws Exception {
119120
}
120121
}
121122

122-
private static String readFully(InputStream is) throws Exception {
123+
private static String readFully(InputStream is) throws IOException {
123124
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
124125
byte[] data = new byte[1024];
125126
int len;
@@ -129,7 +130,7 @@ private static String readFully(InputStream is) throws Exception {
129130
return buffer.toString(UTF_8.name());
130131
}
131132

132-
private static void copyClassFile(Class<?> clazz, File targetDir) throws Exception {
133+
private static void copyClassFile(Class<?> clazz, File targetDir) throws IOException {
133134
String classFileName = clazz.getSimpleName() + ".class";
134135
String classResourcePath = clazz.getName().replace('.', '/') + ".class";
135136
try (InputStream is = clazz.getClassLoader().getResourceAsStream(classResourcePath)) {
@@ -140,7 +141,7 @@ private static void copyClassFile(Class<?> clazz, File targetDir) throws Excepti
140141
}
141142

142143
@Test
143-
void filterLoadedFromWebXmlDoesNotCauseClassCastException() throws Exception {
144+
void filterLoadedFromWebXmlDoesNotCauseClassCastException() throws IOException {
144145
HttpURLConnection connection =
145146
(HttpURLConnection)
146147
URI.create("http://localhost:" + port + "/app/users/123").toURL().openConnection();

instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletFilterMappingResolverFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ protected Mappings getMappings() {
5959

6060
List<String> mappingsList = new ArrayList<>(mappings);
6161
// sort the longest mapping first
62-
mappingsList.sort((s1, s2) -> s2.length() - s1.length());
62+
mappingsList.sort((s1, s2) -> Integer.compare(s2.length(), s1.length()));
6363

6464
return new Mappings(mappingsList);
6565
}

instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletHelper.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ public void end(
2727
ServletRequestContext<REQUEST> requestContext,
2828
REQUEST request,
2929
RESPONSE response,
30-
Throwable throwable,
30+
@Nullable Throwable throwable,
3131
boolean topLevel,
32-
Context context,
33-
Scope scope) {
32+
@Nullable Context context,
33+
@Nullable Scope scope) {
3434

3535
if (scope != null) {
3636
scope.close();

instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/response/HttpServletResponseAdviceHelper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public Scope getScope() {
5757

5858
public static void stopSpan(
5959
Instrumenter<ClassAndMethod, Void> instrumenter,
60-
Throwable throwable,
60+
@Nullable Throwable throwable,
6161
@Nullable Context context,
6262
@Nullable Scope scope,
6363
@Nullable ClassAndMethod request) {

instrumentation/servlet/servlet-common/library/src/main/java/io/opentelemetry/instrumentation/servlet/internal/ServletInstrumenterBuilder.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424
*/
2525
public final class ServletInstrumenterBuilder<REQUEST, RESPONSE> {
2626

27+
private final DefaultHttpServerInstrumenterBuilder<
28+
ServletRequestContext<REQUEST>, ServletResponseContext<RESPONSE>>
29+
builder;
30+
private final ServletAccessor<REQUEST, RESPONSE> accessor;
2731
private final List<ContextCustomizer<? super ServletRequestContext<REQUEST>>> contextCustomizers =
2832
new ArrayList<>();
2933

@@ -32,11 +36,6 @@ public final class ServletInstrumenterBuilder<REQUEST, RESPONSE> {
3236
private boolean captureEnduserId;
3337
private List<String> captureRequestParameters = new ArrayList<>();
3438

35-
private final DefaultHttpServerInstrumenterBuilder<
36-
ServletRequestContext<REQUEST>, ServletResponseContext<RESPONSE>>
37-
builder;
38-
private final ServletAccessor<REQUEST, RESPONSE> accessor;
39-
4039
public static <REQUEST, RESPONSE> ServletInstrumenterBuilder<REQUEST, RESPONSE> create(
4140
String instrumentationName,
4241
OpenTelemetry openTelemetry,

0 commit comments

Comments
 (0)