Skip to content

Commit 0917e81

Browse files
authored
Code review sweep (run 25192686470) (open-telemetry#18463)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
1 parent 5c26f02 commit 0917e81

16 files changed

Lines changed: 55 additions & 48 deletions

File tree

instrumentation/jaxws/jaxws-2.0-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxws/v2_0/metro/v2_2/MetroServerSpanNameUpdater.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package io.opentelemetry.javaagent.instrumentation.jaxws.v2_0.metro.v2_2;
77

88
import static java.util.Objects.requireNonNull;
9+
import static java.util.logging.Level.FINE;
910

1011
import com.sun.xml.ws.api.message.Packet;
1112
import io.opentelemetry.api.trace.Span;
@@ -158,14 +159,9 @@ private static String invokeSafely(MethodHandle methodHandle, Object httpServlet
158159
// servlet path to the span name
159160
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/10986
160161
return null;
161-
} catch (RuntimeException | Error e) {
162-
throw e;
163162
} catch (Throwable t) {
164-
/*
165-
* This is impossible, because the methods being invoked do not throw checked exceptions,
166-
* and unchecked exceptions and errors are handled above
167-
*/
168-
throw new AssertionError(t);
163+
logger.log(FINE, "Failed to get servlet path for jaxws metro span name", t);
164+
return null;
169165
}
170166
}
171167
}

instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/v0_11/KafkaConsumerInstrumentation.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
2323
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
2424
import java.time.Duration;
25+
import javax.annotation.Nullable;
2526
import net.bytebuddy.asm.Advice;
2627
import net.bytebuddy.description.type.TypeDescription;
2728
import net.bytebuddy.matcher.ElementMatcher;
@@ -58,8 +59,8 @@ public static Timer onEnter() {
5859
public static void onExit(
5960
@Advice.Enter Timer timer,
6061
@Advice.This Consumer<?, ?> consumer,
61-
@Advice.Return ConsumerRecords<?, ?> records,
62-
@Advice.Thrown Throwable error) {
62+
@Advice.Return @Nullable ConsumerRecords<?, ?> records,
63+
@Advice.Thrown @Nullable Throwable error) {
6364

6465
// don't create spans when no records were received
6566
if (records == null || records.isEmpty()) {

instrumentation/kafka/kafka-clients/kafka-clients-0.11/testing/src/main/java/io/opentelemetry/instrumentation/kafkaclients/common/v0_11/internal/KafkaTestUtil.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class KafkaTestUtil {
1616
private static Method getConsumerPollDurationMethod() {
1717
try {
1818
return Consumer.class.getMethod("poll", Duration.class);
19-
} catch (NoSuchMethodException e) {
19+
} catch (NoSuchMethodException ignored) {
2020
return null;
2121
}
2222
}

instrumentation/kafka/kafka-clients/kafka-clients-common-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/common/v0_11/internal/KafkaBatchProcessSpanLinksExtractor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ final class KafkaBatchProcessSpanLinksExtractor implements SpanLinksExtractor<Ka
1818

1919
KafkaBatchProcessSpanLinksExtractor(TextMapPropagator propagator) {
2020
this.singleRecordLinkExtractor =
21-
new PropagatorBasedSpanLinksExtractor<>(propagator, KafkaConsumerRecordGetter.INSTANCE);
21+
new PropagatorBasedSpanLinksExtractor<>(propagator, new KafkaConsumerRecordGetter());
2222
}
2323

2424
@Override

instrumentation/kafka/kafka-clients/kafka-clients-common-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/common/v0_11/internal/KafkaConsumerRecordGetter.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@
1515
import javax.annotation.Nullable;
1616
import org.apache.kafka.common.header.Header;
1717

18-
enum KafkaConsumerRecordGetter implements TextMapGetter<KafkaProcessRequest> {
19-
INSTANCE;
20-
18+
class KafkaConsumerRecordGetter implements TextMapGetter<KafkaProcessRequest> {
2119
@Override
2220
public Iterable<String> keys(KafkaProcessRequest carrier) {
2321
return StreamSupport.stream(carrier.getRecord().headers().spliterator(), false)

instrumentation/kafka/kafka-clients/kafka-clients-common-0.11/library/src/main/java/io/opentelemetry/instrumentation/kafkaclients/common/v0_11/internal/KafkaInstrumenterFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,10 @@ public Instrumenter<KafkaProcessRequest, Void> createConsumerProcessInstrumenter
143143
builder.addSpanLinksExtractor(
144144
new PropagatorBasedSpanLinksExtractor<>(
145145
openTelemetry.getPropagators().getTextMapPropagator(),
146-
KafkaConsumerRecordGetter.INSTANCE));
146+
new KafkaConsumerRecordGetter()));
147147
return builder.buildInstrumenter(SpanKindExtractor.alwaysConsumer());
148148
} else {
149-
return builder.buildConsumerInstrumenter(KafkaConsumerRecordGetter.INSTANCE);
149+
return builder.buildConsumerInstrumenter(new KafkaConsumerRecordGetter());
150150
}
151151
}
152152

instrumentation/kafka/kafka-connect-2.6/testing/src/test/java/io/opentelemetry/instrumentation/kafkaconnect/v2_6/KafkaConnectSinkTaskBaseTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ protected static String getKafkaConnectUrl() {
122122
kafkaConnect.getMappedPort(CONNECT_REST_PORT_INTERNAL));
123123
}
124124

125-
protected static String getInternalKafkaBoostrapServers() {
125+
protected static String getInternalKafkaBootstrapServers() {
126126
return KAFKA_NETWORK_ALIAS + ":" + KAFKA_INTERNAL_ADVERTISED_LISTENERS_PORT;
127127
}
128128

129-
protected static String getKafkaBoostrapServers() {
129+
protected static String getKafkaBootstrapServers() {
130130
return kafka.getHost() + ":" + kafkaExposedPort;
131131
}
132132

@@ -279,7 +279,7 @@ private void setupKafkaConnect() {
279279
.withEnv(
280280
"OTEL_SEMCONV_STABILITY_OPT_IN",
281281
System.getProperty("otel.semconv-stability.opt-in"))
282-
.withEnv("CONNECT_BOOTSTRAP_SERVERS", getInternalKafkaBoostrapServers())
282+
.withEnv("CONNECT_BOOTSTRAP_SERVERS", getInternalKafkaBootstrapServers())
283283
.withEnv("CONNECT_REST_ADVERTISED_HOST_NAME", KAFKA_CONNECT_NETWORK_ALIAS)
284284
.withEnv("CONNECT_PLUGIN_PATH", PLUGIN_PATH_CONTAINER)
285285
.withEnv(
@@ -326,7 +326,7 @@ protected void awaitForTopicCreation(String topicName) {
326326

327327
protected AdminClient createAdminClient() {
328328
Properties properties = new Properties();
329-
properties.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBoostrapServers());
329+
properties.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBootstrapServers());
330330
return KafkaAdminClient.create(properties);
331331
}
332332

instrumentation/kafka/kafka-connect-2.6/testing/src/test/java/io/opentelemetry/instrumentation/kafkaconnect/v2_6/MongoKafkaConnectSinkTaskTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ void testSingleMessage() throws IOException {
106106
awaitForTopicCreation(testTopicName);
107107

108108
Properties props = new Properties();
109-
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBoostrapServers());
109+
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBootstrapServers());
110110
props.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class);
111111
props.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class);
112112

@@ -180,7 +180,7 @@ void testMultiTopic() throws IOException {
180180
awaitForTopicCreation(topicName3);
181181

182182
Properties props = new Properties();
183-
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBoostrapServers());
183+
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBootstrapServers());
184184
props.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class);
185185
props.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class);
186186
props.put(ProducerConfig.BATCH_SIZE_CONFIG, 10); // to send messages in one batch

