Skip to content

Commit a88311e

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

15 files changed

Lines changed: 64 additions & 64 deletions

File tree

‎instrumentation/openai/openai-java-1.1/library/build.gradle.kts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ dependencies {
1010
}
1111

1212
tasks {
13-
withType<Test>().configureEach {
13+
test {
1414
systemProperty("testLatestDeps", otelProps.testLatestDeps)
1515
}
1616
}

‎instrumentation/opencensus-shim/testing/src/test/java/io/opentelemetry/opencensusshim/JavaagentInstrumentationTest.java‎

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

88
import static io.opentelemetry.api.common.AttributeKey.booleanKey;
99
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
10-
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
1110

1211
import io.opencensus.trace.AttributeValue;
1312
import io.opencensus.trace.Tracing;
@@ -19,7 +18,6 @@
1918
import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
2019
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
2120
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
22-
import org.assertj.core.api.AbstractBooleanAssert;
2321
import org.junit.jupiter.api.BeforeEach;
2422
import org.junit.jupiter.api.Test;
2523
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -86,26 +84,17 @@ void testInterleavedSpansOcFirst() {
8684
span ->
8785
span.hasName("outer-span")
8886
.hasNoParent()
89-
.hasAttributesSatisfyingExactly(
90-
equalTo(booleanKey("outer"), true),
91-
satisfies(booleanKey("inner"), AbstractBooleanAssert::isNull),
92-
satisfies(booleanKey("middle"), AbstractBooleanAssert::isNull)),
87+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("outer"), true)),
9388
// middle span
9489
span ->
9590
span.hasName("mid-span")
9691
.hasParent(trace.getSpan(0))
97-
.hasAttributesSatisfyingExactly(
98-
equalTo(booleanKey("middle"), true),
99-
satisfies(booleanKey("inner"), AbstractBooleanAssert::isNull),
100-
satisfies(booleanKey("outer"), AbstractBooleanAssert::isNull)),
92+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("middle"), true)),
10193
// inner span
10294
span ->
10395
span.hasName("inner-span")
10496
.hasParent(trace.getSpan(1))
105-
.hasAttributesSatisfyingExactly(
106-
equalTo(booleanKey("inner"), true),
107-
satisfies(booleanKey("middle"), AbstractBooleanAssert::isNull),
108-
satisfies(booleanKey("outer"), AbstractBooleanAssert::isNull))));
97+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("inner"), true))));
10998
}
11099

111100
@Test
@@ -154,26 +143,17 @@ void testInterleavedSpansOtelFirst() {
154143
span ->
155144
span.hasName("outer-span")
156145
.hasNoParent()
157-
.hasAttributesSatisfyingExactly(
158-
equalTo(booleanKey("outer"), true),
159-
satisfies(booleanKey("inner"), AbstractBooleanAssert::isNull),
160-
satisfies(booleanKey("middle"), AbstractBooleanAssert::isNull)),
146+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("outer"), true)),
161147
// middle span
162148
span ->
163149
span.hasName("mid-span")
164150
.hasParent(trace.getSpan(0))
165-
.hasAttributesSatisfyingExactly(
166-
equalTo(booleanKey("middle"), true),
167-
satisfies(booleanKey("inner"), AbstractBooleanAssert::isNull),
168-
satisfies(booleanKey("outer"), AbstractBooleanAssert::isNull)),
151+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("middle"), true)),
169152
// inner span
170153
span ->
171154
span.hasName("inner-span")
172155
.hasParent(trace.getSpan(1))
173-
.hasAttributesSatisfyingExactly(
174-
equalTo(booleanKey("inner"), true),
175-
satisfies(booleanKey("middle"), AbstractBooleanAssert::isNull),
176-
satisfies(booleanKey("outer"), AbstractBooleanAssert::isNull))));
156+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("inner"), true))));
177157
}
178158

