Skip to content

Commit 9e623a0

Browse files
otelbot[bot]trask
otelbot[bot]
and
trask
authored
Code review sweep (run 25132345183) (#18417)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent 095be50 commit 9e623a0

9 files changed

Lines changed: 51 additions & 27 deletions

File tree

‎instrumentation/aws-sdk/aws-sdk-2.2/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AwsSdkTelemetryBuilder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ public AwsSdkTelemetryBuilder setRecordIndividualHttpError(boolean recordIndivid
9898
* #setUseConfiguredPropagatorForMessaging(boolean)}
9999
*/
100100
@CanIgnoreReturnValue
101-
AwsSdkTelemetryBuilder setUseXrayPropagator(boolean useMessagingPropagator) {
102-
this.useXrayPropagator = useMessagingPropagator;
101+
AwsSdkTelemetryBuilder setUseXrayPropagator(boolean useXrayPropagator) {
102+
this.useXrayPropagator = useXrayPropagator;
103103
return this;
104104
}
105105

‎instrumentation/aws-sdk/aws-sdk-2.2/library/src/testBedrockRuntime/java/io/opentelemetry/instrumentation/awssdk/v2_2/internal/Aws2BedrockRuntimeTest.java‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
3434
import java.util.ArrayList;
3535
import java.util.List;
36-
import java.util.concurrent.ExecutionException;
3736
import java.util.function.Consumer;
3837
import org.junit.jupiter.api.BeforeAll;
3938
import org.junit.jupiter.api.Test;
@@ -403,8 +402,7 @@ void testConverseToolCallNoMessageContent() {
403402
}
404403

405404
@Test
406-
void testConverseToolCallStreamNoMessageContent()
407-
throws InterruptedException, ExecutionException {
405+
void testConverseToolCallStreamNoMessageContent() {
408406
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
409407
AwsSdkTelemetry telemetry =
410408
AwsSdkTelemetry.builder(testing.getOpenTelemetry())
@@ -472,7 +470,7 @@ void testConverseToolCallStreamNoMessageContent()
472470
.toolConfig(currentWeatherToolConfig())
473471
.build(),
474472
responseHandler)
475-
.get();
473+
.join();
476474

477475
if (currentToolArgs.length() > 0 && !responseChunksTools.isEmpty()) {
478476
JsonNode node = JsonNode.parser().parse(currentToolArgs.toString());
@@ -650,7 +648,7 @@ void testConverseToolCallStreamNoMessageContent()
650648
.toolConfig(currentWeatherToolConfig())
651649
.build(),
652650
responseHandler1)
653-
.get();
651+
.join();
654652

655653
assertThat(String.join("", responseChunks))
656654
.contains(
@@ -1191,8 +1189,7 @@ void testInvokeModelToolCallAmazonNovaNoMessageContent() {
11911189
}
11921190

11931191
@Test
1194-
void testInvokeModelWithResponseStreamToolCallAmazonNovaNoMessageContent()
1195-
throws InterruptedException, ExecutionException {
1192+
void testInvokeModelWithResponseStreamToolCallAmazonNovaNoMessageContent() {
11961193
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
11971194
AwsSdkTelemetry telemetry =
11981195
AwsSdkTelemetry.builder(testing.getOpenTelemetry())
@@ -1337,7 +1334,7 @@ public void accept(PayloadPart chunk) {
13371334
.build())
13381335
.build();
13391336

1340-
client.invokeModelWithResponseStream(request0, responseHandler0).get();
1337+
client.invokeModelWithResponseStream(request0, responseHandler0).join();
13411338

13421339
String seattleToolUseId0 = "";
13431340
String sanFranciscoToolUseId0 = "";
@@ -1557,7 +1554,7 @@ public void accept(PayloadPart chunk) {
15571554
.build())
15581555
.build();
15591556

1560-
client.invokeModelWithResponseStream(request1, responseHandler1).get();
1557+
client.invokeModelWithResponseStream(request1, responseHandler1).join();
15611558

15621559
assertThat(text.toString())
15631560
.contains(
@@ -2027,8 +2024,7 @@ void testInvokeModelToolCallAnthropicClaudeNoMessageContent() {
20272024
}
20282025

20292026
@Test
2030-
void testInvokeModelWithResponseStreamToolCallAnthropicClaudeNoMessageContent()
2031-
throws InterruptedException, ExecutionException {
2027+
void testInvokeModelWithResponseStreamToolCallAnthropicClaudeNoMessageContent() {
20322028
BedrockRuntimeAsyncClientBuilder builder = BedrockRuntimeAsyncClient.builder();
20332029
AwsSdkTelemetry telemetry =
20342030
AwsSdkTelemetry.builder(testing.getOpenTelemetry())
@@ -2176,7 +2172,7 @@ public void accept(PayloadPart chunk) {
21762172
.build())
21772173
.build();
21782174

2179-
client.invokeModelWithResponseStream(request0, responseHandler0).get();
2175+
client.invokeModelWithResponseStream(request0, responseHandler0).join();
21802176

21812177
String seattleToolUseId0 = "";
21822178
String sanFranciscoToolUseId0 = "";
@@ -2370,7 +2366,7 @@ public void accept(PayloadPart chunk) {
23702366
.build())
23712367
.build();
23722368

2373-
client.invokeModelWithResponseStream(request1, responseHandler1).get();
2369+
client.invokeModelWithResponseStream(request1, responseHandler1).join();
23742370

23752371
assertThat(text.toString())
23762372
.contains(

‎instrumentation/azure-core/azure-core-1.53/javaagent/src/testAzure/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_53/AzureSdkTest.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77

88
import static io.opentelemetry.api.common.AttributeKey.stringKey;
99
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
10+
import static io.opentelemetry.semconv.HttpAttributes.HTTP_REQUEST_METHOD;
1011
import static io.opentelemetry.semconv.HttpAttributes.HTTP_RESPONSE_STATUS_CODE;
12+
import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS;
13+
import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT;
14+
import static io.opentelemetry.semconv.UrlAttributes.URL_FULL;
1115
import static org.assertj.core.api.Assertions.assertThat;
1216

1317
import com.azure.core.annotation.ExpectedResponses;
@@ -105,7 +109,12 @@ void testPipelineAndSuppression() {
105109
span.hasKind(SpanKind.CLIENT)
106110
.hasName("GET")
107111
.hasStatus(StatusData.unset())
108-
.hasAttribute(HTTP_RESPONSE_STATUS_CODE, 200L)));
112+
.hasAttributesSatisfyingExactly(
113+
equalTo(HTTP_REQUEST_METHOD, "GET"),
114+
equalTo(HTTP_RESPONSE_STATUS_CODE, 200L),
115+
equalTo(SERVER_ADDRESS, "azure.com"),
116+
equalTo(SERVER_PORT, 443),
117+
equalTo(URL_FULL, "https://azure.com/path"))));
109118
}
110119

111120
@Test

‎instrumentation/camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/ActiveContextManager.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,11 @@ private static class ContextWithScope {
8484
private final CamelRequest request;
8585
@Nullable private final Scope scope;
8686

87-
ContextWithScope(ContextWithScope parent, Context context, CamelRequest request, Scope scope) {
87+
ContextWithScope(
88+
@Nullable ContextWithScope parent,
89+
@Nullable Context context,
90+
CamelRequest request,
91+
@Nullable Scope scope) {
8892
this.parent = parent;
8993
this.context = context;
9094
this.request = request;
@@ -97,11 +101,12 @@ static ContextWithScope activate(
97101
return new ContextWithScope(parent, context, request, scope);
98102
}
99103

104+
@Nullable
100105
ContextWithScope getParent() {
101106
return parent;
102107
}
103108

104-
void deactivate(Exception exception) {
109+
void deactivate(@Nullable Exception exception) {
105110
if (scope == null) {
106111
return;
107112
}

‎instrumentation/camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/decorators/DecoratorRegistry.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212

1313
public class DecoratorRegistry {
1414

15-
private static final SpanDecorator DEFAULT = new BaseSpanDecorator();
16-
private static final Map<String, SpanDecorator> DECORATORS = loadDecorators();
15+
private static final SpanDecorator defaultDecorator = new BaseSpanDecorator();
16+
private static final Map<String, SpanDecorator> decorators = loadDecorators();
1717

1818
private static Map<String, SpanDecorator> loadDecorators() {
1919
Map<String, SpanDecorator> result = new HashMap<>();
@@ -64,6 +64,6 @@ private static Map<String, SpanDecorator> loadDecorators() {
6464

6565
public SpanDecorator forComponent(String component) {
6666

67-
return DECORATORS.getOrDefault(component, DEFAULT);
67+
return decorators.getOrDefault(component, defaultDecorator);
6868
}
6969
}

‎instrumentation/camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/decorators/RestSpanDecorator.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@
2828
import java.io.UnsupportedEncodingException;
2929
import java.net.URLDecoder;
3030
import java.util.logging.Logger;
31+
import javax.annotation.Nullable;
3132
import org.apache.camel.Endpoint;
3233
import org.apache.camel.Exchange;
3334

3435
class RestSpanDecorator extends HttpSpanDecorator {
3536

3637
private static final Logger logger = Logger.getLogger(RestSpanDecorator.class.getName());
3738

39+
@Nullable
3840
@Override
3941
protected String getPath(Exchange exchange, Endpoint endpoint) {
4042
String endpointUri = endpoint.getEndpointUri();

‎instrumentation/camel-2.20/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/TwoServicesWithDirectClientCamelTest.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@
2626
import com.google.common.collect.ImmutableMap;
2727
import io.opentelemetry.api.trace.SpanKind;
2828
import io.opentelemetry.instrumentation.test.utils.PortUtils;
29+
import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
2930
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
3031
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerUsingTest;
3132
import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension;
3233
import org.apache.camel.CamelContext;
3334
import org.apache.camel.ProducerTemplate;
3435
import org.apache.camel.builder.RouteBuilder;
3536
import org.apache.camel.impl.DefaultCamelContext;
36-
import org.junit.jupiter.api.AfterAll;
3737
import org.junit.jupiter.api.BeforeAll;
3838
import org.junit.jupiter.api.Test;
3939
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -45,6 +45,8 @@ class TwoServicesWithDirectClientCamelTest
4545
@RegisterExtension
4646
static final InstrumentationExtension testing = HttpServerInstrumentationExtension.forAgent();
4747

48+
@RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
49+
4850
private static CamelContext clientContext;
4951

5052
private static Integer portOne;
@@ -74,11 +76,7 @@ protected String getContextPath() {
7476
@BeforeAll
7577
protected void setUp() {
7678
startServer();
77-
}
78-
79-
@AfterAll
80-
protected void cleanUp() {
81-
cleanupServer();
79+
cleanup.deferAfterAll(this::cleanupServer);
8280
}
8381

8482
void createAndStartClient() throws Exception {
@@ -94,6 +92,7 @@ public void configure() {
9492
}
9593
});
9694
clientContext.start();
95+
cleanup.deferAfterAll(clientContext::stop);
9796
}
9897

9998
@Test

‎instrumentation/cassandra/cassandra-3.0/javaagent/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ muzzle {
1717
group.set("com.datastax.cassandra")
1818
module.set("cassandra-driver-core")
1919
versions.set("[3.0,4.0)")
20+
assertInverse.set(true)
2021
// While com.datastax.cassandra uses old versions of Guava, users may depends themselves on newer versions of Guava
2122
extraDependency("com.google.guava:guava:27.0-jre")
2223
}

‎instrumentation/cassandra/cassandra-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v3_0/TracingSession.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ public ResultSetFuture executeAsync(String query) {
116116
ResultSetFuture future = session.executeAsync(query);
117117
addCallbackToEndSpan(future, context, request);
118118
return future;
119+
} catch (Throwable t) {
120+
instrumenter().end(context, request, null, t);
121+
throw t;
119122
}
120123
}
121124

@@ -127,6 +130,9 @@ public ResultSetFuture executeAsync(String query, Object... values) {
127130
ResultSetFuture future = session.executeAsync(query, values);
128131
addCallbackToEndSpan(future, context, request);
129132
return future;
133+
} catch (Throwable t) {
134+
instrumenter().end(context, request, null, t);
135+
throw t;
130136
}
131137
}
132138

@@ -138,6 +144,9 @@ public ResultSetFuture executeAsync(String query, Map<String, Object> values) {
138144
ResultSetFuture future = session.executeAsync(query, values);
139145
addCallbackToEndSpan(future, context, request);
140146
return future;
147+
} catch (Throwable t) {
148+
instrumenter().end(context, request, null, t);
149+
throw t;
141150
}
142151
}
143152

@@ -151,6 +160,9 @@ public ResultSetFuture executeAsync(Statement statement) {
151160
ResultSetFuture future = session.executeAsync(statement);
152161
addCallbackToEndSpan(future, context, request);
153162
return future;
163+
} catch (Throwable t) {
164+
instrumenter().end(context, request, null, t);
165+
throw t;
154166
}
155167
}
156168

0 commit comments

Comments
 (0)