Skip to content

Commit d9fba53

Browse files
otelbot[bot]trask
otelbot[bot]
and
trask
authored
Code review sweep (run 24948463943) (open-telemetry#18307)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent 39b8467 commit d9fba53

13 files changed

Lines changed: 70 additions & 85 deletions

‎instrumentation/aws-sdk/aws-sdk-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v2_2/AwsSdkSingletons.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010

1111
public class AwsSdkSingletons {
1212

13-
private static final AwsSdkTelemetry TELEMETRY = AwsSdkTelemetryFactory.telemetry();
13+
private static final AwsSdkTelemetry telemetry = AwsSdkTelemetryFactory.telemetry();
1414

1515
public static AwsSdkTelemetry telemetry() {
16-
return TELEMETRY;
16+
return telemetry;
1717
}
1818

1919
private AwsSdkSingletons() {}

‎instrumentation/aws-sdk/aws-sdk-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v2_2/DefaultBedrockRuntimeAsyncClientBuilderInstrumentation.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.javaagent.instrumentation.awssdk.v2_2;
77

8+
import static io.opentelemetry.javaagent.instrumentation.awssdk.v2_2.AwsSdkSingletons.telemetry;
89
import static net.bytebuddy.matcher.ElementMatchers.named;
910

1011
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
@@ -36,7 +37,7 @@ public static class BuildClientAdvice {
3637
@Advice.OnMethodExit(suppress = Throwable.class, inline = false)
3738
public static BedrockRuntimeAsyncClient methodExit(
3839
@Advice.Return BedrockRuntimeAsyncClient client) {
39-
return AwsSdkSingletons.telemetry().wrapBedrockRuntimeClient(client);
40+
return telemetry().wrapBedrockRuntimeClient(client);
4041
}
4142
}
4243
}

‎instrumentation/aws-sdk/aws-sdk-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v2_2/DefaultSqsAsyncClientBuilderInstrumentation.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.javaagent.instrumentation.awssdk.v2_2;
77

8+
import static io.opentelemetry.javaagent.instrumentation.awssdk.v2_2.AwsSdkSingletons.telemetry;
89
import static net.bytebuddy.matcher.ElementMatchers.named;
910

1011
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
@@ -34,7 +35,7 @@ public static class BuildClientAdvice {
3435
@AssignReturned.ToReturned
3536
@Advice.OnMethodExit(suppress = Throwable.class, inline = false)
3637
public static SqsAsyncClient methodExit(@Advice.Return SqsAsyncClient sqsClient) {
37-
return AwsSdkSingletons.telemetry().wrap(sqsClient);
38+
return telemetry().wrap(sqsClient);
3839
}
3940
}
4041
}

‎instrumentation/aws-sdk/aws-sdk-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v2_2/DefaultSqsClientBuilderInstrumentation.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.javaagent.instrumentation.awssdk.v2_2;
77

8+
import static io.opentelemetry.javaagent.instrumentation.awssdk.v2_2.AwsSdkSingletons.telemetry;
89
import static net.bytebuddy.matcher.ElementMatchers.named;
910

1011
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
@@ -34,7 +35,7 @@ public static class BuildClientAdvice {
3435
@AssignReturned.ToReturned
3536
@Advice.OnMethodExit(suppress = Throwable.class, inline = false)
3637
public static SqsClient methodExit(@Advice.Return SqsClient sqsClient) {
37-
return AwsSdkSingletons.telemetry().wrap(sqsClient);
38+
return telemetry().wrap(sqsClient);
3839
}
3940
}
4041
}

‎instrumentation/aws-sdk/aws-sdk-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
package io.opentelemetry.javaagent.instrumentation.awssdk.v2_2;
77

