Skip to content

Commit f5fd99e

Browse files
authored
Code review sweep (run 25218392073) (#18487)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
1 parent 63485ae commit f5fd99e

11 files changed

Lines changed: 58 additions & 23 deletions

File tree

instrumentation/oracle-ucp-11.2/testing/src/main/java/io/opentelemetry/instrumentation/oracleucp/AbstractOracleUcpInstrumentationTest.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ public abstract class AbstractOracleUcpInstrumentationTest {
3333
LoggerFactory.getLogger(AbstractOracleUcpInstrumentationTest.class);
3434

3535
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.oracle-ucp-11.2";
36-
private static OracleContainer oracle;
36+
private static final OracleContainer oracle =
37+
new OracleContainer("gvenzl/oracle-free:23-slim-faststart")
38+
.withLogConsumer(new Slf4jLogConsumer(logger))
39+
.withStartupTimeout(Duration.ofMinutes(2));
3740

3841
protected abstract InstrumentationExtension testing();
3942

@@ -43,18 +46,12 @@ public abstract class AbstractOracleUcpInstrumentationTest {
4346

4447
@BeforeAll
4548
static void setUp() {
46-
oracle =
47-
new OracleContainer("gvenzl/oracle-free:23-slim-faststart")
48-
.withLogConsumer(new Slf4jLogConsumer(logger))
49-
.withStartupTimeout(Duration.ofMinutes(2));
5049
oracle.start();
5150
}
5251

5352
@AfterAll
5453
static void cleanUp() {
55-
if (oracle != null) {
56-
oracle.stop();
57-
}
54+
oracle.stop();
5855
}
5956

6057
@ParameterizedTest

instrumentation/pekko/pekko-actor-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/pekkoactor/v1_0/PekkoActorCellInstrumentation.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public void transform(TypeTransformer transformer) {
4242
@SuppressWarnings("unused")
4343
public static class InvokeAdvice {
4444

45+
@Nullable
4546
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
4647
public static Scope enter(@Advice.Argument(0) Envelope envelope) {
4748
return TaskAdviceHelper.makePropagatedContextCurrent(ENVELOPE_PROPAGATED_CONTEXT, envelope);
@@ -58,6 +59,7 @@ public static void exit(@Advice.Enter @Nullable Scope scope) {
5859
@SuppressWarnings("unused")
5960
public static class SystemInvokeAdvice {
6061

62+
@Nullable
6163
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
6264
public static Scope enter(@Advice.Argument(0) SystemMessage systemMessage) {
6365
return TaskAdviceHelper.makePropagatedContextCurrent(

instrumentation/pekko/pekko-actor-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkoactor/v1_0/PekkoActorTest.scala

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,27 @@ import io.opentelemetry.instrumentation.testing.junit.{
1010
InstrumentationExtension
1111
}
1212
import io.opentelemetry.sdk.testing.assertj.{SpanDataAssert, TraceAssert}
13-
import org.junit.jupiter.api.TestInstance
13+
import org.junit.jupiter.api.{AfterAll, TestInstance}
1414
import org.junit.jupiter.api.extension.RegisterExtension
1515
import org.junit.jupiter.params.ParameterizedTest
1616
import org.junit.jupiter.params.provider.ValueSource
1717

1818
import java.util.function.Consumer
1919
import scala.collection.JavaConverters._
20+
import scala.concurrent.Await
21+
import scala.concurrent.duration.DurationInt
2022

2123
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
2224
class PekkoActorTest {
2325

2426
@RegisterExtension val testing: InstrumentationExtension =
2527
AgentInstrumentationExtension.create
2628

29+
@AfterAll
30+
def cleanup(): Unit = {
31+
Await.result(PekkoActors.system.terminate(), 10.seconds)
32+
}
33+
2734
@ParameterizedTest
2835
@ValueSource(ints = Array(1, 150))
2936
def basicTell(count: Int): Unit = {

instrumentation/pekko/pekko-actor-1.0/javaagent/src/test/scala/io/opentelemetry/javaagent/instrumentation/pekkoactor/v1_0/PekkoSchedulerTest.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class PekkoSchedulerTest {
3737
} finally {
3838
scope.close()
3939
initialSpan.end()
40-
system.terminate()
40+
Await.result(system.terminate(), 5.seconds)
4141
}
4242
}
4343

instrumentation/pekko/pekko-http-1.0/javaagent/build.gradle.kts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,9 @@ testing {
6868
val tapirTest by registering(JvmTestSuite::class) {
6969
dependencies {
7070
if (otelProps.testLatestDeps) {
71-
implementation("com.typesafe.akka:akka-http_2.13:latest.release")
72-
implementation("com.typesafe.akka:akka-stream_2.13:latest.release")
71+
implementation("org.apache.pekko:pekko-http_2.13:latest.release")
72+
implementation("org.apache.pekko:pekko-slf4j_2.13:latest.release")
73+
implementation("org.apache.pekko:pekko-stream_2.13:latest.release")
7374
implementation("com.softwaremill.sttp.tapir:tapir-pekko-http-server_2.13:latest.release")
7475
} else {
7576
implementation("org.apache.pekko:pekko-http_2.12:1.0.0")

instrumentation/pekko/pekko-http-1.0/metadata.yaml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,48 +10,58 @@ semantic_conventions:
1010
- HTTP_SERVER_METRICS
1111
features:
1212
- HTTP_ROUTE
13+
- CONTEXT_PROPAGATION
1314
configurations:
1415
- name: otel.instrumentation.http.known-methods
16+
declarative_name: java.common.http.known_methods
1517
description: >
1618
Configures the instrumentation to recognize an alternative set of HTTP request methods. All
1719
other methods will be treated as `_OTHER`.
1820
type: list
1921
default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
2022
- name: otel.instrumentation.http.client.capture-request-headers
23+
declarative_name: general.http.client.request_captured_headers
2124
description: List of HTTP request headers to capture in HTTP client telemetry.
2225
type: list
2326
default: ""
2427
- name: otel.instrumentation.http.client.capture-response-headers
28+
declarative_name: general.http.client.response_captured_headers
2529
description: List of HTTP response headers to capture in HTTP client telemetry.
2630
type: list
2731
default: ""
2832
- name: otel.instrumentation.common.peer-service-mapping
33+
declarative_name: java.common.peer_service_mapping
2934
description: Used to specify a mapping from host names or IP addresses to peer services.
3035
type: map
3136
default: ""
3237
- name: otel.instrumentation.http.client.emit-experimental-telemetry
38+
declarative_name: java.common.http.client.emit_experimental_telemetry/development
3339
description: >
3440
Enable the capture of experimental HTTP client telemetry. Adds the `http.request.body.size`
3541
and `http.response.body.size` attributes to spans, and records `http.client.request.size` and
3642
`http.client.response.size` metrics.
3743
type: boolean
3844
default: false
3945
- name: otel.instrumentation.http.server.capture-request-headers
46+
declarative_name: general.http.server.request_captured_headers
4047
description: List of HTTP request headers to capture in HTTP server telemetry.
4148
type: list
4249
default: ""
4350
- name: otel.instrumentation.http.server.capture-response-headers
51+
declarative_name: general.http.server.response_captured_headers
4452
description: List of HTTP response headers to capture in HTTP server telemetry.
4553
type: list
4654
default: ""
4755
- name: otel.instrumentation.http.server.emit-experimental-telemetry
56+
declarative_name: java.common.http.server.emit_experimental_telemetry/development
4857
description: >
4958
Enable the capture of experimental HTTP server telemetry. Adds the `http.request.body.size`
5059
and `http.response.body.size` attributes to spans, and records `http.server.request.size` and
5160
`http.server.response.size` metrics.
5261
type: boolean
5362
default: false
5463
- name: otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters
64+
declarative_name: general.sanitization.url.sensitive_query_parameters/development
5565
description: List of URL query parameter names whose values are redacted in URL attributes. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.
5666
type: list
5767
default: "AWSAccessKeyId,Signature,sig,X-Goog-Signature"

instrumentation/play/play-mvc/play-mvc-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/playmvc/v2_4/ActionInstrumentation.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public static class AdviceScope {
5454
private final Context context;
5555
private final Scope scope;
5656

57-
public AdviceScope(Context context, Scope scope) {
57+
private AdviceScope(Context context, Scope scope) {
5858
this.context = context;
5959
this.scope = scope;
6060
}
@@ -71,33 +71,34 @@ public static AdviceScope start(Context parentContext) {
7171

7272
public void end(
7373
@Nullable Throwable throwable,
74-
Future<Result> responseFuture,
74+
@Nullable Future<Result> responseFuture,
7575
Action<?> thisAction,
7676
Request<?> req) {
7777
scope.close();
7878
updateSpan(context, req);
7979

80-
if (throwable == null) {
80+
if (throwable != null || responseFuture == null) {
81+
instrumenter().end(context, null, null, throwable);
82+
} else {
8183
// span finished in RequestCompleteCallback
8284
responseFuture.onComplete(
8385
new RequestCompleteCallback(context), thisAction.executionContext());
84-
} else {
85-
instrumenter().end(context, null, null, throwable);
8686
}
8787
}
8888
}
8989

9090
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
91+
@Nullable
9192
public static AdviceScope onEnter(@Advice.Argument(0) Request<?> req) {
9293
return AdviceScope.start(currentContext());
9394
}
9495

9596
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
9697
public static void stopTraceOnResponse(
9798
@Advice.This Action<?> thisAction,
98-
@Advice.Thrown Throwable throwable,
99+
@Advice.Thrown @Nullable Throwable throwable,
99100
@Advice.Argument(0) Request<?> req,
100-
@Advice.Return Future<Result> responseFuture,
101+
@Advice.Return @Nullable Future<Result> responseFuture,
101102
@Advice.Enter @Nullable AdviceScope actionScope) {
102103
if (actionScope == null) {
103104
return;

instrumentation/play/play-mvc/play-mvc-2.6/javaagent/build.gradle.kts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,5 +79,9 @@ configurations.configureEach {
7979
}
8080
tasks.withType<Test>().configureEach {
8181
systemProperty("collectMetadata", otelProps.collectMetadata)
82+
systemProperty(
83+
"metadataConfig",
84+
"otel.instrumentation.common.experimental.controller-telemetry.enabled=true"
85+
)
8286
jvmArgs("-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true")
8387
}

instrumentation/play/play-mvc/play-mvc-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/playmvc/v2_6/ActionInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public static class AdviceScope {
5454
private final Context context;
5555
private final Scope scope;
5656

57-
public AdviceScope(Context context, Scope scope) {
57+
private AdviceScope(Context context, Scope scope) {
5858
this.context = context;
5959
this.scope = scope;
6060
}

instrumentation/play/play-ws/play-ws-2.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/playws/v2_1/PlayWsInstrumentationModule.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ public static Object[] methodEnter(
4949
@Advice.Argument(0) Request request,
5050
@Advice.Argument(1) AsyncHandler<?> originalAsyncHandler) {
5151
AsyncHandler<?> asyncHandler = originalAsyncHandler;
52+
if (asyncHandler instanceof WebSocketUpgradeHandler) {
53+
// websocket upgrade handlers aren't supported
54+
return new Object[] {null, asyncHandler};
55+
}
56+
5257
Context parentContext = currentContext();
5358
if (!instrumenter().shouldStart(parentContext, request)) {
5459
return new Object[] {null, asyncHandler};
@@ -60,8 +65,7 @@ public static Object[] methodEnter(
6065
asyncHandler =
6166
new StreamedAsyncHandlerWrapper<>(
6267
(StreamedAsyncHandler<?>) asyncHandler, request, context, parentContext);
63-
} else if (!(asyncHandler instanceof WebSocketUpgradeHandler)) {
64-
// websocket upgrade handlers aren't supported
68+
} else {
6569
asyncHandler = new AsyncHandlerWrapper<>(asyncHandler, request, context, parentContext);
6670
}
6771
return new Object[] {context, asyncHandler};
@@ -71,7 +75,10 @@ public static Object[] methodEnter(
7175
public static void methodExit(
7276
@Advice.Argument(0) Request request,
7377
@Advice.Thrown @Nullable Throwable throwable,
74-
@Advice.Enter Object[] enterResult) {
78+
@Advice.Enter @Nullable Object[] enterResult) {
79+
if (enterResult == null) {
80+
return;
81+
}
7582
Context context = (Context) enterResult[0];
7683
if (context != null && throwable != null) {
7784
instrumenter().end(context, request, null, throwable);

0 commit comments

Comments
 (0)