Skip to content

Commit 723a8a0

Browse files
otelbot[bot]otelbot[bot]
authored
Code review sweep (run 25245621016) (open-telemetry#18507)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
1 parent fb14c6f commit 723a8a0

10 files changed

Lines changed: 66 additions & 53 deletions

File tree

‎instrumentation/spring/spring-jms/spring-jms-2.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/jms/v2_0/SpringTemplateTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ class SpringTemplateTest extends AbstractJmsTest {
5656
@RegisterExtension
5757
private static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
5858

59-
private static HornetQServer server;
6059
private static final String MESSAGE_TEXT = "a message";
60+
61+
private static HornetQServer server;
6162
private static JmsTemplate template;
6263
private static Session session;
6364
private static Connection connection;

‎instrumentation/spring/spring-jms/spring-jms-2.0/testing/src/main/java/io/opentelemetry/instrumentation/spring/jms/v2_0/AbstractJmsTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.List;
2727
import javax.annotation.Nullable;
2828

29+
@SuppressWarnings("deprecation") // using deprecated semconv
2930
public abstract class AbstractJmsTest {
3031

3132
protected void assertProducerSpan(
@@ -38,7 +39,6 @@ protected void assertProducerSpan(
3839
.hasAttributesSatisfyingExactly(attributeAssertions);
3940
}
4041

41-
@SuppressWarnings("deprecation") // using deprecated semconv
4242
protected List<AttributeAssertion> producerAttributeAssertions(
4343
String destinationName, boolean testHeaders) {
4444
List<AttributeAssertion> attributeAssertions =
@@ -82,7 +82,6 @@ protected void assertConsumerSpan(
8282
consumerAttributeAssertions(destinationName, testHeaders, operation, msgId));
8383
}
8484

85-
@SuppressWarnings("deprecation") // using deprecated semconv
8685
protected List<AttributeAssertion> consumerAttributeAssertions(
8786
String destinationName, boolean testHeaders, String operation, @Nullable String msgId) {
8887
List<AttributeAssertion> attributeAssertions =

‎instrumentation/spring/spring-jms/spring-jms-6.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/jms/v6_0/AbstractSpringJmsListenerTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,14 @@
3030
import org.testcontainers.containers.wait.strategy.Wait;
3131

3232
abstract class AbstractSpringJmsListenerTest {
33-
static final Logger logger = LoggerFactory.getLogger(AbstractSpringJmsListenerTest.class);
33+
private static final Logger logger = LoggerFactory.getLogger(AbstractSpringJmsListenerTest.class);
3434

3535
@RegisterExtension
3636
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
3737

3838
@RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
3939

40-
static GenericContainer<?> broker;
40+
private static GenericContainer<?> broker;
4141

4242
@BeforeAll
4343
static void setUp() {

‎instrumentation/spring/spring-kafka-2.7/javaagent/build.gradle.kts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,18 @@ tasks {
7777

7878
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
7979
jvmArgs("-Dotel.instrumentation.kafka.experimental-span-attributes=true")
80-
systemProperty("metadataConfig", "otel.instrumentation.kafka.experimental-span-attributes=true")
80+
systemProperty(
81+
"metadataConfig",
82+
"otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true,otel.instrumentation.kafka.experimental-span-attributes=true",
83+
)
8184
}
8285

8386
test {
8487
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
88+
systemProperty(
89+
"metadataConfig",
90+
"otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true",
91+
)
8592
}
8693

8794
check {

‎instrumentation/spring/spring-kafka-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/kafka/v2_7/ListenerConsumerInstrumentation.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ private AdviceScope(KafkaReceiveRequest request, Context context, Scope scope) {
8989
}
9090

9191
@Nullable
92-
public static AdviceScope enter(ConsumerRecords<?, ?> records, Consumer<?, ?> consumer) {
92+
public static AdviceScope start(ConsumerRecords<?, ?> records, Consumer<?, ?> consumer) {
9393
KafkaConsumerContext consumerContext = KafkaConsumerContextUtil.get(records);
9494
Context receiveContext = consumerContext.getContext();
9595

@@ -104,7 +104,7 @@ public static AdviceScope enter(ConsumerRecords<?, ?> records, Consumer<?, ?> co
104104
return new AdviceScope(request, context, context.makeCurrent());
105105
}
106106

107-
public void exit(@Nullable Throwable throwable) {
107+
public void end(@Nullable Throwable throwable) {
108108
scope.close();
109109
batchProcessInstrumenter().end(context, request, null, throwable);
110110
}
@@ -115,15 +115,15 @@ public void exit(@Nullable Throwable throwable) {
115115
public static AdviceScope onEnter(
116116
@Advice.Argument(0) ConsumerRecords<?, ?> records,
117117
@Advice.FieldValue("consumer") Consumer<?, ?> consumer) {
118-
return AdviceScope.enter(records, consumer);
118+
return AdviceScope.start(records, consumer);
119119
}
120120

121121
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)
122122
public static void onExit(
123123
@Advice.Thrown @Nullable Throwable throwable,
124124
@Advice.Enter @Nullable AdviceScope adviceScope) {
125125
if (adviceScope != null) {
126-
adviceScope.exit(throwable);
126+
adviceScope.end(throwable);
127127
}
128128
}
129129
}

‎instrumentation/spring/spring-kafka-2.7/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/kafka/v2_7/SpringKafkaTest.java‎

Lines changed: 39 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ protected List<Class<?>> additionalSpringConfigs() {
6060
return emptyList();
6161
}
6262

63-
private static final boolean EXPERIMENTAL_ATTRIBUTES_ENABLED =
63+
private static final boolean EXPERIMENTAL_ATTRIBUTES =
6464
Boolean.getBoolean("otel.instrumentation.kafka.experimental-span-attributes");
6565

6666
@Test
@@ -95,12 +95,11 @@ void shouldCreateSpansForSingleRecordProcess() {
9595
satisfies(
9696
MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative),
9797
equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "10"),
98-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("producer")),
98+
satisfies(
99+
stringKey("messaging.client_id"), val -> val.startsWith("producer")),
99100
equalTo(
100101
stringKey("messaging.kafka.bootstrap.servers"),
101-
EXPERIMENTAL_ATTRIBUTES_ENABLED
102-
? kafka.getBootstrapServers()
103-
: null)));
102+
EXPERIMENTAL_ATTRIBUTES ? kafka.getBootstrapServers() : null)));
104103

105104
producer.set(trace.getSpan(1));
106105
},
@@ -115,7 +114,9 @@ void shouldCreateSpansForSingleRecordProcess() {
115114
equalTo(MESSAGING_DESTINATION_NAME, "testSingleTopic"),
116115
equalTo(MESSAGING_OPERATION, "receive"),
117116
equalTo(MESSAGING_KAFKA_CONSUMER_GROUP, "testSingleListener"),
118-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("consumer")),
117+
satisfies(
118+
stringKey("messaging.client_id"),
119+
val -> val.startsWith("consumer")),
119120
equalTo(MESSAGING_BATCH_MESSAGE_COUNT, 1)),
120121
span ->
121122
span.hasName("testSingleTopic process")
@@ -135,11 +136,13 @@ void shouldCreateSpansForSingleRecordProcess() {
135136
MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative),
136137
equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "10"),
137138
equalTo(MESSAGING_KAFKA_CONSUMER_GROUP, "testSingleListener"),
138-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("consumer")),
139+
satisfies(
140+
stringKey("messaging.client_id"),
141+
val -> val.startsWith("consumer")),
139142
satisfies(
140143
longKey("kafka.record.queue_time_ms"),
141144
val -> {
142-
if (EXPERIMENTAL_ATTRIBUTES_ENABLED) {
145+
if (EXPERIMENTAL_ATTRIBUTES) {
143146
val.isNotNegative();
144147
}
145148
})),
@@ -168,7 +171,7 @@ void shouldHandleFailureInKafkaListener() {
168171
equalTo(MESSAGING_DESTINATION_NAME, "testSingleTopic"),
169172
equalTo(MESSAGING_OPERATION, "receive"),
170173
equalTo(MESSAGING_KAFKA_CONSUMER_GROUP, "testSingleListener"),
171-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("consumer")),
174+
satisfies(stringKey("messaging.client_id"), val -> val.startsWith("consumer")),
172175
equalTo(MESSAGING_BATCH_MESSAGE_COUNT, 1));
173176
List<AttributeAssertion> processAttributes =
174177
asList(
@@ -180,11 +183,11 @@ void shouldHandleFailureInKafkaListener() {
180183
satisfies(MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative),
181184
equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "10"),
182185
equalTo(MESSAGING_KAFKA_CONSUMER_GROUP, "testSingleListener"),
183-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("consumer")),
186+
satisfies(stringKey("messaging.client_id"), val -> val.startsWith("consumer")),
184187
satisfies(
185188
longKey("kafka.record.queue_time_ms"),
186189
val -> {
187-
if (EXPERIMENTAL_ATTRIBUTES_ENABLED) {
190+
if (EXPERIMENTAL_ATTRIBUTES) {
188191
val.isNotNegative();
189192
}
190193
}));
@@ -212,12 +215,12 @@ void shouldHandleFailureInKafkaListener() {
212215
satisfies(
213216
MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative),
214217
equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "10"),
215-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("producer")),
218+
satisfies(
219+
stringKey("messaging.client_id"),
220+
val -> val.startsWith("producer")),
216221
equalTo(
217222
stringKey("messaging.kafka.bootstrap.servers"),
218-
EXPERIMENTAL_ATTRIBUTES_ENABLED
219-
? kafka.getBootstrapServers()
220-
: null)));
223+
EXPERIMENTAL_ATTRIBUTES ? kafka.getBootstrapServers() : null)));
221224