instrumentation/kafka/kafka-connect-2.6/testing/src/test/java/io/opentelemetry/instrumentation/kafkaconnect/v2_6/PostgresKafkaConnectSinkTaskTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ void testSingleMessage() throws IOException {
121121
awaitForTopicCreation(testTopicName);
122122

123123
Properties props = new Properties();
124-
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBoostrapServers());
124+
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBootstrapServers());
125125
props.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class);
126126
props.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class);
127127

@@ -217,7 +217,7 @@ void testMultiTopic() throws IOException {
217217
awaitForTopicCreation(topicName3);
218218

219219
Properties props = new Properties();
220-
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBoostrapServers());
220+
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, getKafkaBootstrapServers());
221221
props.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class);
222222
props.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class);
223223
props.put(ProducerConfig.BATCH_SIZE_CONFIG, 10); // to send messages in one batch

instrumentation/kafka/kafka-streams-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkastreams/v0_11/PartitionGroupInstrumentation.java

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

88
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
99
import static io.opentelemetry.javaagent.instrumentation.kafkastreams.v0_11.KafkaStreamsSingletons.instrumenter;
10-
import static io.opentelemetry.javaagent.instrumentation.kafkastreams.v0_11.StateHolder.HOLDER;
10+
import static io.opentelemetry.javaagent.instrumentation.kafkastreams.v0_11.StateHolder.holder;
1111
import static net.bytebuddy.matcher.ElementMatchers.isPackagePrivate;
1212
import static net.bytebuddy.matcher.ElementMatchers.named;
1313
import static net.bytebuddy.matcher.ElementMatchers.returns;
@@ -18,6 +18,7 @@
1818
import io.opentelemetry.instrumentation.kafkaclients.common.v0_11.internal.KafkaProcessRequest;
1919
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
2020
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
21+
import javax.annotation.Nullable;
2122
import net.bytebuddy.asm.Advice;
2223
import net.bytebuddy.description.type.TypeDescription;
2324
import net.bytebuddy.matcher.ElementMatcher;
@@ -44,13 +45,13 @@ public void transform(TypeTransformer transformer) {
4445
public static class NextRecordAdvice {
4546

4647
@Advice.OnMethodExit(suppress = Throwable.class, inline = false)
47-
public static void onExit(@Advice.Return StampedRecord record) {
48+
public static void onExit(@Advice.Return @Nullable StampedRecord record) {
4849
if (record == null) {
4950
return;
5051
}
5152

52-
StateHolder holder = HOLDER.get();
53-
if (holder == null) {
53+
StateHolder stateHolder = holder().get();
54+
if (stateHolder == null) {
5455
// somehow nextRecord() was called outside of process()
5556
return;
5657
}
@@ -66,7 +67,7 @@ public static void onExit(@Advice.Return StampedRecord record) {
6667
return;
6768
}
6869
Context context = instrumenter().start(parentContext, request);
69-
holder.set(request, context, context.makeCurrent());
70+
stateHolder.set(request, context, context.makeCurrent());
7071
}
7172
}
7273
}

0 commit comments

Comments
 (0)