Skip to content

Commit e2ec96f

Browse files
otelbot[bot]trask
otelbot[bot]
and
trask
authored
Code review sweep (run 24974154176) (#18333)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent f4a2cd7 commit e2ec96f

11 files changed

Lines changed: 46 additions & 40 deletions

File tree

‎instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroHelper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public class MetroHelper {
1818
private static final String SCOPE_KEY = MetroHelper.class.getName() + ".Scope";
1919
private static final String THROWABLE_KEY = MetroHelper.class.getName() + ".Throwable";
2020

21-
private static final MetroServerSpanNameUpdater SPAN_NAME_UPDATER =
21+
private static final MetroServerSpanNameUpdater spanNameUpdater =
2222
new MetroServerSpanNameUpdater();
2323

2424
private MetroHelper() {}
@@ -27,7 +27,7 @@ public static void start(WSEndpoint<?> endpoint, Packet packet) {
2727
Context parentContext = Context.current();
2828

2929
MetroRequest request = new MetroRequest(endpoint, packet);
30-
SPAN_NAME_UPDATER.updateServerSpanName(parentContext, request);
30+
spanNameUpdater.updateServerSpanName(parentContext, request);
3131

3232
if (!instrumenter().shouldStart(parentContext, request)) {
3333
return;

‎instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroRequest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,20 @@
99
import com.sun.xml.ws.api.server.WSEndpoint;
1010
import javax.xml.namespace.QName;
1111

12-
public class MetroRequest {
12+
class MetroRequest {
1313
private final Packet packet;
1414
private final String spanName;
1515

16-
public MetroRequest(WSEndpoint<?> endpoint, Packet packet) {
16+
MetroRequest(WSEndpoint<?> endpoint, Packet packet) {
1717
this.packet = packet;
1818
this.spanName = getSpanName(endpoint, packet);
1919
}
2020

21-
public String spanName() {
21+
String spanName() {
2222
return spanName;
2323
}
2424

25-
public Packet packet() {
25+
Packet packet() {
2626
return packet;
2727
}
2828

‎instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroServerSpanNameUpdater.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ final class MetroServerSpanNameUpdater {
3939
*/
4040
private final Map<String, HttpServletRequestAdapter> servletRequestAdapters;
4141

42-
public MetroServerSpanNameUpdater() {
42+
MetroServerSpanNameUpdater() {
4343
this.servletRequestAdapters = new LinkedHashMap<>();
4444

4545
registerHttpServletRequestAdapter(
@@ -72,7 +72,7 @@ private void registerHttpServletRequestAdapter(
7272
logger.finest(() -> "Enabled " + name + " jaxws metro server span naming");
7373
}
7474

75-
public void updateServerSpanName(Context context, MetroRequest metroRequest) {
75+
void updateServerSpanName(Context context, MetroRequest metroRequest) {
7676
String spanName = metroRequest.spanName();
7777

7878
Span serverSpan = LocalRootSpan.fromContextOrNull(context);

‎instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroSingletons.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
1010
import io.opentelemetry.javaagent.bootstrap.internal.ExperimentalConfig;
1111

12-
public class MetroSingletons {
12+
class MetroSingletons {
1313
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jaxws-metro-2.2";
1414

1515
private static final Instrumenter<MetroRequest, Void> instrumenter;
@@ -22,7 +22,7 @@ public class MetroSingletons {
2222
.buildInstrumenter();
2323
}
2424

25-
public static Instrumenter<MetroRequest, Void> instrumenter() {
25+
static Instrumenter<MetroRequest, Void> instrumenter() {
2626
return instrumenter;
2727
}
2828

‎instrumentation/jboss-logmanager/jboss-logmanager-appender-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/appender/v1_1/LoggingEventMapper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ public class LoggingEventMapper {
3232

3333
private static final Cache<String, AttributeKey<String>> mdcAttributeKeys = Cache.bounded(100);
3434

35-
private final List<AttributeKey<String>> captureMdcAttributeKeys;
36-
3735
private static final boolean captureExperimentalAttributes =
3836
DeclarativeConfigUtil.getInstrumentationConfig(GlobalOpenTelemetry.get(), "jboss_logmanager")
3937
.getBoolean("experimental_log_attributes/development", false);
4038

39+
private final List<AttributeKey<String>> captureMdcAttributeKeys;
40+
4141
// cached as an optimization
4242
private final boolean captureAllMdcAttributes;
4343

‎instrumentation/jboss-logmanager/jboss-logmanager-mdc-1.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/mdc/v1_1/JbossLogmanagerMdcTest.java‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
package io.opentelemetry.javaagent.instrumentation.jbosslogmanager.mdc.v1_1;
77

8-
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
8+
import static org.assertj.core.api.Assertions.assertThat;
99

1010
import io.opentelemetry.api.trace.Span;
1111
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
@@ -58,11 +58,12 @@ void noIdsGeneratedWhenNoSpanProvided() {
5858
try {
5959
logger.info("log message 1");
6060

61-
assertThat(logRecords).hasSize(1);
62-
assertThat(logRecords.get(0).getMessage()).isEqualTo("log message 1");
63-
assertThat(logRecords.get(0).getMdc("trace_id")).isNull();
64-
assertThat(logRecords.get(0).getMdc("span_id")).isNull();
65-
assertThat(logRecords.get(0).getMdc("trace_flags")).isNull();
61+
assertThat(logRecords).extracting(ExtLogRecord::getMessage).containsExactly("log message 1");
62+
63+
ExtLogRecord logRecord = logRecords.get(0);
64+
assertThat(logRecord.getMdc("trace_id")).isNull();
65+
assertThat(logRecord.getMdc("span_id")).isNull();
66+
assertThat(logRecord.getMdc("trace_flags")).isNull();
6667
} finally {
6768
logger.removeHandler(handler);
6869
}
@@ -94,46 +95,47 @@ void idsGeneratedWhenSpanProvided() throws ReflectiveOperationException {
9495
return Span.current();
9596
});
9697

97-
assertThat(logRecords).hasSize(3);
98+
assertThat(logRecords)
99+
.extracting(ExtLogRecord::getMessage)
100+
.containsExactly("log message 1", "log message 2", "log message 3");
101+
102+
ExtLogRecord firstLogRecord = logRecords.get(0);
103+
ExtLogRecord secondLogRecord = logRecords.get(1);
104+
ExtLogRecord thirdLogRecord = logRecords.get(2);
98105

99106
Method getMdcCopy = null;
100107
try {
101-
getMdcCopy = logRecords.get(0).getClass().getMethod("getMdcCopy");
108+
getMdcCopy = firstLogRecord.getClass().getMethod("getMdcCopy");
102109
} catch (NoSuchMethodException ignored) {
103110
// ignored
104111
}
105112

106-
assertThat(logRecords.get(0).getMessage()).isEqualTo("log message 1");
107-
assertThat(logRecords.get(0).getMdc("trace_id"))
108-
.isEqualTo(span1.getSpanContext().getTraceId());
109-
assertThat(logRecords.get(0).getMdc("span_id")).isEqualTo(span1.getSpanContext().getSpanId());
110-
assertThat(logRecords.get(0).getMdc("trace_flags"))
113+
assertThat(firstLogRecord.getMdc("trace_id")).isEqualTo(span1.getSpanContext().getTraceId());
114+
assertThat(firstLogRecord.getMdc("span_id")).isEqualTo(span1.getSpanContext().getSpanId());
115+
assertThat(firstLogRecord.getMdc("trace_flags"))
111116
.isEqualTo(span1.getSpanContext().getTraceFlags().asHex());
112117

113118
if (getMdcCopy != null) {
114119
@SuppressWarnings("unchecked")
115-
Map<String, String> copiedMdc = (Map<String, String>) getMdcCopy.invoke(logRecords.get(0));
120+
Map<String, String> copiedMdc = (Map<String, String>) getMdcCopy.invoke(firstLogRecord);
116121
assertThat(copiedMdc.get("trace_id")).isEqualTo(span1.getSpanContext().getTraceId());
117122
assertThat(copiedMdc.get("span_id")).isEqualTo(span1.getSpanContext().getSpanId());
118123
assertThat(copiedMdc.get("trace_flags"))
119124
.isEqualTo(span1.getSpanContext().getTraceFlags().asHex());
120125
}
121126

122-
assertThat(logRecords.get(1).getMessage()).isEqualTo("log message 2");
123-
assertThat(logRecords.get(1).getMdc("trace_id")).isNull();
124-
assertThat(logRecords.get(1).getMdc("span_id")).isNull();
125-
assertThat(logRecords.get(1).getMdc("trace_flags")).isNull();
127+
assertThat(secondLogRecord.getMdc("trace_id")).isNull();
128+
assertThat(secondLogRecord.getMdc("span_id")).isNull();
129+
assertThat(secondLogRecord.getMdc("trace_flags")).isNull();
126130

127-
assertThat(logRecords.get(2).getMessage()).isEqualTo("log message 3");
128-
assertThat(logRecords.get(2).getMdc("trace_id"))
129-
.isEqualTo(span2.getSpanContext().getTraceId());
130-
assertThat(logRecords.get(2).getMdc("span_id")).isEqualTo(span2.getSpanContext().getSpanId());
131-
assertThat(logRecords.get(2).getMdc("trace_flags"))
131+
assertThat(thirdLogRecord.getMdc("trace_id")).isEqualTo(span2.getSpanContext().getTraceId());
132+
assertThat(thirdLogRecord.getMdc("span_id")).isEqualTo(span2.getSpanContext().getSpanId());
133+
assertThat(thirdLogRecord.getMdc("trace_flags"))
132134
.isEqualTo(span2.getSpanContext().getTraceFlags().asHex());
133135

134136
if (getMdcCopy != null) {
135137
@SuppressWarnings("unchecked")
136-
Map<String, String> copiedMdc = (Map<String, String>) getMdcCopy.invoke(logRecords.get(2));
138+
Map<String, String> copiedMdc = (Map<String, String>) getMdcCopy.invoke(thirdLogRecord);
137139
assertThat(copiedMdc.get("trace_id")).isEqualTo(span2.getSpanContext().getTraceId());
138140
assertThat(copiedMdc.get("span_id")).isEqualTo(span2.getSpanContext().getSpanId());
139141
assertThat(copiedMdc.get("trace_flags"))

‎instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ public void end(@Nullable Throwable throwable) {
170170
}
171171
}
172172

173+
@Nullable
173174
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
174175
public static AdviceScope onEnter(
175176
@Advice.This Connection connection, @Advice.Origin("#m") String methodName) {

‎instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcAdviceScope.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ private static JdbcAdviceScope start(CallDepth callDepth, Supplier<DbRequest> re
7474
return new JdbcAdviceScope(callDepth, request, context, context.makeCurrent());
7575
}
7676

77+
@Nullable
7778
private static DbRequest createBatchRequest(Statement statement) {
7879
if (statement instanceof PreparedStatement) {
7980
String sql = JdbcData.PREPARED_STATEMENT.get((PreparedStatement) statement);

‎instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ public static void onExit(
239239
public static class ClearParametersAdvice {
240240

241241
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
242-
public static void clearBatch(@Advice.This PreparedStatement statement) {
242+
public static void clearParameters(@Advice.This PreparedStatement statement) {
243243
JdbcData.clearParameters(statement);
244244
}
245245
}

‎instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ public void setOpenTelemetry(OpenTelemetry openTelemetry) {
240240

241241
@Nullable
242242
@Override
243-
public Connection connect(String url, Properties info) throws SQLException {
243+
public Connection connect(String url, @Nullable Properties info) throws SQLException {
244244
if (url == null || url.trim().isEmpty()) {
245245
throw new IllegalArgumentException("url is required");
246246
}
@@ -283,7 +283,8 @@ public boolean acceptsURL(String url) {
283283
}
284284

285285
@Override
286-
public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException {
286+
public DriverPropertyInfo[] getPropertyInfo(String url, @Nullable Properties info)
287+
throws SQLException {
287288
if (url == null || url.trim().isEmpty()) {
288289
throw new IllegalArgumentException("url is required");
289290
}

0 commit comments

Comments
 (0)