Skip to content

Commit 0989282

Browse files
otelbot[bot]trask
andauthored
Code review sweep (run 25092961388) (#18402)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent c3dc69c commit 0989282

10 files changed

Lines changed: 74 additions & 42 deletions

File tree

instrumentation/undertow-1.4/javaagent/build.gradle.kts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ dependencies {
1919
bootstrap(project(":instrumentation:undertow-1.4:bootstrap"))
2020
}
2121

22-
tasks.withType<Test>().configureEach {
22+
tasks.test {
2323
systemProperty("collectMetadata", otelProps.collectMetadata)
2424
}
2525

instrumentation/vertx/vertx-http-client/vertx-http-client-3.0/metadata.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,31 +8,37 @@ semantic_conventions:
88
library_link: https://vertx.io/docs/vertx-core/java/
99
configurations:
1010
- name: otel.instrumentation.http.known-methods
11+
declarative_name: java.common.http.known_methods
1112
description: >
1213
Configures the instrumentation to recognize an alternative set of HTTP request methods. All
1314
other methods will be treated as `_OTHER`.
1415
type: list
1516
default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
1617
- name: otel.instrumentation.http.client.capture-request-headers
18+
declarative_name: general.http.client.request_captured_headers
1719
description: List of HTTP request headers to capture in HTTP client telemetry.
1820
type: list
1921
default: ""
2022
- name: otel.instrumentation.http.client.capture-response-headers
23+
declarative_name: general.http.client.response_captured_headers
2124
description: List of HTTP response headers to capture in HTTP client telemetry.
2225
type: list
2326
default: ""
2427
- name: otel.instrumentation.common.peer-service-mapping
28+
declarative_name: java.common.peer_service_mapping
2529
description: Used to specify a mapping from host names or IP addresses to peer services.
2630
type: map
2731
default: ""
2832
- name: otel.instrumentation.http.client.emit-experimental-telemetry
33+
declarative_name: java.common.http.client.emit_experimental_telemetry/development
2934
description: >
3035
Enable the capture of experimental HTTP client telemetry. Adds the `http.request.body.size`
3136
and `http.response.body.size` attributes to spans, and records `http.client.request.size` and
3237
`http.client.response.size` metrics.
3338
type: boolean
3439
default: false
3540
- name: otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters
41+
declarative_name: general.sanitization.url.sensitive_query_parameters/development
3642
description: List of URL query parameter names whose values are redacted in URL attributes. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.
3743
type: list
3844
default: "AWSAccessKeyId,Signature,sig,X-Goog-Signature"

instrumentation/vertx/vertx-http-client/vertx-http-client-4.0/metadata.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,37 @@ semantic_conventions:
66
library_link: https://vertx.io/
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: ""
2225
- name: otel.instrumentation.common.peer-service-mapping
26+
declarative_name: java.common.peer_service_mapping
2327
description: Used to specify a mapping from host names or IP addresses to peer services.
2428
type: map
2529
default: ""
2630
- name: otel.instrumentation.http.client.emit-experimental-telemetry
31+
declarative_name: java.common.http.client.emit_experimental_telemetry/development
2732
description: >
2833
Enable the capture of experimental HTTP client telemetry. Adds the `http.request.body.size`
2934
and `http.response.body.size` attributes to spans, and records `http.client.request.size` and
3035
`http.client.response.size` metrics.
3136
type: boolean
3237
default: false
3338
- name: otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters
39+
declarative_name: general.sanitization.url.sensitive_query_parameters/development
3440
description: List of URL query parameter names whose values are redacted in URL attributes. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.
3541
type: list
3642
default: "AWSAccessKeyId,Signature,sig,X-Goog-Signature"

instrumentation/vertx/vertx-http-client/vertx-http-client-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/httpclient/v5_0/HttpRequestInstrumentation.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,20 +87,20 @@ public static class AdviceScope {
8787
private final Context context;
8888
private final Scope scope;
8989

90-
private AdviceScope(Context context, Scope scope) {
90+
private AdviceScope(Context context) {
9191
this.context = context;
92-
this.scope = scope;
92+
this.scope = context.makeCurrent();
9393
}
9494

9595
@Nullable
96-
public static AdviceScope startAndAttachContext(HttpClientRequest request) {
96+
public static AdviceScope start(HttpClientRequest request) {
9797
Context parentContext = Context.current();
9898
if (!instrumenter().shouldStart(parentContext, request)) {
9999
return null;
100100
}
101101
Context context = instrumenter().start(parentContext, request);
102102
CONTEXTS.set(request, new Contexts(parentContext, context));
103-
return new AdviceScope(context, context.makeCurrent());
103+
return new AdviceScope(context);
104104
}
105105

106106
public void end(HttpClientRequest request, @Nullable Throwable throwable) {
@@ -114,7 +114,7 @@ public void end(HttpClientRequest request, @Nullable Throwable throwable) {
114114
@Nullable
115115
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
116116
public static AdviceScope attachContext(@Advice.This HttpClientRequest request) {
117-
return AdviceScope.startAndAttachContext(request);
117+
return AdviceScope.start(request);
118118
}
119119

120120
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)

instrumentation/vertx/vertx-http-client/vertx-http-client-5.0/metadata.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,31 +7,37 @@ semantic_conventions:
77
library_link: https://vertx.io/docs/vertx-core/java/
88
configurations:
99
- name: otel.instrumentation.http.known-methods
10+
declarative_name: java.common.http.known_methods
1011
description: >
1112
Configures the instrumentation to recognize an alternative set of HTTP request methods. All
1213
other methods will be treated as `_OTHER`.
1314
type: list
1415
default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
1516
- name: otel.instrumentation.http.client.capture-request-headers
17+
declarative_name: general.http.client.request_captured_headers
1618
description: List of HTTP request headers to capture in HTTP client telemetry.
1719
type: list
1820
default: ""
1921
- name: otel.instrumentation.http.client.capture-response-headers
22+
declarative_name: general.http.client.response_captured_headers
2023
description: List of HTTP response headers to capture in HTTP client telemetry.
2124
type: list
2225
default: ""
2326
- name: otel.instrumentation.common.peer-service-mapping
27+
declarative_name: java.common.peer_service_mapping
2428
description: Used to specify a mapping from host names or IP addresses to peer services.
2529
type: map
2630
default: ""
2731
- name: otel.instrumentation.http.client.emit-experimental-telemetry
32+
declarative_name: java.common.http.client.emit_experimental_telemetry/development
2833
description: >
2934
Enable the capture of experimental HTTP client telemetry. Adds the `http.request.body.size`
3035
and `http.response.body.size` attributes to spans, and records `http.client.request.size` and
3136
`http.client.response.size` metrics.
3237
type: boolean
3338
default: false
3439
- name: otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters
40+
declarative_name: general.sanitization.url.sensitive_query_parameters/development
3541
description: List of URL query parameter names whose values are redacted in URL attributes. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.
3642
type: list
3743
default: "AWSAccessKeyId,Signature,sig,X-Goog-Signature"

instrumentation/vertx/vertx-kafka-client-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/kafkaclient/v3_6/KafkaReadStreamImplInstrumentation.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1515
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
1616
import io.vertx.core.Handler;
17+
import javax.annotation.Nullable;
1718
import net.bytebuddy.asm.Advice;
1819
import net.bytebuddy.asm.Advice.AssignReturned;
1920
import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument;
@@ -50,9 +51,13 @@ public static class HandlerAdvice {
5051

5152
@AssignReturned.ToArguments(@ToArgument(0))
5253
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
54+
@Nullable
5355
public static <K, V> Handler<ConsumerRecord<K, V>> onEnter(
54-
@Advice.Argument(0) Handler<ConsumerRecord<K, V>> handler) {
56+
@Advice.Argument(0) @Nullable Handler<ConsumerRecord<K, V>> handler) {
5557

58+
if (handler == null) {
59+
return null;
60+
}
5661
return new InstrumentedSingleRecordHandler<>(handler);
5762
}
5863
}
@@ -62,9 +67,13 @@ public static class BatchHandlerAdvice {
6267

6368
@AssignReturned.ToArguments(@ToArgument(0))
6469
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
70+
@Nullable
6571
public static <K, V> Handler<ConsumerRecords<K, V>> onEnter(
66-
@Advice.Argument(0) Handler<ConsumerRecords<K, V>> handler) {
72+
@Advice.Argument(0) @Nullable Handler<ConsumerRecords<K, V>> handler) {
6773

74+
if (handler == null) {
75+
return null;
76+
}
6877
return new InstrumentedBatchRecordsHandler<>(handler);
6978
}
7079
}

instrumentation/vertx/vertx-kafka-client-3.6/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/kafka/AbstractVertxKafkaTest.java

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static java.util.concurrent.TimeUnit.SECONDS;
2525
import static org.assertj.core.api.Assertions.assertThat;
2626

27+
import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
2728
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
2829
import io.opentelemetry.sdk.testing.assertj.AttributeAssertion;
2930
import io.vertx.core.AsyncResult;
@@ -42,9 +43,9 @@
4243
import org.apache.kafka.common.serialization.StringSerializer;
4344
import org.assertj.core.api.AbstractLongAssert;
4445
import org.assertj.core.api.AbstractStringAssert;
45-
import org.junit.jupiter.api.AfterAll;
4646
import org.junit.jupiter.api.BeforeAll;
4747
import org.junit.jupiter.api.TestInstance;
48+
import org.junit.jupiter.api.extension.RegisterExtension;
4849
import org.slf4j.Logger;
4950
import org.slf4j.LoggerFactory;
5051
import org.testcontainers.containers.output.Slf4jLogConsumer;
@@ -56,6 +57,10 @@
5657
public abstract class AbstractVertxKafkaTest {
5758

5859
private static final Logger logger = LoggerFactory.getLogger(AbstractVertxKafkaTest.class);
60+
private static final boolean EXPERIMENTAL_ATTRIBUTES =
61+
Boolean.getBoolean("otel.instrumentation.kafka.experimental-span-attributes");
62+
63+
@RegisterExtension final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
5964

6065
KafkaContainer kafka;
6166
Vertx vertx;
@@ -82,25 +87,15 @@ void setUpAll() {
8287
.withLogConsumer(new Slf4jLogConsumer(logger))
8388
.waitingFor(Wait.forLogMessage(".*started \\(kafka.server.Kafka.*Server\\).*", 1))
8489
.withStartupTimeout(Duration.ofMinutes(1));
90+
cleanup.deferAfterAll(kafka::stop);
8591
kafka.start();
8692

8793
vertx = Vertx.vertx();
94+
cleanup.deferAfterAll(() -> closeVertx(vertx));
8895
kafkaProducer = KafkaProducer.create(vertx, producerProps());
96+
cleanup.deferAfterAll(() -> closeKafkaProducer(kafkaProducer));
8997
kafkaConsumer = KafkaConsumer.create(vertx, consumerProps());
90-
}
91-
92-
@AfterAll
93-
void tearDownAll() {
94-
if (kafkaConsumer != null) {
95-
closeKafkaConsumer(kafkaConsumer);
96-
}
97-
if (kafkaProducer != null) {
98-
closeKafkaProducer(kafkaProducer);
99-
}
100-
if (vertx != null) {
101-
closeVertx(vertx);
102-
}
103-
kafka.stop();
98+
cleanup.deferAfterAll(() -> closeKafkaConsumer(kafkaConsumer));
10499
}
105100

106101
private Properties producerProps() {
@@ -180,7 +175,7 @@ protected static List<AttributeAssertion> sendAttributes(
180175
satisfies(stringKey("messaging.client_id"), val -> val.startsWith("producer")),
181176
satisfies(MESSAGING_DESTINATION_PARTITION_ID, AbstractStringAssert::isNotEmpty),
182177
satisfies(MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative)));
183-
if (Boolean.getBoolean("otel.instrumentation.kafka.experimental-span-attributes")) {
178+
if (EXPERIMENTAL_ATTRIBUTES) {
184179
assertions.add(
185180
satisfies(
186181
stringKey("messaging.kafka.bootstrap.servers"),
@@ -228,7 +223,7 @@ protected List<AttributeAssertion> processAttributes(KafkaProducerRecord<String,
228223
satisfies(stringKey("messaging.client_id"), val -> val.startsWith("consumer")),
229224
satisfies(MESSAGING_DESTINATION_PARTITION_ID, AbstractStringAssert::isNotEmpty),
230225
satisfies(MESSAGING_KAFKA_MESSAGE_OFFSET, AbstractLongAssert::isNotNegative)));
231-
if (Boolean.getBoolean("otel.instrumentation.kafka.experimental-span-attributes")) {
226+
if (EXPERIMENTAL_ATTRIBUTES) {
232227
assertions.add(
233228
satisfies(longKey("kafka.record.queue_time_ms"), AbstractLongAssert::isNotNegative));
234229
}

instrumentation/vertx/vertx-kafka-client-3.6/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/kafka/BatchRecordsHandler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package io.opentelemetry.javaagent.instrumentation.vertx.kafka;
77

88
import static java.util.concurrent.TimeUnit.SECONDS;
9+
import static org.assertj.core.api.Assertions.assertThat;
910

1011
import io.opentelemetry.instrumentation.testing.GlobalTraceUtil;
1112
import io.vertx.core.Handler;
@@ -44,7 +45,7 @@ public static void reset(int expectedBatchSize) {
4445
}
4546

4647
public static void waitForMessages() throws InterruptedException {
47-
messageReceived.await(30, SECONDS);
48+
assertThat(messageReceived.await(30, SECONDS)).isTrue();
4849
}
4950

5051
public static int getLastBatchSize() {

instrumentation/vertx/vertx-kafka-client-3.6/vertx-kafka-client-3.6-testing/build.gradle.kts

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,6 @@ testing {
2626
implementation("io.vertx:vertx-kafka-client:$version")
2727
implementation("io.vertx:vertx-codegen:$version")
2828
}
29-
30-
targets {
31-
all {
32-
testTask.configure {
33-
systemProperty(
34-
"metadataConfig",
35-
"otel.instrumentation.messaging.experimental.receive-telemetry.enabled=false",
36-
)
37-
jvmArgs("-Dotel.instrumentation.kafka.experimental-span-attributes=false")
38-
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=false")
39-
}
40-
}
41-
}
4229
}
4330
}
4431
}
@@ -52,6 +39,10 @@ tasks {
5239

5340
test {
5441
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
42+
systemProperty(
43+
"metadataConfig",
44+
"otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true",
45+
)
5546
}
5647

5748
val testExperimental by registering(Test::class) {
@@ -60,7 +51,10 @@ tasks {
6051

6152
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
6253
jvmArgs("-Dotel.instrumentation.kafka.experimental-span-attributes=true")
63-
systemProperty("metadataConfig", "otel.instrumentation.kafka.experimental-span-attributes=true")
54+
systemProperty(
55+
"metadataConfig",
56+
"otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true,otel.instrumentation.kafka.experimental-span-attributes=true",
57+
)
6458
}
6559

6660
check {

instrumentation/vertx/vertx-kafka-client-3.6/vertx-kafka-client-4-testing/build.gradle.kts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,26 @@ tasks {
4646
}
4747

4848
test {
49-
jvmArgs("-Dotel.instrumentation.kafka.experimental-span-attributes=true")
5049
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
50+
systemProperty(
51+
"metadataConfig",
52+
"otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true",
53+
)
54+
}
55+
56+
val testExperimental by registering(Test::class) {
57+
testClassesDirs = sourceSets.test.get().output.classesDirs
58+
classpath = sourceSets.test.get().runtimeClasspath
59+
60+
jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
61+
jvmArgs("-Dotel.instrumentation.kafka.experimental-span-attributes=true")
62+
systemProperty(
63+
"metadataConfig",
64+
"otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true,otel.instrumentation.kafka.experimental-span-attributes=true",
65+
)
5166
}
5267

5368
check {
54-
dependsOn(testing.suites)
69+
dependsOn(testing.suites, testExperimental)
5570
}
5671
}

0 commit comments

Comments
 (0)