179159
@Test
@@ -288,25 +268,16 @@ void testNestedOpenCensusSpans() {
288268
span ->
289269
span.hasName("outer-span")
290270
.hasNoParent()
291-
.hasAttributesSatisfyingExactly(
292-
equalTo(booleanKey("outer"), true),
293-
satisfies(booleanKey("inner"), AbstractBooleanAssert::isNull),
294-
satisfies(booleanKey("middle"), AbstractBooleanAssert::isNull)),
271+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("outer"), true)),
295272
// middle span
296273
span ->
297274
span.hasName("mid-span")
298275
.hasParent(trace.getSpan(0))
299-
.hasAttributesSatisfyingExactly(
300-
equalTo(booleanKey("middle"), true),
301-
satisfies(booleanKey("inner"), AbstractBooleanAssert::isNull),
302-
satisfies(booleanKey("outer"), AbstractBooleanAssert::isNull)),
276+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("middle"), true)),
303277
// inner span
304278
span ->
305279
span.hasName("inner-span")
306280
.hasParent(trace.getSpan(1))
307-
.hasAttributesSatisfyingExactly(
308-
equalTo(booleanKey("inner"), true),
309-
satisfies(booleanKey("middle"), AbstractBooleanAssert::isNull),
310-
satisfies(booleanKey("outer"), AbstractBooleanAssert::isNull))));
281+
.hasAttributesSatisfyingExactly(equalTo(booleanKey("inner"), true))));
311282
}
312283
}

‎instrumentation/opensearch/opensearch-rest-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/v3_0/OpenSearchRestSingletons.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,17 @@
1313
import javax.annotation.Nullable;
1414
import org.opensearch.client.Response;
1515