222225
producer.set(trace.getSpan(1));
223226
},
@@ -272,12 +275,12 @@ void shouldHandleFailureInKafkaListener() {
272275
satisfies(
273276
MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative),
274277
equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "10"),
275-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("producer")),
278+
satisfies(
279+
stringKey("messaging.client_id"),
280+
val -> val.startsWith("producer")),
276281
equalTo(
277282
stringKey("messaging.kafka.bootstrap.servers"),
278-
EXPERIMENTAL_ATTRIBUTES_ENABLED
279-
? kafka.getBootstrapServers()
280-
: null)));
283+
EXPERIMENTAL_ATTRIBUTES ? kafka.getBootstrapServers() : null)));
281284

282285
producer.set(trace.getSpan(1));
283286
},
@@ -346,12 +349,11 @@ void shouldCreateSpansForBatchReceiveAndProcess() throws InterruptedException {
346349
satisfies(
347350
MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative),
348351
equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "10"),
349-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("producer")),
352+
satisfies(
353+
stringKey("messaging.client_id"), val -> val.startsWith("producer")),
350354
equalTo(
351355
stringKey("messaging.kafka.bootstrap.servers"),
352-
EXPERIMENTAL_ATTRIBUTES_ENABLED
353-
? kafka.getBootstrapServers()
354-
: null)),
356+
EXPERIMENTAL_ATTRIBUTES ? kafka.getBootstrapServers() : null)),
355357
span ->
356358
span.hasName("testBatchTopic publish")
357359
.hasKind(SpanKind.PRODUCER)
@@ -365,12 +367,11 @@ void shouldCreateSpansForBatchReceiveAndProcess() throws InterruptedException {
365367
satisfies(
366368
MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative),
367369
equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "20"),
368-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("producer")),
370+
satisfies(
371+
stringKey("messaging.client_id"), val -> val.startsWith("producer")),
369372
equalTo(
370373
stringKey("messaging.kafka.bootstrap.servers"),
371-
EXPERIMENTAL_ATTRIBUTES_ENABLED
372-
? kafka.getBootstrapServers()
373-
: null)));
374+
EXPERIMENTAL_ATTRIBUTES ? kafka.getBootstrapServers() : null)));
374375