8+
import static io.opentelemetry.javaagent.instrumentation.awssdk.v2_2.AwsSdkSingletons.telemetry;
9+
810
import java.io.InputStream;
911
import java.nio.ByteBuffer;
1012
import java.util.Optional;
@@ -25,8 +27,7 @@
2527
*/
2628
public class TracingExecutionInterceptor implements ExecutionInterceptor {
2729

28-
private final ExecutionInterceptor delegate =
29-
AwsSdkSingletons.telemetry().createExecutionInterceptor();
30+
private final ExecutionInterceptor delegate = telemetry().createExecutionInterceptor();
3031

3132
@Override
3233
public void beforeExecution(

‎instrumentation/aws-sdk/aws-sdk-2.2/testing/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2BedrockRuntimeTest.java‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import java.net.URI;
4040
import java.util.ArrayList;
4141
import java.util.List;
42-
import java.util.concurrent.ExecutionException;
4342
import java.util.function.Consumer;
4443
import java.util.stream.Stream;
4544
import org.junit.jupiter.api.Test;
@@ -669,7 +668,7 @@ void testConverseToolCall() {
669668
}
670669

671670
@Test
672-
void testConverseToolCallStream() throws InterruptedException, ExecutionException {
671+
void testConverseToolCallStream() {
673672
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
674673
builder.overrideConfiguration(createOverrideConfigurationBuilder().build());
675674
configureClient(builder);
@@ -730,7 +729,7 @@ void testConverseToolCallStream() throws InterruptedException, ExecutionExceptio
730729
.toolConfig(currentWeatherToolConfig())
731730
.build(),
732731
responseHandler)
733-
.get();
732+
.join();
734733

735734
if (currentToolArgs.length() > 0 && !responseChunksTools.isEmpty()) {
736735
JsonNode node = JsonNode.parser().parse(currentToolArgs.toString());
@@ -922,7 +921,7 @@ void testConverseToolCallStream() throws InterruptedException, ExecutionExceptio
922921
.toolConfig(currentWeatherToolConfig())
923922
.build(),
924923
responseHandler1)
925-
.get();
924+
.join();
926925

927926
assertThat(String.join("", responseChunks))
928927
.contains(
@@ -1103,7 +1102,7 @@ private static ToolConfiguration currentWeatherToolConfig() {
11031102
}
11041103

11051104
@Test
1106-
void testConverseStream() throws InterruptedException, ExecutionException {
1105+
void testConverseStream() {
11071106
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
11081107
builder.overrideConfiguration(createOverrideConfigurationBuilder().build());
11091108
configureClient(builder);
@@ -1135,7 +1134,7 @@ void testConverseStream() throws InterruptedException, ExecutionException {
11351134
.build())
11361135
.build(),
11371136
responseHandler)
1138-
.get();
1137+
.join();
11391138

11401139
assertThat(String.join("", responseChunks)).isEqualTo("\"Test, test\"");
11411140

@@ -1222,7 +1221,7 @@ void testConverseStream() throws InterruptedException, ExecutionException {
12221221
}
12231222

12241223
@Test
1225-
void testConverseStreamOptions() throws InterruptedException, ExecutionException {
1224+
void testConverseStreamOptions() {
12261225
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
12271226
builder.overrideConfiguration(createOverrideConfigurationBuilder().build());
12281227
configureClient(builder);
@@ -1261,7 +1260,7 @@ void testConverseStreamOptions() throws InterruptedException, ExecutionException
12611260
.build())
12621261
.build(),
12631262
responseHandler)
1264-
.get();
1263+
.join();
12651264

12661265
assertThat(String.join("", responseChunks)).isEqualTo("This model");
12671266

@@ -1439,8 +1438,7 @@ void testInvokeModelAmazonTitan() {
14391438
}
14401439

14411440
@Test
1442-
void testInvokeModelWithResponseStreamAmazonTitan()
1443-
throws InterruptedException, ExecutionException {
1441+
void testInvokeModelWithResponseStreamAmazonTitan() {
14441442
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
14451443
builder.overrideConfiguration(createOverrideConfigurationBuilder().build());
14461444
configureClient(builder);
@@ -1488,7 +1486,7 @@ void testInvokeModelWithResponseStreamAmazonTitan()
14881486
.build())
14891487
.build();
14901488

1491-
client.invokeModelWithResponseStream(request, responseHandler).get();
1489+
client.invokeModelWithResponseStream(request, responseHandler).join();
14921490

14931491
assertThat(text.toString()).contains("Here is the list of every country in the world");
14941492

@@ -1736,8 +1734,7 @@ void testInvokeModelAmazonNova() {
17361734
}
17371735

17381736
@Test
1739-
void testInvokeModelWithResponseStreamAmazonNova()
1740-
throws InterruptedException, ExecutionException {
1737+
void testInvokeModelWithResponseStreamAmazonNova() {
17411738
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
17421739
builder.overrideConfiguration(createOverrideConfigurationBuilder().build());
17431740
configureClient(builder);
@@ -1804,7 +1801,7 @@ void testInvokeModelWithResponseStreamAmazonNova()
18041801
.build())
18051802
.build();
18061803

1807-
client.invokeModelWithResponseStream(request, responseHandler).get();
1804+
client.invokeModelWithResponseStream(request, responseHandler).join();
18081805

18091806
assertThat(text.toString())
18101807
.contains("Listing every country in the world is a comprehensive task");
@@ -2278,8 +2275,7 @@ void testInvokeModelMistralMistral() {
22782275
}
22792276

22802277
@Test
2281-
void testInvokeModelWithResponseStreamAnthropicClaude()
2282-
throws InterruptedException, ExecutionException {
2278+
void testInvokeModelWithResponseStreamAnthropicClaude() {
22832279
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
22842280
builder.overrideConfiguration(createOverrideConfigurationBuilder().build());
22852281
configureClient(builder);
@@ -2340,7 +2336,7 @@ void testInvokeModelWithResponseStreamAnthropicClaude()
23402336
.build())
23412337
.build();
23422338

2343-
client.invokeModelWithResponseStream(request, responseHandler).get();
2339+
client.invokeModelWithResponseStream(request, responseHandler).join();
23442340

23452341
assertThat(text.toString()).contains("Unfortunately I do not have a complete list of every");
23462342

@@ -2860,8 +2856,7 @@ void testInvokeModelToolCallAmazonNova() {
28602856
}
28612857

28622858
@Test
2863-
void testInvokeModelWithResponseStreamToolCallAmazonNova()
2864-
throws InterruptedException, ExecutionException {
2859+
void testInvokeModelWithResponseStreamToolCallAmazonNova() {
28652860
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
28662861
builder.overrideConfiguration(createOverrideConfigurationBuilder().build());
28672862
configureClient(builder);
@@ -2999,7 +2994,7 @@ public void accept(PayloadPart chunk) {
29992994
.build())
30002995
.build();
30012996

3002-
client.invokeModelWithResponseStream(request0, responseHandler0).get();
2997+
client.invokeModelWithResponseStream(request0, responseHandler0).join();
30032998

30042999
String seattleToolUseId0 = "";
30053000
String sanFranciscoToolUseId0 = "";
@@ -3233,7 +3228,7 @@ public void accept(PayloadPart chunk) {
32333228
.build())
32343229
.build();
32353230

3236-
client.invokeModelWithResponseStream(request1, responseHandler1).get();
3231+
client.invokeModelWithResponseStream(request1, responseHandler1).join();
32373232

32383233
assertThat(text.toString())
32393234
.contains(
@@ -3770,8 +3765,7 @@ void testInvokeModelToolCallAnthropicClaude() {
37703765
}
37713766

37723767
@Test
3773-
void testInvokeModelWithResponseStreamToolCallAnthropicClaude()
3774-
throws InterruptedException, ExecutionException {
3768+
void testInvokeModelWithResponseStreamToolCallAnthropicClaude() {
37753769
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
37763770
builder.overrideConfiguration(createOverrideConfigurationBuilder().build());
37773771
configureClient(builder);
@@ -3912,7 +3906,7 @@ public void accept(PayloadPart chunk) {
39123906
.build())
39133907
.build();
39143908

3915-
client.invokeModelWithResponseStream(request0, responseHandler0).get();
3909+
client.invokeModelWithResponseStream(request0, responseHandler0).join();
39163910

39173911
String seattleToolUseId0 = "";
39183912
String sanFranciscoToolUseId0 = "";
@@ -4120,7 +4114,7 @@ public void accept(PayloadPart chunk) {
41204114
.build())
41214115
.build();
41224116

4123-
client.invokeModelWithResponseStream(request1, responseHandler1).get();
4117+
client.invokeModelWithResponseStream(request1, responseHandler1).join();
41244118

41254119
assertThat(text.toString())
41264120
.contains(

‎instrumentation/aws-sdk/aws-sdk-2.2/testing/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientCoreTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ protected static <T, U> T wrapClient(
315315
asyncClientClass.getMethod(method.getName(), method.getParameterTypes());
316316
CompletableFuture<?> future =
317317
(CompletableFuture<?>) asyncMethod.invoke(asyncClient, args);
318-
return future.get();
318+
return future.join();
319319
});
320320
}
321321

‎instrumentation/aws-sdk/aws-sdk-2.2/testing/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2ClientRecordHttpErrorTest.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,25 @@
5656
import software.amazon.awssdk.services.dynamodb.model.PutItemRequest;
5757

5858
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
59-
@SuppressWarnings("deprecation") // using deprecated semconv
6059
public abstract class AbstractAws2ClientRecordHttpErrorTest {
6160
private static final StaticCredentialsProvider CREDENTIALS_PROVIDER =
6261
StaticCredentialsProvider.create(
6362
AwsBasicCredentials.create("my-access-key", "my-secret-key"));
6463

6564
private static final MockWebServerExtension server = new MockWebServerExtension();
66-
protected static List<String> httpErrorMessages = new ArrayList<>();
65+
protected static final List<String> httpErrorMessages = new ArrayList<>();
6766

6867
@BeforeAll
69-
public static void setup() {
68+
static void setup() {
7069
server.start();
7170
}
7271

7372
@AfterAll
74-
public static void cleanup() {
73+
static void cleanup() {
7574
server.stop();
7675
}
7776

78-
public abstract ClientOverrideConfiguration.Builder createOverrideConfigurationBuilder();
77+
protected abstract ClientOverrideConfiguration.Builder createOverrideConfigurationBuilder();
7978

8079
protected abstract InstrumentationExtension getTesting();
8180

@@ -124,15 +123,15 @@ private static void cleanResponses() {
124123
httpErrorMessages.clear();
125124
}
126125

127-
public boolean isRecordIndividualHttpErrorEnabled() {
126+
protected boolean isRecordIndividualHttpErrorEnabled() {
128127
// See io.opentelemetry.instrumentation.awssdk.v2_2.internal.AwsSdkTelemetryFactory
129128
return Boolean.getBoolean(
130129
"otel.instrumentation.aws-sdk.experimental-record-individual-http-error");
131130
}
132131

133132
@SuppressWarnings("deprecation") // using deprecated semconv
134133
@Test
135-
public void testSendDynamoDbRequestWithRetries() {
134+
void testSendDynamoDbRequestWithRetries() {
136135
cleanResponses();
137136
// Setup and configuration
138137
String service = "DynamoDb";

0 commit comments

Comments
 (0)