16-
public class OpenSearchRestSingletons {
16+
class OpenSearchRestSingletons {
1717

1818
private static final Instrumenter<OpenSearchRestRequest, OpenSearchRestResponse> instrumenter =
1919
OpenSearchRestInstrumenterFactory.create("io.opentelemetry.opensearch-rest-3.0");
2020

21-
public static Instrumenter<OpenSearchRestRequest, OpenSearchRestResponse> instrumenter() {
21+
static Instrumenter<OpenSearchRestRequest, OpenSearchRestResponse> instrumenter() {
2222
return instrumenter;
2323
}
2424

2525
@Nullable
26-
public static OpenSearchRestResponse convertResponse(@Nullable Response response) {
26+
static OpenSearchRestResponse convertResponse(@Nullable Response response) {
2727
if (response == null) {
2828
return null;
2929
}

‎instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,11 @@ public String getNetworkType(
7272
@Nullable
7373
public String getNetworkPeerAddress(
7474
OpenSearchRestRequest request, @Nullable OpenSearchRestResponse response) {
75-
if (response != null && response.getAddress() != null) {
76-
return response.getAddress().getHostAddress();
75+
if (response != null) {
76+
InetAddress address = response.getAddress();
77+
if (address != null) {
78+
return address.getHostAddress();
79+
}
7780
}
7881
return null;
7982
}

‎instrumentation/opensearch/opensearch-rest-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/AbstractOpenSearchRestTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT;
2222
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM;
2323
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.OPENSEARCH;
24+
import static java.util.concurrent.TimeUnit.SECONDS;
2425
import static org.assertj.core.api.Assertions.assertThat;
2526

2627
import io.opentelemetry.api.trace.SpanKind;
@@ -143,7 +144,7 @@ public void onFailure(Exception e) {
143144
() -> {
144145
client.performRequestAsync(new Request("GET", "_cluster/health"), responseListener);
145146
});
146-
countDownLatch.await();
147+
assertThat(countDownLatch.await(10, SECONDS)).isTrue();
147148

148149
if (exception.get() != null) {
149150
throw exception.get();

‎instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/ApplicationOpenTelemetry.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ private static application.io.opentelemetry.api.OpenTelemetry getOpenTelemetry11
6969
"io.opentelemetry.javaagent.instrumentation.opentelemetryapi.v1_10.ApplicationOpenTelemetry110");
7070
}
7171

72+
@Nullable
7273
private static application.io.opentelemetry.api.OpenTelemetry getOpenTelemetry(String className) {
7374
try {
7475
Class<?> clazz = Class.forName(className);

‎instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/trace/Bridging.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.ArrayList;
2121
import java.util.List;
2222
import java.util.logging.Logger;
23+
import javax.annotation.Nullable;
2324

2425
/**
2526
* This class translates between the (unshaded) OpenTelemetry API that the application brings and
@@ -72,6 +73,7 @@ private static application.io.opentelemetry.api.trace.TraceState toApplication(
7273
return applicationTraceState.build();
7374
}
7475

76+
@Nullable
7577
public static Span toAgentOrNull(application.io.opentelemetry.api.trace.Span applicationSpan) {
7678
if (!applicationSpan.getSpanContext().isValid()) {
7779
// no need to wrap
@@ -83,6 +85,7 @@ public static Span toAgentOrNull(application.io.opentelemetry.api.trace.Span app
8385
}
8486
}
8587

88+
@Nullable
8689
public static SpanKind toAgentOrNull(
8790
application.io.opentelemetry.api.trace.SpanKind applicationSpanKind) {
8891
try {
@@ -131,6 +134,7 @@ public static Attributes toAgent(
131134
// TODO optimize this by storing shaded AttributeKey inside of application AttributeKey instead of
132135
// creating every time
133136
@SuppressWarnings({"rawtypes"}) // conversion uses raw AttributeKey
137+
@Nullable
134138
public static AttributeKey toAgent(
135139
application.io.opentelemetry.api.common.AttributeKey applicationKey) {
136140
switch (applicationKey.getType()) {

‎instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/ContextBridgeTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ void agentPropagatesApplicationsBaggage() throws Exception {
171171
}
172172

173173
// Then
174-
assertThat(ref.get().size()).isEqualTo(1);
174+
assertThat(ref.get().asMap()).hasSize(1);
175175
assertThat(ref.get().getEntryValue("cat")).isEqualTo("yes");
176176
}
177177

‎instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/TracerTest.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_STACKTRACE;
1717
import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_TYPE;
1818
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.entry;
1920

2021
import io.opentelemetry.api.GlobalOpenTelemetry;
2122
import io.opentelemetry.api.common.Attributes;
@@ -354,7 +355,7 @@ void captureSpanLinkWithoutAttributes() {
354355
.isEqualTo(linkedSpanContext.getTraceFlags().asByte());
355356
assertThat(link.getSpanContext().isRemote())
356357
.isEqualTo(linkedSpanContext.isRemote());
357-
assertThat(link.getAttributes().size()).isEqualTo(0);
358+
assertThat(link.getAttributes().asMap()).isEmpty();
358359
})));
359360
}
360361

@@ -401,12 +402,13 @@ void captureSpanLinkWithAttributes() {
401402
.isEqualTo(linkedSpanContext.getTraceFlags().asByte());
402403
assertThat(link.getSpanContext().isRemote())
403404
.isEqualTo(linkedSpanContext.isRemote());
404-
assertThat(link.getTotalAttributeCount()).isEqualTo(4);
405405
Attributes attrs = link.getAttributes();
406-
assertThat(attrs.get(stringKey("string"))).isEqualTo("1");
407-
assertThat(attrs.get(longKey("long"))).isEqualTo(2L);
408-
assertThat(attrs.get(doubleKey("double"))).isEqualTo(3.0);
409-
assertThat(attrs.get(booleanKey("boolean"))).isEqualTo(true);
406+
assertThat(attrs.asMap())
407+
.containsOnly(
408+
entry(stringKey("string"), "1"),
409+
entry(longKey("long"), 2L),
410+
entry(doubleKey("double"), 3.0),
411+
entry(booleanKey("boolean"), true));
410412
})));
411413
}
412414
}

‎instrumentation/opentelemetry-api/opentelemetry-api-1.10/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/v1_10/metrics/ApplicationObservableDoubleCounter.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ public ApplicationObservableDoubleCounter(
2222
// not adding @Override because this method was introduced in 1.12
2323
@SuppressWarnings("unused")
2424
public void close() {
25-
agentCounter.close();
26-
onClose.run();
25+
try {
26+
agentCounter.close();
27+
} finally {
28+
onClose.run();
29+
}
2730
}
2831
}

0 commit comments

Comments
 (0)