375376
producer1.set(trace.getSpan(1));
376377
producer2.set(trace.getSpan(2));
@@ -386,7 +387,9 @@ void shouldCreateSpansForBatchReceiveAndProcess() throws InterruptedException {
386387
equalTo(MESSAGING_DESTINATION_NAME, "testBatchTopic"),
387388
equalTo(MESSAGING_OPERATION, "receive"),
388389
equalTo(MESSAGING_KAFKA_CONSUMER_GROUP, "testBatchListener"),
389-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("consumer")),
390+
satisfies(
391+
stringKey("messaging.client_id"),
392+
val -> val.startsWith("consumer")),
390393
equalTo(MESSAGING_BATCH_MESSAGE_COUNT, 2)),
391394
span ->
392395
span.hasName("testBatchTopic process")
@@ -400,7 +403,9 @@ void shouldCreateSpansForBatchReceiveAndProcess() throws InterruptedException {
400403
equalTo(MESSAGING_DESTINATION_NAME, "testBatchTopic"),
401404
equalTo(MESSAGING_OPERATION, "process"),
402405
equalTo(MESSAGING_KAFKA_CONSUMER_GROUP, "testBatchListener"),
403-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("consumer")),
406+
satisfies(
407+
stringKey("messaging.client_id"),
408+
val -> val.startsWith("consumer")),
404409
equalTo(MESSAGING_BATCH_MESSAGE_COUNT, 2)),
405410
span -> span.hasName("consumer").hasParent(trace.getSpan(1))));
406411
}
@@ -437,12 +442,11 @@ void shouldHandleFailureInKafkaBatchListener() {
437442
satisfies(
438443
MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative),
439444
equalTo(MESSAGING_KAFKA_MESSAGE_KEY, "10"),
440-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("producer")),
445+
satisfies(
446+
stringKey("messaging.client_id"), val -> val.startsWith("producer")),
441447
equalTo(
442448
stringKey("messaging.kafka.bootstrap.servers"),
443-
EXPERIMENTAL_ATTRIBUTES_ENABLED
444-
? kafka.getBootstrapServers()
445-
: null)));
449+
EXPERIMENTAL_ATTRIBUTES ? kafka.getBootstrapServers() : null)));
446450

