From 0fe76c4985cfde56852385939680aabea5156bf2 Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Sat, 2 May 2026 20:05:10 +0000 Subject: [PATCH 01/11] Review fixes for clickhouse-client-common-0.5:javaagent Automated code review of instrumentation/clickhouse/clickhouse-client-common-0.5/javaagent. --- .../client/common/v0_5/ClickHouseAttributesGetter.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation/clickhouse/clickhouse-client-common-0.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/clickhouse/client/common/v0_5/ClickHouseAttributesGetter.java b/instrumentation/clickhouse/clickhouse-client-common-0.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/clickhouse/client/common/v0_5/ClickHouseAttributesGetter.java index 2c56a077fdcc..c7538a4cea27 100644 --- a/instrumentation/clickhouse/clickhouse-client-common-0.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/clickhouse/client/common/v0_5/ClickHouseAttributesGetter.java +++ b/instrumentation/clickhouse/clickhouse-client-common-0.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/clickhouse/client/common/v0_5/ClickHouseAttributesGetter.java @@ -15,8 +15,7 @@ import java.util.function.Function; import javax.annotation.Nullable; -final class ClickHouseAttributesGetter - implements SqlClientAttributesGetter { +class ClickHouseAttributesGetter implements SqlClientAttributesGetter { private final Function errorCodeExtractor; From ef5d886a7247e64c9ae891fb5ecf6427536521d2 Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Sat, 2 May 2026 20:15:55 +0000 Subject: [PATCH 02/11] Review fixes for play-ws-common-1.0:testing Automated code review of instrumentation/play/play-ws/play-ws-common-1.0/testing. --- .../playws/common/v1_0/PlayWsClientBaseTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/play/play-ws/play-ws-common-1.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/playws/common/v1_0/PlayWsClientBaseTest.java b/instrumentation/play/play-ws/play-ws-common-1.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/playws/common/v1_0/PlayWsClientBaseTest.java index bf9d2bc5c183..b5452af42e80 100644 --- a/instrumentation/play/play-ws/play-ws-common-1.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/playws/common/v1_0/PlayWsClientBaseTest.java +++ b/instrumentation/play/play-ws/play-ws-common-1.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/playws/common/v1_0/PlayWsClientBaseTest.java @@ -44,9 +44,9 @@ abstract class PlayWsClientBaseTest extends AbstractHttpClientTest Date: Sat, 2 May 2026 20:19:27 +0000 Subject: [PATCH 03/11] Review fixes for vibur-dbcp-11.0:javaagent Automated code review of instrumentation/vibur-dbcp-11.0/javaagent. --- .../viburdbcp/v11_0/ViburDbcpInstrumentationModule.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/v11_0/ViburDbcpInstrumentationModule.java b/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/v11_0/ViburDbcpInstrumentationModule.java index ade9b62bfa57..317611010d21 100644 --- a/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/v11_0/ViburDbcpInstrumentationModule.java +++ b/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/v11_0/ViburDbcpInstrumentationModule.java @@ -24,9 +24,8 @@ public ViburDbcpInstrumentationModule() { public ElementMatcher.Junction classLoaderMatcher() { // in 11.0, the ViburDBCPDataSource#getPool() method signature was changed - this is detected by // muzzle - return hasClassesNamed( - // added in 10.0 - "org.vibur.dbcp.ViburConfig"); + // added in 10.0 + return hasClassesNamed("org.vibur.dbcp.ViburConfig"); } @Override From 1a8812cf2431c0213952a22369a887f32f5c88d1 Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Sat, 2 May 2026 20:25:51 +0000 Subject: [PATCH 04/11] Review fixes for vibur-dbcp-11.0:testing Automated code review of instrumentation/vibur-dbcp-11.0/testing. --- .../viburdbcp/AbstractViburInstrumentationTest.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java index 0f2e12b0417d..ee6c6b31d64e 100644 --- a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java +++ b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java @@ -6,13 +6,14 @@ package io.opentelemetry.instrumentation.viburdbcp; import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv; -import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.awaitility.Awaitility.await; import static org.mockito.Mockito.when; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.db.DbConnectionPoolMetricsAssertions; import java.sql.Connection; import java.sql.SQLException; +import java.time.Duration; import javax.sql.DataSource; import org.assertj.core.api.AbstractIterableAssert; import org.junit.jupiter.api.Test; @@ -24,6 +25,7 @@ @ExtendWith(MockitoExtension.class) public abstract class AbstractViburInstrumentationTest { private static final String INSTRUMENTATION_NAME = "io.opentelemetry.vibur-dbcp-11.0"; + private static final Duration EXPORTER_INTERVAL = Duration.ofMillis(100); @Mock private DataSource dataSourceMock; @Mock private Connection connectionMock; @@ -35,7 +37,7 @@ public abstract class AbstractViburInstrumentationTest { protected abstract void shutdown(ViburDBCPDataSource viburDataSource); @Test - void shouldReportMetrics() throws SQLException, InterruptedException { + void shouldReportMetrics() throws SQLException { // given when(dataSourceMock.getConnection()).thenReturn(connectionMock); @@ -47,7 +49,7 @@ void shouldReportMetrics() throws SQLException, InterruptedException { // when Connection viburConnection = viburDataSource.getConnection(); - MILLISECONDS.sleep(100); + await().pollDelay(EXPORTER_INTERVAL).until(() -> true); viburConnection.close(); // then @@ -68,9 +70,9 @@ void shouldReportMetrics() throws SQLException, InterruptedException { shutdown(viburDataSource); // sleep exporter interval - Thread.sleep(100); + await().pollDelay(EXPORTER_INTERVAL).until(() -> true); testing().clearData(); - Thread.sleep(100); + await().pollDelay(EXPORTER_INTERVAL).until(() -> true); // then String countMetricName = From cbeeb2e0205622540df83d8b0c52f425b1696c97 Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Sat, 2 May 2026 20:29:21 +0000 Subject: [PATCH 05/11] Review fixes for wicket-8.0:javaagent Automated code review of instrumentation/wicket-8.0/javaagent. --- .../wicket/v8_0/RequestHandlerExecutorInstrumentation.java | 2 +- .../instrumentation/wicket/v8_0/WicketServerSpanNaming.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/RequestHandlerExecutorInstrumentation.java b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/RequestHandlerExecutorInstrumentation.java index fa2297a6bec9..e138ff8c8706 100644 --- a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/RequestHandlerExecutorInstrumentation.java +++ b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/RequestHandlerExecutorInstrumentation.java @@ -42,7 +42,7 @@ public static void onEnter(@Advice.Argument(0) IRequestHandler handler) { HttpServerRoute.update( Java8BytecodeBridge.currentContext(), CONTROLLER, - WicketServerSpanNaming.getServerSpanName(), + WicketServerSpanNaming.serverSpanName(), (IPageClassRequestHandler) handler); } } diff --git a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/WicketServerSpanNaming.java b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/WicketServerSpanNaming.java index 2e66a219b935..8135459b174e 100644 --- a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/WicketServerSpanNaming.java +++ b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/WicketServerSpanNaming.java @@ -22,7 +22,7 @@ public class WicketServerSpanNaming { return ServletContextPath.prepend(context, filterPath + "/" + pageName); }; - public static HttpServerRouteGetter getServerSpanName() { + public static HttpServerRouteGetter serverSpanName() { return serverSpanName; } From 4a8ac3c7aab2d5526eb3895b6a2f4d5136fdae29 Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Sat, 2 May 2026 20:31:51 +0000 Subject: [PATCH 06/11] Review fixes for wicket-8.0:wicket10-testing Automated code review of instrumentation/wicket-8.0/wicket10-testing. --- instrumentation/wicket-8.0/wicket10-testing/build.gradle.kts | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/wicket-8.0/wicket10-testing/build.gradle.kts b/instrumentation/wicket-8.0/wicket10-testing/build.gradle.kts index a45771640a09..b819365bfc79 100644 --- a/instrumentation/wicket-8.0/wicket10-testing/build.gradle.kts +++ b/instrumentation/wicket-8.0/wicket10-testing/build.gradle.kts @@ -6,7 +6,6 @@ dependencies { library("org.apache.wicket:wicket:10.0.0") testImplementation(project(":instrumentation:wicket-8.0:common-testing")) - testImplementation("org.jsoup:jsoup:1.13.1") testImplementation("org.eclipse.jetty:jetty-server:11.0.0") testImplementation("org.eclipse.jetty:jetty-servlet:11.0.0") From 5ca9b90b42ee9e48a6f20a13d0cf55843fa0985a Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Sat, 2 May 2026 20:35:00 +0000 Subject: [PATCH 07/11] Review fixes for xxl-job-1.9.2:javaagent Automated code review of instrumentation/xxl-job/xxl-job-1.9.2/javaagent. --- .../instrumentation/xxljob/v1_9_2/XxlJobSingletons.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/xxl-job/xxl-job-1.9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/xxljob/v1_9_2/XxlJobSingletons.java b/instrumentation/xxl-job/xxl-job-1.9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/xxljob/v1_9_2/XxlJobSingletons.java index cdb0b83735ad..e7e92a8767cd 100644 --- a/instrumentation/xxl-job/xxl-job-1.9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/xxljob/v1_9_2/XxlJobSingletons.java +++ b/instrumentation/xxl-job/xxl-job-1.9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/xxljob/v1_9_2/XxlJobSingletons.java @@ -31,7 +31,7 @@ public static XxlJobHelper helper() { return helper; } - @SuppressWarnings({"Unused", "ReturnValueIgnored"}) + @SuppressWarnings({"unused", "ReturnValueIgnored"}) private static void limitSupportedVersions() { // GLUE_POWERSHELL was added in 1.9.2. Using this constant here ensures that muzzle will disable // this instrumentation on earlier versions where this constant does not exist. From 5c85e40ce48028065de8ca8a9788b1c6627ade49 Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Sat, 2 May 2026 20:44:11 +0000 Subject: [PATCH 08/11] Review fixes for xxl-job-common:testing Automated code review of instrumentation/xxl-job/xxl-job-common/testing. --- .../instrumentation/xxljob/ReflectiveMethodsFactory.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/xxl-job/xxl-job-common/testing/src/main/java/io/opentelemetry/instrumentation/xxljob/ReflectiveMethodsFactory.java b/instrumentation/xxl-job/xxl-job-common/testing/src/main/java/io/opentelemetry/instrumentation/xxljob/ReflectiveMethodsFactory.java index 508247c4f16a..1f95d64ed982 100644 --- a/instrumentation/xxl-job/xxl-job-common/testing/src/main/java/io/opentelemetry/instrumentation/xxljob/ReflectiveMethodsFactory.java +++ b/instrumentation/xxl-job/xxl-job-common/testing/src/main/java/io/opentelemetry/instrumentation/xxljob/ReflectiveMethodsFactory.java @@ -21,7 +21,7 @@ static Object getTarget() { static Method getMethod() { try { return ReflectObject.class.getMethod("echo", String.class); - } catch (Throwable t) { + } catch (Throwable ignored) { return null; } } @@ -30,7 +30,7 @@ static Method getMethod() { static Method getInitMethod() { try { return ReflectObject.class.getMethod("initMethod"); - } catch (Throwable t) { + } catch (Throwable ignored) { return null; } } @@ -39,7 +39,7 @@ static Method getInitMethod() { static Method getDestroyMethod() { try { return ReflectObject.class.getMethod("destroyMethod"); - } catch (Throwable t) { + } catch (Throwable ignored) { return null; } } From 0b7407454a6a079c050ddb01b439356bd181c050 Mon Sep 17 00:00:00 2001 From: otelbot <197425009+otelbot@users.noreply.github.com> Date: Sat, 2 May 2026 20:47:06 +0000 Subject: [PATCH 09/11] Review fixes for zio-2.0:javaagent Automated code review of instrumentation/zio/zio-2.0/javaagent. --- .../instrumentation/zio/v2_0/TracingSupervisor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/instrumentation/zio/zio-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/zio/v2_0/TracingSupervisor.java b/instrumentation/zio/zio-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/zio/v2_0/TracingSupervisor.java index 3992149fab02..2bdc9c4e0d6b 100644 --- a/instrumentation/zio/zio-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/zio/v2_0/TracingSupervisor.java +++ b/instrumentation/zio/zio-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/zio/v2_0/TracingSupervisor.java @@ -15,7 +15,8 @@ import zio.ZIO; import zio.ZIO$; -@SuppressWarnings("unchecked") // fine +// ZIO's Supervisor API uses Scala generic signatures that javac cannot verify. +@SuppressWarnings("unchecked") public class TracingSupervisor extends Supervisor { public static final TracingSupervisor INSTANCE = new TracingSupervisor(); @@ -25,7 +26,8 @@ public class TracingSupervisor extends Supervisor { private TracingSupervisor() {} @Override - @SuppressWarnings("rawtypes") // fine + // The upstream Supervisor.value signature returns raw ZIO. + @SuppressWarnings("rawtypes") public ZIO value(Object trace) { return ZIO$.MODULE$.unit(); } From afbad74fd371f8e2f85da1e11e1d1b31f128c680 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sat, 2 May 2026 14:07:48 -0700 Subject: [PATCH 10/11] static import --- .github/agents/knowledge/README.md | 2 +- .github/agents/knowledge/general-rules.md | 2 +- .../knowledge/javaagent-singletons-patterns.md | 16 +++++++++++----- .../RequestHandlerExecutorInstrumentation.java | 3 ++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/agents/knowledge/README.md b/.github/agents/knowledge/README.md index 3cb97fe6b3ab..dc937f3f39a5 100644 --- a/.github/agents/knowledge/README.md +++ b/.github/agents/knowledge/README.md @@ -15,7 +15,7 @@ Load only files relevant to the current scope to reduce noise and avoid over-con | `gradle-conventions.md` | `build.gradle.kts` or `settings.gradle.kts` changes, custom test task registration or wiring | | `javaagent-advice-patterns.md` | ByteBuddy `@Advice` class or advice-method changes | | `javaagent-module-patterns.md` | `InstrumentationModule`, `TypeInstrumentation`, `VirtualField`, `CallDepth` | -| `javaagent-singletons-patterns.md` | `*Singletons` holder classes, singleton accessors, callers of singleton accessors/fields | +| `javaagent-singletons-patterns.md` | `*Singletons`, `*SpanNaming`, and similar holder classes; singleton accessors; callers of singleton accessors/fields | | `library-patterns.md` | Library instrumentation telemetry, builder, getter, or setter pattern changes | | `module-naming.md` | New or renamed modules or packages; settings includes | | `testing-general-patterns.md` | Test files in scope — assertion style, test method signatures and throws clauses, resource cleanup patterns, attribute assertion patterns, `satisfies()` lambda usage | diff --git a/.github/agents/knowledge/general-rules.md b/.github/agents/knowledge/general-rules.md index 2205d03dc099..2860e4170f46 100644 --- a/.github/agents/knowledge/general-rules.md +++ b/.github/agents/knowledge/general-rules.md @@ -21,7 +21,7 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th | Naming | Module/package naming | New or renamed modules/packages | `module-naming.md` | | Javaagent | Advice patterns | `@Advice` classes | `javaagent-advice-patterns.md` | | Javaagent | Module structure patterns | `InstrumentationModule`, `TypeInstrumentation` | `javaagent-module-patterns.md` | -| Javaagent | Singletons patterns | `*Singletons` holder classes, singleton accessors, callers of singleton accessors/fields | `javaagent-singletons-patterns.md` | +| Javaagent | Singletons patterns | `*Singletons`, `*SpanNaming`, and similar holder classes; singleton accessors; callers of singleton accessors/fields | `javaagent-singletons-patterns.md` | | Javaagent | Incorrect `classLoaderMatcher()` | `classLoaderMatcher()` override that is redundant (muzzle already handles it) or missing when needed (muzzle cannot distinguish version range) | `javaagent-module-patterns.md` | | Semconv | Library vs javaagent semconv constant usage | Semconv constants/assertions | — | | Semconv | Dual semconv testing | `SemconvStability`, `maybeStable`, semconv Gradle tasks | `testing-semconv-stability.md` | diff --git a/.github/agents/knowledge/javaagent-singletons-patterns.md b/.github/agents/knowledge/javaagent-singletons-patterns.md index bbfc0b120ac0..bc05261f7d4a 100644 --- a/.github/agents/knowledge/javaagent-singletons-patterns.md +++ b/.github/agents/knowledge/javaagent-singletons-patterns.md @@ -2,11 +2,13 @@ ## Quick Reference -- Use when: reviewing `*Singletons` holder classes and their callers +- Use when: reviewing `*Singletons`, `*SpanNaming`, and similar holder classes and their callers - Review focus: field/accessor naming, eager initialization, singleton accessor call sites Javaagent modules keep shared `Instrumenter` instances and related collaborators in a dedicated -`Singletons` holder class such as `MyLibrarySingletons`. +`Singletons` holder class such as `MyLibrarySingletons`. Some modules also use focused helper +holders such as `*ServerSpanNaming` for shared span-name or route-name collaborators; apply the +same accessor and call-site rules when these classes expose stored singleton fields. ## Rules @@ -18,8 +20,10 @@ Javaagent modules keep shared `Instrumenter` instances and related collaborators - For exported collaborators, keep the field `private`, use a lower camel case field name, and give the accessor method the exact same name as the field. Do not prefix these accessors with `get`. This rule applies only to zero-arg methods that directly return a stored singleton - field; methods that take arguments or compute a value are not singleton accessors and keep - their normal names (including `get*` when appropriate). + field. It applies regardless of whether the holder class is named `*Singletons`, + `*ServerSpanNaming`, or another focused holder name. Methods that take arguments or compute a + value are not singleton accessors and keep their normal names (including `get*` when + appropriate). - `instrumenter` -> `instrumenter()` - `helper` -> `helper()` - `setter` -> `setter()` @@ -32,7 +36,9 @@ Javaagent modules keep shared `Instrumenter` instances and related collaborators - `RESPONSE_STATUS` stays `RESPONSE_STATUS` - Callers should static import only exported singleton accessors and uppercase constant-like fields, and use those members unqualified: accessors for lower camel collaborators, fields for - uppercase constant-like members. + uppercase constant-like members. This includes route/span naming accessors such as + `serverSpanName()` when they simply return a stored `HttpServerRouteGetter` or similar + collaborator. - Keep verb-named helper methods as verbs when they perform work instead of returning a stored field. These methods are not singleton accessors and should not be static imported under this rule. diff --git a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/RequestHandlerExecutorInstrumentation.java b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/RequestHandlerExecutorInstrumentation.java index e138ff8c8706..3cbd7ff96daf 100644 --- a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/RequestHandlerExecutorInstrumentation.java +++ b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/v8_0/RequestHandlerExecutorInstrumentation.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.instrumentation.wicket.v8_0; import static io.opentelemetry.instrumentation.api.semconv.http.HttpServerRouteSource.CONTROLLER; +import static io.opentelemetry.javaagent.instrumentation.wicket.v8_0.WicketServerSpanNaming.serverSpanName; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -42,7 +43,7 @@ public static void onEnter(@Advice.Argument(0) IRequestHandler handler) { HttpServerRoute.update( Java8BytecodeBridge.currentContext(), CONTROLLER, - WicketServerSpanNaming.serverSpanName(), + serverSpanName(), (IPageClassRequestHandler) handler); } } From 988ce88c4743a318694f46a6dd35d621dddfd177 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sat, 2 May 2026 14:19:32 -0700 Subject: [PATCH 11/11] simplify --- .../agents/knowledge/testing-general-patterns.md | 14 ++++++++++++++ .../AbstractViburInstrumentationTest.java | 10 ++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/.github/agents/knowledge/testing-general-patterns.md b/.github/agents/knowledge/testing-general-patterns.md index 3ab19425b340..c70f5a1dfc43 100644 --- a/.github/agents/knowledge/testing-general-patterns.md +++ b/.github/agents/knowledge/testing-general-patterns.md @@ -97,6 +97,20 @@ `equalTo(AttributeKey, int)` overload, so `equalTo(longKey("iteration"), iteration)` is preferred over `equalTo(longKey("iteration"), (long) iteration)`. +## Metric Assertions + +- Prefer `InstrumentationExtension.waitAndAssertMetrics(...)` for metric assertions. It waits + until the supplied assertion passes, so fixed sleeps such as `Thread.sleep(100)` + usually unnecessary and make tests slower and more fragile. +- `DbConnectionPoolMetricsAssertions.assertConnectionPoolEmitsMetrics()` already uses + `waitAndAssertMetrics(...)` for each expected pool metric. Keep the pool state being asserted + valid until this method returns; for example, if the assertion expects a `used` connection point, + keep the borrowed connection open until after the assertion. +- After removing a metric-producing source or unregistering an observable callback, call + `testing().clearData()` and then use `waitAndAssertMetrics(..., AbstractIterableAssert::isEmpty)` + for absence checks. Do not add an exporter-interval sleep before or after `clearData()` solely to + wait for metrics; the test runners force-flush metrics when reading them. + ## Attribute Assertion `satisfies()` Lambda Parameters **Attribute-assertion `satisfies()` lambda parameters are `AbstractAssert` instances, not raw diff --git a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java index ee6c6b31d64e..ef91bd1e2eda 100644 --- a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java +++ b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java @@ -6,14 +6,12 @@ package io.opentelemetry.instrumentation.viburdbcp; import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv; -import static org.awaitility.Awaitility.await; import static org.mockito.Mockito.when; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.db.DbConnectionPoolMetricsAssertions; import java.sql.Connection; import java.sql.SQLException; -import java.time.Duration; import javax.sql.DataSource; import org.assertj.core.api.AbstractIterableAssert; import org.junit.jupiter.api.Test; @@ -25,7 +23,6 @@ @ExtendWith(MockitoExtension.class) public abstract class AbstractViburInstrumentationTest { private static final String INSTRUMENTATION_NAME = "io.opentelemetry.vibur-dbcp-11.0"; - private static final Duration EXPORTER_INTERVAL = Duration.ofMillis(100); @Mock private DataSource dataSourceMock; @Mock private Connection connectionMock; @@ -49,8 +46,6 @@ void shouldReportMetrics() throws SQLException { // when Connection viburConnection = viburDataSource.getConnection(); - await().pollDelay(EXPORTER_INTERVAL).until(() -> true); - viburConnection.close(); // then DbConnectionPoolMetricsAssertions.create(testing(), INSTRUMENTATION_NAME, "testPool") @@ -64,15 +59,14 @@ void shouldReportMetrics() throws SQLException { .assertConnectionPoolEmitsMetrics(); // when + viburConnection.close(); + // this one too shouldn't cause any problems when called more than once viburDataSource.close(); viburDataSource.close(); shutdown(viburDataSource); - // sleep exporter interval - await().pollDelay(EXPORTER_INTERVAL).until(() -> true); testing().clearData(); - await().pollDelay(EXPORTER_INTERVAL).until(() -> true); // then String countMetricName =