Skip to content

Code review sweep (run 24857196932)#18243

Merged
trask merged 9 commits into
mainfrom
otelbot/code-review-sweep-24857196932
Apr 24, 2026
Merged

Code review sweep (run 24857196932)#18243
trask merged 9 commits into
mainfrom
otelbot/code-review-sweep-24857196932

Conversation

@otelbot

@otelbot otelbot Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Automated code review sweep walked the following modules in order
and stopped after accumulating at least 10 modified files:

  • opentelemetry-instrumentation-api:javaagent
  • opentelemetry-instrumentation-api:testing
  • oracle-ucp-11.2:javaagent
  • oracle-ucp-11.2:library
  • oracle-ucp-11.2:testing
  • oshi:javaagent
  • oshi:library
  • oshi:testing
  • payara:javaagent
  • pekko-actor-1.0:javaagent

Module: opentelemetry-instrumentation-api:javaagent

Module path: instrumentation/opentelemetry-instrumentation-api/javaagent

Summary

Reviewed instrumentation/opentelemetry-instrumentation-api/javaagent; applied one safe test cleanup fix in ContextBridgeTest and found no other deterministic repository-guideline fixes in scope.

Applied Changes

Testing

File: ContextBridgeTest.java:36
Change: Added `AutoCleanupExtension` and deferred cleanup of the test-created `OpenTelemetrySdk` in `testSpanKeyBridge_UnbridgedSpan()`.
Reason: `testing-general-patterns.md` prefers `AutoCleanupExtension` with `deferCleanup(...)` for `AutoCloseable` resources that remain live for most of a test; this fixes the unclosed `OpenTelemetrySdk` created by the test.

Module: opentelemetry-instrumentation-api:testing

Module path: instrumentation/opentelemetry-instrumentation-api/testing

Summary

Applied 1 safe review fix in instrumentation/opentelemetry-instrumentation-api/testing: renamed the uppercase Instrumenter collaborator field in AgentSpanTestingInstrumenter to lower camel case to match repository style guidance. No other deterministic fixes were needed in scope.

Applied Changes

Style

File: AgentSpanTestingInstrumenter.java:31
Change: Renamed `HTTP_SERVER_INSTRUMENTER` to `httpServerInstrumenter` and updated its call sites.
Reason: The style guide says runtime-created collaborator objects such as instrumenters should use lower camel case, not `SCREAMING_SNAKE_CASE`, which is reserved for constants and stable semantic handles.

Module: oracle-ucp-11.2:javaagent

Module path: instrumentation/oracle-ucp-11.2/javaagent

Summary

Applied 1 safe review fix under instrumentation/oracle-ucp-11.2/javaagent: renamed the stored telemetry singleton in OracleUcpSingletons.java so the field name matches the exported telemetry() accessor per the repository *Singletons naming rule. Required validation completed with :instrumentation:oracle-ucp-11.2:javaagent:check, :instrumentation:oracle-ucp-11.2:javaagent:check -PtestLatestDeps=true, and ./gradlew spotlessApply.

Applied Changes

Javaagent

File: OracleUcpSingletons.java:13
Change: Renamed the private static `OracleUcpTelemetry` field from `oracleUcpTelemetry` to `telemetry` and updated `telemetry()` to return the renamed field.
Reason: Repository `*Singletons` guidance says exported lower-camel collaborators should use a field name that exactly matches the zero-arg accessor returning that stored singleton.

Module: oracle-ucp-11.2:library

Module path: instrumentation/oracle-ucp-11.2/library

Summary

No safe repository-guideline fixes were needed under instrumentation/oracle-ucp-11.2/library; the reviewed sources already match the applicable library, testing, Gradle, and metadata.yaml rules for this scope.

Applied Changes

No safe automated changes were applied.

Module: oracle-ucp-11.2:testing

Module path: instrumentation/oracle-ucp-11.2/testing

Summary

Reviewed instrumentation/oracle-ucp-11.2/testing and the module metadata.yaml; no safe repository-guideline fixes were needed.

Applied Changes

No safe automated changes were applied.

Module: oshi:javaagent

Module path: instrumentation/oshi/javaagent

Summary

Applied one safe review fix in oshi: added the missing declarative_name mapping in instrumentation/oshi/metadata.yaml so the module metadata matches the repository's mandatory metadata.yaml format for instrumentation configs.

Applied Changes

Config

File: metadata.yaml:8
Change: Added `declarative_name: java.oshi.experimental_metrics/development.enabled` for `otel.instrumentation.oshi.experimental-metrics.enabled`.
Reason: `metadata-yaml-format.md` requires every instrumentation config entry to declare the declarative mapping, and experimental flat names must map to a `/development` declarative path.