447451
producer.set(trace.getSpan(1));
448452
});
@@ -492,7 +496,7 @@ private static void assertReceiveSpan(SpanDataAssert span) {
492496
equalTo(MESSAGING_DESTINATION_NAME, "testBatchTopic"),
493497
equalTo(MESSAGING_OPERATION, "receive"),
494498
equalTo(MESSAGING_KAFKA_CONSUMER_GROUP, "testBatchListener"),
495-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("consumer")),
499+
satisfies(stringKey("messaging.client_id"), val -> val.startsWith("consumer")),
496500
equalTo(MESSAGING_BATCH_MESSAGE_COUNT, 1));
497501
}
498502

@@ -507,7 +511,7 @@ private static void assertProcessSpan(
507511
equalTo(MESSAGING_DESTINATION_NAME, "testBatchTopic"),
508512
equalTo(MESSAGING_OPERATION, "process"),
509513
equalTo(MESSAGING_KAFKA_CONSUMER_GROUP, "testBatchListener"),
510-
satisfies(MESSAGING_CLIENT_ID, val -> val.startsWith("consumer")),
514+
satisfies(stringKey("messaging.client_id"), val -> val.startsWith("consumer")),
511515
equalTo(MESSAGING_BATCH_MESSAGE_COUNT, 1));
512516
if (failed) {
513517
span.hasStatus(StatusData.error()).hasException(new IllegalArgumentException("boom"));

‎instrumentation/spring/spring-kafka-2.7/testing/src/main/java/io/opentelemetry/testing/AbstractSpringKafkaTest.java‎

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

88
import static org.assertj.core.api.Assertions.assertThat;
99

10-
import io.opentelemetry.api.common.AttributeKey;
1110
import io.opentelemetry.api.trace.SpanContext;
1211
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
1312
import io.opentelemetry.sdk.trace.data.LinkData;
@@ -38,8 +37,6 @@ public abstract class AbstractSpringKafkaTest {
3837

3938
private static final Logger logger = LoggerFactory.getLogger(AbstractSpringKafkaTest.class);
4039

41-
protected static final AttributeKey<String> MESSAGING_CLIENT_ID =
42-
AttributeKey.stringKey("messaging.client_id");
4340
protected static KafkaContainer kafka;
4441

4542
private ConfigurableApplicationContext applicationContext;

‎instrumentation/spring/spring-pulsar-1.0/javaagent/build.gradle.kts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ testing {
5151
testTask.configure {
5252
jvmArgs("-Dotel.instrumentation.pulsar.experimental-span-attributes=true")
5353
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=false")
54+
systemProperty(
55+
"metadataConfig",
56+
"otel.instrumentation.pulsar.experimental-span-attributes=true",
57+
)
5458
}
5559
}
5660
}
@@ -67,6 +71,10 @@ tasks {
6771
test {
6872
jvmArgs("-Dotel.instrumentation.pulsar.experimental-span-attributes=false")
6973
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
74+
systemProperty(
75+
"metadataConfig",
76+
"otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true",
77+
)
7078
}
7179

7280
check {

‎instrumentation/spring/spring-pulsar-1.0/testing/src/main/java/io/opentelemetry/instrumentation/spring/pulsar/v1_0/AbstractSpringPulsarTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ public abstract class AbstractSpringPulsarTest {
6565
private static PulsarTemplate<String> pulsarTemplate;
6666
private static PulsarClient client;
6767
private static CountDownLatch latch;
68-
protected static String brokerHost;
69-
protected static int brokerPort;
68+
private static String brokerHost;
69+
private static int brokerPort;
7070

7171
@BeforeAll
7272
@SuppressWarnings("unchecked")

‎instrumentation/spring/spring-rmi-4.0/javaagent/build.gradle.kts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ muzzle {
1313
}
1414

1515
dependencies {
16-
compileOnly("com.google.auto.value:auto-value-annotations")
17-
annotationProcessor("com.google.auto.value:auto-value")
18-
1916
bootstrap(project(":instrumentation:rmi:bootstrap"))
2017
testInstrumentation(project(":instrumentation:rmi:javaagent"))
2118

0 commit comments

Comments
 (0)