Code review sweep (run 25203544834)

[otelbot]

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

• netty-4.1:library
• netty-4.1:testing
• netty-common-4.0:javaagent
• netty-common-4.0:library
• netty-common:library
• okhttp-2.2:javaagent
• okhttp-3.0:javaagent
• okhttp-3.0:library

---

Module: netty-4.1:library

Module path: instrumentation/netty/netty-4.1/library

Summary

Applied 1 safe repository-guideline fix: added required declarative_name mappings to Netty 4.1 metadata.yaml.

Applied Changes

Config

File: metadata.yaml:13
Change: Added `declarative_name` entries for all configuration items, including common HTTP mappings and Netty-specific `connection_telemetry` and `ssl_telemetry` paths.
Reason: `metadata-yaml-format.md` requires each configuration entry to include `declarative_name`, with special mappings for common HTTP and sanitization properties and standard conversion for module-specific `otel.instrumentation.netty.*` keys.

Module: netty-4.1:testing

Module path: instrumentation/netty/netty-4.1/testing

Summary

Applied one safe repository-guideline fix in ClientHandler.java and committed it as Review fixes for netty-4.1 testing.

Applied Changes

Style

File: ClientHandler.java:34
Change: Changed `channelRead0(...)` from `public` to `protected`.
Reason: The style guide requires minimal necessary visibility; `SimpleChannelInboundHandler.channelRead0(...)` is a protected override point, so widening it to `public` exposes unnecessary API surface.

Module: netty-common-4.0:javaagent

Module path: instrumentation/netty/netty-common-4.0/javaagent

Summary

No safe repository-guideline fixes were found in instrumentation/netty/netty-common-4.0/javaagent; no source files were changed.

Applied Changes

No safe automated changes were applied.

Module: netty-common-4.0:library

Module path: instrumentation/netty/netty-common-4.0/library

Summary

Applied safe style-guideline fixes in 3 files under instrumentation/netty/netty-common-4.0/library and committed them in 661c27d8.

Applied Changes

Style

File: HttpSchemeUtil.java:18
Change: Renamed static final descriptor field `sslHandlerClass` to `SSL_HANDLER_CLASS`.
Reason: The style guide says constant-like static fields and immutable descriptors should use `SCREAMING_SNAKE_CASE`.

File: NettyClientInstrumenterBuilderFactory.java:28
Change: Moved the private utility constructor after the static factory method `create(...)`.
Reason: The style guide says static utility classes should place the private constructor after all methods.

File: NettySslInstrumentationHandler.java:32
Change: Renamed static final `VirtualField` handle `instrumentationHandlerField` to `INSTRUMENTATION_HANDLER_FIELD` and unused catch parameters to `ignored`.
Reason: The style guide treats `VirtualField` handles as constant-like fields, and general review rules prefer `ignored` for intentionally unused catch parameters.

Module: netty-common:library

Module path: instrumentation/netty/netty-common/library

Summary

Applied and committed one safe style-guide fix under instrumentation/netty/netty-common/library.

Applied Changes

Style

File: Timer.java:22
Change: Moved `Timer` instance fields above the `start()` static factory method.
Reason: Repository style guide class organization places instance fields before constructors and methods, with static factory entry points below fields.

Module: okhttp-2.2:javaagent

Module path: instrumentation/okhttp/okhttp-2.2/javaagent

Summary

Applied and committed safe review fixes for okhttp-2.2 javaagent: nullable advice throwable metadata and missing declarative_name coverage.

Applied Changes

Javaagent

File: DispatcherInstrumentation.java:53
Change: Annotated the `@Advice.Thrown` `Throwable` parameter with `@Nullable`.
Reason: `@Advice.Thrown` is `null` when the instrumented method exits normally, and repository nullability guidance requires parameters that can be `null` to be annotated accurately.

Config

File: metadata.yaml:25
Change: Added `declarative_name: java.common.peer_service_mapping` for `otel.instrumentation.common.peer-service-mapping`.
Reason: `metadata.yaml` format guidance requires configuration entries to include the matching declarative config key; standard conversion maps this common instrumentation property to `java.common.peer_service_mapping`.

Module: okhttp-3.0:javaagent

Module path: instrumentation/okhttp/okhttp-3.0/javaagent

Summary

Applied one safe metadata-guideline fix for the okhttp-3.0 instrumentation review and committed it.

Applied Changes

Config

File: metadata.yaml:26
Change: Added `declarative_name: java.common.peer_service_mapping` for `otel.instrumentation.common.peer-service-mapping`.
Reason: `metadata-yaml-format.md` requires each instrumentation configuration entry to include the declarative YAML key; the sibling OkHttp metadata and standard conversion map this property to `java.common.peer_service_mapping`.

Module: okhttp-3.0:library

Module path: instrumentation/okhttp/okhttp-3.0/library

Summary

Applied 2 safe repository-guideline fixes under instrumentation/okhttp/okhttp-3.0/library and committed them in e59ac800.

Applied Changes

Style

File: TracingCallFactory.java:36
Change: Renamed unused catch variables from `e` to `ignored` in `NoSuchMethodException` and reflective invocation catch blocks.
Reason: `knowledge/general-rules.md` catch-variable guidance says intentionally unused catch parameters should be named `ignored`.

General

File: TracingInterceptor.java:55
Change: Replaced a stale context-injection comment that referenced `Instrumenter<Request, Response>` with an accurate explanation that `OkHttp` `Request` is immutable.
Reason: `knowledge/general-rules.md` requires fixing incorrect comments; the old comment no longer matched the actual `Instrumenter<Chain, Response>` implementation.

---

Download code review diagnostics