Skip to content

[Feat] [SDK-399] Okhttp interceptor#367

Merged
buongarzoni merged 21 commits into
masterfrom
feat/SDK-399/network-telemetry-interceptor
May 5, 2026
Merged

[Feat] [SDK-399] Okhttp interceptor#367
buongarzoni merged 21 commits into
masterfrom
feat/SDK-399/network-telemetry-interceptor

fix(okhttp): strip query params from recorded URLs by default to prev…

02c7ea0
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 27, 2026 in 8m 26s

Code review found 3 potential issues

Found 6 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 3
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit rollbar-okhttp/README.md:14-30 README install snippet omits the rollbar-java notifier required by the recorder example
🟡 Nit rollbar-okhttp/build.gradle.kts:8 Mockito version drift: rollbar-okhttp redeclares 5.23.0 over the root subprojects 5.18.0
🟡 Nit rollbar-okhttp/src/main/java/com/rollbar/okhttp/RollbarOkHttpInterceptor.java:8-17 Use SLF4J instead of java.util.logging for consistency with the rest of the SDK

Annotations

Check warning on line 30 in rollbar-okhttp/README.md

See this annotation in the file changed.

@claude claude / Claude Code Review

README install snippet omits the rollbar-java notifier required by the recorder example

The Installation snippet in rollbar-okhttp/README.md only lists `com.rollbar:rollbar-okhttp` and `com.squareup.okhttp3:okhttp`, but the recorder example below calls `rollbar.recordNetworkEventFor(...)` and `rollbar.log(...)` — methods defined on `com.rollbar.notifier.RollbarBase` in the rollbar-java module. `rollbar-okhttp/build.gradle.kts` only declares `api(project(":rollbar-api"))`, and rollbar-api contains no `Rollbar` notifier class, so a user who copy-pastes the install snippet alongside t

Check warning on line 8 in rollbar-okhttp/build.gradle.kts

See this annotation in the file changed.

@claude claude / Claude Code Review

Mockito version drift: rollbar-okhttp redeclares 5.23.0 over the root subprojects 5.18.0

rollbar-okhttp/build.gradle.kts:8 declares `testImplementation("org.mockito:mockito-core:5.23.0")`, but the root subprojects block already adds `org.mockito:mockito-core:5.18.0` to every non-example non-Android subproject (build.gradle.kts:55). Both versions land on the test classpath; Gradle resolves to the highest (5.23.0), so tests pass — but rollbar-okhttp silently diverges from every other module's pinned version, and a future root mockito bump won't apply uniformly here. Drop the local lin

Check warning on line 17 in rollbar-okhttp/src/main/java/com/rollbar/okhttp/RollbarOkHttpInterceptor.java

See this annotation in the file changed.

@claude claude / Claude Code Review

Use SLF4J instead of java.util.logging for consistency with the rest of the SDK

**Logging consistency:** `RollbarOkHttpInterceptor` uses `java.util.logging.Logger`, but every other production module in the SDK (rollbar-java, rollbar-web, rollbar-jakarta-web, rollbar-struts2, rollbar-reactive-streams, rollbar-android, etc.) uses SLF4J via `org.slf4j.LoggerFactory`, and rollbar-java declares `slf4j-api` as an `api` dependency. Switching to `org.slf4j.Logger` / `LoggerFactory.getLogger(RollbarOkHttpInterceptor.class)` would keep this module consistent with the rest of the SDK