Module: oshi:library

Module path: instrumentation/oshi/library

Summary

Applied two safe review fixes in instrumentation/oshi/library by reducing unnecessary public visibility on JUnit @RegisterExtension fields; instrumentation/oshi/metadata.yaml was also reviewed and its config wiring matches the module's oshi instrumentation config lookup.

Applied Changes

Style

File: ProcessMetricsTest.java:22
Change: Changed the `@RegisterExtension` field `testing` from `public static final` to package-private `static final`.
Reason: Repository visibility rules prefer the minimum necessary visibility, and JUnit 5 test classes and members do not need `public` access here.

File: SystemMetricsTest.java:22
Change: Changed the `@RegisterExtension` field `testing` from `public static final` to package-private `static final`.
Reason: Repository visibility rules prefer the minimum necessary visibility, and JUnit 5 test classes and members do not need `public` access here.

Module: oshi:testing

Module path: instrumentation/oshi/testing

Summary

Applied 2 safe fixes under instrumentation/oshi/testing: the shared OSHI metric helper assertions now use the sum/gauge assertion subjects directly via satisfies(...) while preserving the existing .anySatisfy(...) point checks.

Applied Changes

test

File: AbstractProcessMetricsTest.java:37
Change: Reworked the `runtime.java.memory` and `runtime.java.cpu_time` assertions to read points from the `sum`/`gauge` subject passed into `hasLongSumSatisfying(...)` and `hasLongGaugeSatisfying(...)` instead of re-reading them from the outer `metric`.
Reason: This is a safe clarity fix aligned with the repository rule to reuse the current assertion subject instead of duplicating access paths, while preserving existing assertion behavior.

File: AbstractSystemMetricsTest.java:35
Change: Reworked the `system.memory.usage` and `system.memory.utilization` assertions to read points from the `sum`/`gauge` subject passed into `hasLongSumSatisfying(...)` and `hasDoubleGaugeSatisfying(...)` instead of re-reading them from the outer `metric`.
Reason: This is a safe clarity fix aligned with the repository rule to keep changes surgical and assertion logic anchored to the active subject, without changing expected metric matching semantics.

Module: payara:javaagent

Module path: instrumentation/payara/javaagent

Summary

Reviewed instrumentation/payara/javaagent and the module metadata.yaml; no safe repository-guideline fixes were needed.

Applied Changes

No safe automated changes were applied.

Module: pekko-actor-1.0:javaagent

Module path: instrumentation/pekko/pekko-actor-1.0/javaagent

Summary

Applied safe nullability fixes in the Pekko actor javaagent advice code so nullable ByteBuddy submit-state paths are declared accurately and consistently with repository guidance.

Applied Changes

Nullability

File: PekkoDispatcherInstrumentation.java:18
Change: Added `@Nullable` to the `PropagatedContext` enter result and to the `@Advice.Enter` / `@Advice.Thrown` exit parameters in `DispatchEnvelopeAdvice`.
Reason: The repository `Nullability correctness` rule requires annotating parameters and return values when `null` is actually produced; this advice returns `null` when context propagation is skipped, and ByteBuddy supplies `null` for `@Advice.Thrown` on successful calls.

File: PekkoDefaultSystemMessageQueueInstrumentation.java:20
Change: Added `@Nullable` to the `PropagatedContext` enter result and to the `@Advice.Enter` / `@Advice.Thrown` exit parameters in `DispatchSystemAdvice`.
Reason: The repository `Nullability correctness` rule requires annotating parameters and return values when `null` is actually produced; this advice returns `null` when context propagation is skipped, and ByteBuddy supplies `null` for `@Advice.Thrown` on successful calls.


Download code review diagnostics

otelbot Bot added 7 commits April 23, 2026 20:35
Automated code review of instrumentation/opentelemetry-instrumentation-api/javaagent.
Automated code review of instrumentation/opentelemetry-instrumentation-api/testing.
Automated code review of instrumentation/oracle-ucp-11.2/javaagent.
Automated code review of instrumentation/oshi/javaagent.
Automated code review of instrumentation/oshi/library.
Automated code review of instrumentation/oshi/testing.
Automated code review of instrumentation/pekko/pekko-actor-1.0/javaagent.
@otelbot
otelbot Bot requested a review from a team as a code owner April 23, 2026 21:19
@trask
trask merged commit cd45740 into main Apr 24, 2026
95 checks passed
@trask
trask deleted the otelbot/code-review-sweep-24857196932 branch April 24, 2026 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant