From 57625387ef11b8a6e0a18a4d0df56d3e5196ac4c Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 25 May 2026 17:30:59 -0300 Subject: [PATCH 01/28] feat(java-agent): add rollbar-java-agent module skeleton --- rollbar-java-agent/build.gradle.kts | 55 +++++++++++++++++ .../rollbar/agent/AgentTelemetryStore.java | 27 +++++++++ .../com/rollbar/agent/NetworkEventBridge.java | 60 +++++++++++++++++++ .../java/com/rollbar/agent/RollbarAgent.java | 54 +++++++++++++++++ .../java/com/rollbar/agent/UrlSanitizer.java | 33 ++++++++++ settings.gradle.kts | 6 ++ 6 files changed, 235 insertions(+) create mode 100644 rollbar-java-agent/build.gradle.kts create mode 100644 rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java create mode 100644 rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java create mode 100644 rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java create mode 100644 rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java diff --git a/rollbar-java-agent/build.gradle.kts b/rollbar-java-agent/build.gradle.kts new file mode 100644 index 00000000..34facdef --- /dev/null +++ b/rollbar-java-agent/build.gradle.kts @@ -0,0 +1,55 @@ +plugins { + `java-library` + id("com.github.johnrengelman.shadow") version "8.1.1" +} + +dependencies { + implementation("net.bytebuddy:byte-buddy:1.14.18") + implementation("net.bytebuddy:byte-buddy-agent:1.14.18") + api(project(":rollbar-api")) + implementation(project(":rollbar-java")) + compileOnly("org.apache.httpcomponents:httpclient:4.5.14") + compileOnly("org.apache.httpcomponents.client5:httpclient5:5.3.1") + + testImplementation(platform("org.junit:junit-bom:5.14.3")) + testImplementation("org.junit.jupiter:junit-jupiter") + testRuntimeOnly("org.junit.platform:junit-platform-launcher") + testImplementation("org.mockito:mockito-core:5.11.0") + testImplementation("com.github.tomakehurst:wiremock-jre8:2.35.2") + testImplementation("org.apache.httpcomponents:httpclient:4.5.14") + testImplementation("org.apache.httpcomponents.client5:httpclient5:5.3.1") +} + +tasks.jar { + manifest { + attributes( + "Premain-Class" to "com.rollbar.agent.RollbarAgent", + "Agent-Class" to "com.rollbar.agent.RollbarAgent", + "Can-Redefine-Classes" to "true", + "Can-Retransform-Classes" to "true" + ) + } +} + +tasks.shadowJar { + archiveClassifier.set("") + relocate("net.bytebuddy", "com.rollbar.agent.shaded.bytebuddy") + mergeServiceFiles() +} + +// Override root's Java 8 compatibility — this agent targets Java 11+ to support +// java.net.http.HttpClient instrumentation. +tasks.withType().configureEach { + options.release.set(11) +} + +tasks.test { + useJUnitPlatform() + val agentJar = tasks.shadowJar.get().archiveFile.get().asFile + // Load as Java agent (instruments HTTP classes on startup) + jvmArgs("-javaagent:$agentJar") + // Also put on test classpath — the TCCL reflection bridge finds agent classes via the + // system classloader; mirrors production use where rollbar-java-agent is a Gradle/Maven dep + classpath += files(agentJar) + dependsOn(tasks.shadowJar) +} diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java new file mode 100644 index 00000000..35bf036f --- /dev/null +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java @@ -0,0 +1,27 @@ +package com.rollbar.agent; + +import com.rollbar.api.payload.data.TelemetryEvent; +import com.rollbar.notifier.telemetry.RollbarTelemetryEventTracker; +import com.rollbar.notifier.telemetry.TelemetryEventTracker; + +import java.util.List; + +public final class AgentTelemetryStore { + + private static volatile TelemetryEventTracker INSTANCE = + new RollbarTelemetryEventTracker(System::currentTimeMillis, 100); + + private AgentTelemetryStore() {} + + public static TelemetryEventTracker getInstance() { + return INSTANCE; + } + + public static List getAll() { + return INSTANCE.getAll(); + } + + public static void resetForTesting() { + INSTANCE = new RollbarTelemetryEventTracker(System::currentTimeMillis, 100); + } +} diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java new file mode 100644 index 00000000..947c5b34 --- /dev/null +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java @@ -0,0 +1,60 @@ +package com.rollbar.agent; + +import com.rollbar.api.payload.data.Level; +import com.rollbar.api.payload.data.Source; + +import java.util.Collections; +import java.util.Set; +import java.util.WeakHashMap; + +/** + * Called by JDK-class advice via reflection to bridge the classloader gap. + * + *

ByteBuddy advice inlined into bootstrap/platform classloader classes (e.g. + * {@code HttpURLConnection}, {@code HttpClient}) cannot directly reference application-classloader + * classes. Advice code uses {@code Thread.currentThread().getContextClassLoader().loadClass(...)} + * to reach this class and delegates all Rollbar-specific logic here. + */ +public final class NetworkEventBridge { + + // Tracks connections/responses already recorded to deduplicate re-entrant calls. + // WeakHashMap so entries are garbage-collected when the connection is released. + private static final Set RECORDED = Collections.newSetFromMap( + Collections.synchronizedMap(new WeakHashMap<>()) + ); + + private NetworkEventBridge() {} + + public static void resetRecordedForTesting() { + RECORDED.clear(); + } + + /** + * Marks the given key as recorded. Returns {@code true} if this is the first time, + * {@code false} if already recorded (duplicate/re-entrant call). + */ + public static boolean markAsRecorded(Object key) { + return RECORDED.add(key); + } + + public static void recordNetworkEvent(Object key, String method, String url, String statusCode) { + if (!markAsRecorded(key)) { + return; // deduplicate re-entrant calls for the same connection + } + AgentTelemetryStore.getInstance().recordNetworkEventFor( + Level.CRITICAL, + Source.SERVER, + method, + UrlSanitizer.sanitize(url), + statusCode + ); + } + + public static void recordError(String message) { + AgentTelemetryStore.getInstance().recordManualEventFor( + Level.CRITICAL, + Source.SERVER, + "Network error: " + (message != null ? message : "unknown") + ); + } +} diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java new file mode 100644 index 00000000..8eb04d68 --- /dev/null +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java @@ -0,0 +1,54 @@ +package com.rollbar.agent; + +import com.rollbar.agent.instrumentation.ApacheHttpClient4Instrumentation; +import com.rollbar.agent.instrumentation.ApacheHttpClient5Instrumentation; +import com.rollbar.agent.instrumentation.HttpURLConnectionInstrumentation; +import com.rollbar.agent.instrumentation.JavaHttpClientInstrumentation; +import com.rollbar.notifier.telemetry.TelemetryEventTracker; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.matcher.ElementMatchers; + +import java.lang.instrument.Instrumentation; + +/** + * Java agent entry point. Attach with {@code -javaagent:/path/to/rollbar-java-agent.jar}. + * + *

Wire into your Rollbar configuration with: + * 

+ *   Rollbar.init(withAccessToken("...")
+ *       .telemetryEventTracker(RollbarAgent.getTelemetryTracker())
+ *       .build());
+ *
+ */ +public class RollbarAgent { + + private RollbarAgent() {} + + public static void premain(String args, Instrumentation inst) { + installInstrumentation(inst); + } + + public static void agentmain(String args, Instrumentation inst) { + installInstrumentation(inst); + } + + private static void installInstrumentation(Instrumentation inst) { + // Override ByteBuddy's default which ignores all java.* and javax.* classes, + // so we can instrument JDK HTTP clients (HttpURLConnection, HttpClient). + // We still ignore ByteBuddy's own classes to avoid instrumentation loops. + AgentBuilder builder = new AgentBuilder.Default() + .ignore(ElementMatchers.nameStartsWith("net.bytebuddy.") + .or(ElementMatchers.nameStartsWith("com.rollbar.agent.shaded."))) + .with(AgentBuilder.InitializationStrategy.NoOp.INSTANCE) + .with(AgentBuilder.TypeStrategy.Default.REDEFINE); + + HttpURLConnectionInstrumentation.install(builder, inst); + JavaHttpClientInstrumentation.installIfAvailable(builder, inst); + ApacheHttpClient4Instrumentation.installIfAvailable(builder, inst); + ApacheHttpClient5Instrumentation.installIfAvailable(builder, inst); + } + + public static TelemetryEventTracker getTelemetryTracker() { + return AgentTelemetryStore.getInstance(); + } +} diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java new file mode 100644 index 00000000..d11ae48d --- /dev/null +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java @@ -0,0 +1,33 @@ +package com.rollbar.agent; + +import java.net.URI; +import java.net.URISyntaxException; + +public final class UrlSanitizer { + + private UrlSanitizer() {} + + /** + * Strips userinfo, query parameters, and fragment from the URL, leaving only + * scheme, host, port, and path. + */ + public static String sanitize(String rawUrl) { + if (rawUrl == null) { + return null; + } + try { + URI uri = new URI(rawUrl); + return new URI( + uri.getScheme(), + null, + uri.getHost(), + uri.getPort(), + uri.getPath(), + null, + null + ).toString(); + } catch (URISyntaxException e) { + return rawUrl; + } + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 73e10c5b..93e158bf 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -67,3 +67,9 @@ if (isJava8 || isJava11) { println("Java ${JavaVersion.current()} detected: including Android modules") include(":rollbar-android", ":examples:rollbar-android") } + +if (isJava8) { + println("Java 8 detected: excluding :rollbar-java-agent (requires Java 11+)") +} else { + include(":rollbar-java-agent") +} From 70573e5090fdfc389765c79ee6bc9d72e77e0414 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 25 May 2026 17:31:14 -0300 Subject: [PATCH 02/28] feat(java-agent): instrument HttpURLConnection and java.net.http.HttpClient --- .../HttpURLConnectionInstrumentation.java | 69 ++++++++++++++++ .../JavaHttpClientInstrumentation.java | 80 +++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java create mode 100644 rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java new file mode 100644 index 00000000..b2a8c513 --- /dev/null +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java @@ -0,0 +1,69 @@ +package com.rollbar.agent.instrumentation; + +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.matcher.ElementMatchers; + +import java.lang.instrument.Instrumentation; + +public final class HttpURLConnectionInstrumentation { + + private HttpURLConnectionInstrumentation() {} + + public static void install(AgentBuilder builder, Instrumentation inst) { + builder + .type(ElementMatchers.named("java.net.HttpURLConnection")) + .transform((b, typeDescription, classLoader, module, protectionDomain) -> + b.visit(Advice.to(GetResponseCodeAdvice.class) + .on(ElementMatchers.named("getResponseCode"))) + ) + .installOn(inst); + } + + /** + * Advice inlined into {@code java.net.HttpURLConnection.getResponseCode()}. + * + *

Only JDK types are referenced directly. The Rollbar bridge is reached via TCCL + * to cross the classloader boundary. The connection instance is used as the deduplication + * key — getResponseCode() is called re-entrantly up to 3 times per request internally. + */ + public static class GetResponseCodeAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void onExit( + @Advice.This Object connection, + @Advice.Return int statusCode, + @Advice.Thrown Throwable thrown + ) { + try { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + if (cl == null) { + cl = ClassLoader.getSystemClassLoader(); + } + Class bridge = cl.loadClass("com.rollbar.agent.NetworkEventBridge"); + + if (thrown != null) { + Boolean recorded = (Boolean) bridge + .getMethod("markAsRecorded", Object.class).invoke(null, thrown); + if (recorded) { + String msg = thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName(); + bridge.getMethod("recordError", String.class).invoke(null, msg); + } + return; + } + + if (statusCode >= 400) { + Object url = connection.getClass().getMethod("getURL").invoke(connection); + String urlStr = url != null ? url.toString() : ""; + String method = (String) connection.getClass().getMethod("getRequestMethod").invoke(connection); + // connection instance as dedup key — same object across all re-entrant getResponseCode() calls + bridge.getMethod("recordNetworkEvent", + Object.class, String.class, String.class, String.class) + .invoke(null, connection, method, urlStr, String.valueOf(statusCode)); + } + } catch (Throwable ignored) { + // Advice must never throw — swallow all errors including Error subclasses + } + } + } +} diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java new file mode 100644 index 00000000..5247f832 --- /dev/null +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java @@ -0,0 +1,80 @@ +package com.rollbar.agent.instrumentation; + +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.matcher.ElementMatchers; + +import java.lang.instrument.Instrumentation; +import java.net.http.HttpClient; + +public final class JavaHttpClientInstrumentation { + + private JavaHttpClientInstrumentation() {} + + public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { + try { + Class.forName("java.net.http.HttpClient"); + } catch (ClassNotFoundException e) { + return; + } + + builder + .type(ElementMatchers.isSubTypeOf(HttpClient.class)) + .transform((b, typeDescription, classLoader, module, protectionDomain) -> + b.visit(Advice.to(SendAdvice.class) + .on(ElementMatchers.named("send"))) + ) + .installOn(inst); + } + + /** + * Advice inlined into JDK's HttpClient concrete implementation's send(). + * + *

Only JDK types are referenced directly. The Rollbar bridge is reached via TCCL + * to cross the classloader boundary. The response object is used as a deduplication key + * since both HttpClientFacade and HttpClientImpl instrument send() and the same response + * object flows through both. + */ + public static class SendAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void onExit( + @Advice.Argument(0) Object request, + @Advice.Return Object response, + @Advice.Thrown Throwable thrown + ) { + try { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + if (cl == null) { + cl = ClassLoader.getSystemClassLoader(); + } + Class bridge = cl.loadClass("com.rollbar.agent.NetworkEventBridge"); + + if (thrown != null) { + Boolean recorded = (Boolean) bridge + .getMethod("markAsRecorded", Object.class).invoke(null, thrown); + if (recorded) { + String msg = thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName(); + bridge.getMethod("recordError", String.class).invoke(null, msg); + } + return; + } + + if (response != null) { + int statusCode = (Integer) response.getClass().getMethod("statusCode").invoke(response); + if (statusCode >= 400) { + Object uri = request.getClass().getMethod("uri").invoke(request); + String method = (String) request.getClass().getMethod("method").invoke(request); + // response object is the dedup key — unique per send() call, shared between + // HttpClientFacade and HttpClientImpl so only one event is recorded + bridge.getMethod("recordNetworkEvent", + Object.class, String.class, String.class, String.class) + .invoke(null, response, method, uri.toString(), String.valueOf(statusCode)); + } + } + } catch (Throwable ignored) { + // Advice must never throw — swallow all errors including Error subclasses + } + } + } +} From 3ee326df2f8f3f9b4826f1097d83ba4d6a537c5c Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 25 May 2026 17:31:27 -0300 Subject: [PATCH 03/28] feat(java-agent): instrument Apache HttpClient 4.x and 5.x --- .../ApacheHttpClient4Instrumentation.java | 78 +++++++++++++++++++ .../ApacheHttpClient5Instrumentation.java | 78 +++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java create mode 100644 rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java new file mode 100644 index 00000000..be57fabf --- /dev/null +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java @@ -0,0 +1,78 @@ +package com.rollbar.agent.instrumentation; + +import com.rollbar.agent.AgentTelemetryStore; +import com.rollbar.agent.UrlSanitizer; +import com.rollbar.api.payload.data.Level; +import com.rollbar.api.payload.data.Source; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.matcher.ElementMatchers; +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpUriRequest; + +import java.lang.instrument.Instrumentation; + +public final class ApacheHttpClient4Instrumentation { + + private ApacheHttpClient4Instrumentation() {} + + public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { + try { + Class.forName("org.apache.http.impl.client.CloseableHttpClient"); + } catch (ClassNotFoundException e) { + return; + } + + builder + .type(ElementMatchers.hasSuperType( + ElementMatchers.named("org.apache.http.impl.client.CloseableHttpClient"))) + .transform((b, typeDescription, classLoader, module, protectionDomain) -> + b.visit(Advice.to(ExecuteAdvice.class) + .on(ElementMatchers.named("execute") + .and(ElementMatchers.takesArgument(0, + ElementMatchers.named("org.apache.http.client.methods.HttpUriRequest"))))) + ) + .installOn(inst); + } + + /** + * Apache HC 4.x runs in the application classloader, so we can reference Rollbar classes + * directly without the TCCL reflection bridge. + */ + public static class ExecuteAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void onExit( + @Advice.Argument(0) HttpUriRequest request, + @Advice.Return HttpResponse response, + @Advice.Thrown Throwable thrown + ) { + try { + if (thrown != null) { + AgentTelemetryStore.getInstance().recordManualEventFor( + Level.CRITICAL, + Source.SERVER, + "Network error: " + (thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName()) + ); + return; + } + + if (response != null) { + int statusCode = response.getStatusLine().getStatusCode(); + if (statusCode >= 400) { + String url = request.getURI() != null ? request.getURI().toString() : ""; + AgentTelemetryStore.getInstance().recordNetworkEventFor( + Level.CRITICAL, + Source.SERVER, + request.getMethod(), + UrlSanitizer.sanitize(url), + String.valueOf(statusCode) + ); + } + } + } catch (Throwable ignored) { + // Advice must never throw + } + } + } +} diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java new file mode 100644 index 00000000..95d6e8c4 --- /dev/null +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java @@ -0,0 +1,78 @@ +package com.rollbar.agent.instrumentation; + +import com.rollbar.agent.AgentTelemetryStore; +import com.rollbar.agent.UrlSanitizer; +import com.rollbar.api.payload.data.Level; +import com.rollbar.api.payload.data.Source; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.matcher.ElementMatchers; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.ClassicHttpResponse; + +import java.lang.instrument.Instrumentation; + +public final class ApacheHttpClient5Instrumentation { + + private ApacheHttpClient5Instrumentation() {} + + public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { + try { + Class.forName("org.apache.hc.client5.http.impl.classic.CloseableHttpClient"); + } catch (ClassNotFoundException e) { + return; + } + + builder + .type(ElementMatchers.hasSuperType( + ElementMatchers.named("org.apache.hc.client5.http.impl.classic.CloseableHttpClient"))) + .transform((b, typeDescription, classLoader, module, protectionDomain) -> + b.visit(Advice.to(ExecuteAdvice.class) + .on(ElementMatchers.named("execute") + .and(ElementMatchers.takesArgument(0, + ElementMatchers.named("org.apache.hc.core5.http.ClassicHttpRequest"))))) + ) + .installOn(inst); + } + + /** + * Apache HC 5.x runs in the application classloader, so we can reference Rollbar classes + * directly without the TCCL reflection bridge. + */ + public static class ExecuteAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void onExit( + @Advice.Argument(0) ClassicHttpRequest request, + @Advice.Return ClassicHttpResponse response, + @Advice.Thrown Throwable thrown + ) { + try { + if (thrown != null) { + AgentTelemetryStore.getInstance().recordManualEventFor( + Level.CRITICAL, + Source.SERVER, + "Network error: " + (thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName()) + ); + return; + } + + if (response != null) { + int statusCode = response.getCode(); + if (statusCode >= 400) { + String url = request.getRequestUri() != null ? request.getRequestUri() : ""; + AgentTelemetryStore.getInstance().recordNetworkEventFor( + Level.CRITICAL, + Source.SERVER, + request.getMethod(), + UrlSanitizer.sanitize(url), + String.valueOf(statusCode) + ); + } + } + } catch (Throwable ignored) { + // Advice must never throw + } + } + } +} From e5226c131e4097da590bd11875ee897f4a64f641 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 25 May 2026 17:31:40 -0300 Subject: [PATCH 04/28] test(java-agent): add integration tests for instrumented HTTP clients --- .../com/rollbar/agent/RollbarAgentTest.java | 39 ++++++ .../HttpURLConnectionInstrumentationTest.java | 113 ++++++++++++++++++ .../JavaHttpClientInstrumentationTest.java | 106 ++++++++++++++++ 3 files changed, 258 insertions(+) create mode 100644 rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java create mode 100644 rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java create mode 100644 rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java new file mode 100644 index 00000000..0efbc44c --- /dev/null +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java @@ -0,0 +1,39 @@ +package com.rollbar.agent; + +import com.rollbar.notifier.telemetry.TelemetryEventTracker; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class RollbarAgentTest { + + @BeforeEach + public void setUp() { + AgentTelemetryStore.resetForTesting(); + } + + @Test + public void getTelemetryTracker_returnsSingletonInstance() { + TelemetryEventTracker first = RollbarAgent.getTelemetryTracker(); + TelemetryEventTracker second = RollbarAgent.getTelemetryTracker(); + assertSame(first, second); + } + + @Test + public void urlSanitizer_stripsQueryAndFragment() { + String sanitized = UrlSanitizer.sanitize("https://api.example.com/path?token=secret#section"); + assertEquals("https://api.example.com/path", sanitized); + } + + @Test + public void urlSanitizer_handlesNullGracefully() { + assertNull(UrlSanitizer.sanitize(null)); + } + + @Test + public void urlSanitizer_handlesInvalidUrl() { + String raw = "not a url"; + assertEquals(raw, UrlSanitizer.sanitize(raw)); + } +} diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java new file mode 100644 index 00000000..6044e832 --- /dev/null +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java @@ -0,0 +1,113 @@ +package com.rollbar.agent.instrumentation; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.rollbar.agent.AgentTelemetryStore; +import com.rollbar.agent.NetworkEventBridge; +import com.rollbar.api.payload.data.TelemetryEvent; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.URL; +import java.util.List; +import java.util.Map; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static org.junit.jupiter.api.Assertions.*; + +public class HttpURLConnectionInstrumentationTest { + + private WireMockServer server; + + @BeforeEach + public void setUp() { + server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); + server.start(); + AgentTelemetryStore.resetForTesting(); + NetworkEventBridge.resetRecordedForTesting(); + } + + @AfterEach + public void tearDown() { + server.stop(); + } + + @Test + public void successResponse_doesNotRecordEvent() throws IOException { + server.stubFor(get(urlEqualTo("/ok")).willReturn(aResponse().withStatus(200))); + + makeRequest("GET", "/ok"); + + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void clientErrorResponse_recordsNetworkEvent() throws IOException { + server.stubFor(get(urlEqualTo("/not-found")).willReturn(aResponse().withStatus(404))); + + makeRequest("GET", "/not-found"); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map json = events.get(0).asJson(); + assertEquals("network", json.get("type")); + Map body = (Map) json.get("body"); + assertEquals("404", body.get("status_code")); + assertEquals("GET", body.get("method")); + assertTrue(body.get("url").toString().contains("/not-found")); + } + + @Test + public void serverErrorResponse_recordsNetworkEvent() throws IOException { + server.stubFor(get(urlEqualTo("/error")).willReturn(aResponse().withStatus(500))); + + makeRequest("GET", "/error"); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("500", body.get("status_code")); + } + + @Test + public void redirectResponse_doesNotRecordEvent() throws IOException { + server.stubFor(get(urlEqualTo("/redirect")).willReturn( + aResponse().withStatus(301).withHeader("Location", "/other"))); + + HttpURLConnection conn = (HttpURLConnection) new URL(server.baseUrl() + "/redirect").openConnection(); + conn.setInstanceFollowRedirects(false); + conn.getResponseCode(); + conn.disconnect(); + + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void urlSanitization_stripsQueryAndCredentials() throws IOException { + server.stubFor(get(anyUrl()).willReturn(aResponse().withStatus(500))); + + HttpURLConnection conn = (HttpURLConnection) new URL( + server.baseUrl() + "/path?secret=abc&token=xyz" + ).openConnection(); + conn.getResponseCode(); + conn.disconnect(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + String url = body.get("url").toString(); + assertTrue(url.contains("/path")); + assertFalse(url.contains("secret")); + assertFalse(url.contains("token")); + } + + private void makeRequest(String method, String path) throws IOException { + HttpURLConnection conn = (HttpURLConnection) new URL(server.baseUrl() + path).openConnection(); + conn.setRequestMethod(method); + conn.getResponseCode(); + conn.disconnect(); + } +} diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java new file mode 100644 index 00000000..fa31cafd --- /dev/null +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java @@ -0,0 +1,106 @@ +package com.rollbar.agent.instrumentation; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.rollbar.agent.AgentTelemetryStore; +import com.rollbar.agent.NetworkEventBridge; +import com.rollbar.api.payload.data.TelemetryEvent; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.List; +import java.util.Map; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static org.junit.jupiter.api.Assertions.*; + +public class JavaHttpClientInstrumentationTest { + + private WireMockServer server; + private HttpClient client; + + @BeforeEach + public void setUp() { + server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); + server.start(); + client = HttpClient.newHttpClient(); + AgentTelemetryStore.resetForTesting(); + NetworkEventBridge.resetRecordedForTesting(); + } + + @AfterEach + public void tearDown() { + server.stop(); + } + + @Test + public void successResponse_doesNotRecordEvent() throws Exception { + server.stubFor(get(urlEqualTo("/ok")).willReturn(aResponse().withStatus(200))); + + client.send( + HttpRequest.newBuilder(URI.create(server.baseUrl() + "/ok")).GET().build(), + HttpResponse.BodyHandlers.discarding() + ); + + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void clientErrorResponse_recordsNetworkEvent() throws Exception { + server.stubFor(get(urlEqualTo("/not-found")).willReturn(aResponse().withStatus(404))); + + client.send( + HttpRequest.newBuilder(URI.create(server.baseUrl() + "/not-found")).GET().build(), + HttpResponse.BodyHandlers.discarding() + ); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map json = events.get(0).asJson(); + assertEquals("network", json.get("type")); + Map body = (Map) json.get("body"); + assertEquals("404", body.get("status_code")); + assertEquals("GET", body.get("method")); + assertTrue(body.get("url").toString().contains("/not-found")); + } + + @Test + public void serverErrorResponse_recordsNetworkEvent() throws Exception { + server.stubFor(post(urlEqualTo("/error")).willReturn(aResponse().withStatus(500))); + + client.send( + HttpRequest.newBuilder(URI.create(server.baseUrl() + "/error")) + .POST(HttpRequest.BodyPublishers.noBody()) + .build(), + HttpResponse.BodyHandlers.discarding() + ); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("500", body.get("status_code")); + assertEquals("POST", body.get("method")); + } + + @Test + public void urlSanitization_stripsQuery() throws Exception { + server.stubFor(get(anyUrl()).willReturn(aResponse().withStatus(500))); + + client.send( + HttpRequest.newBuilder(URI.create(server.baseUrl() + "/path?token=secret")).GET().build(), + HttpResponse.BodyHandlers.discarding() + ); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + String url = body.get("url").toString(); + assertTrue(url.contains("/path")); + assertFalse(url.contains("secret")); + } +} From 8ab880c60aa5d9cc0feafc368adb6a39f7e9f98b Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 25 May 2026 17:36:13 -0300 Subject: [PATCH 05/28] docs(java-agent): add README with installation and manual testing guide --- rollbar-java-agent/README.md | 176 +++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 rollbar-java-agent/README.md diff --git a/rollbar-java-agent/README.md b/rollbar-java-agent/README.md new file mode 100644 index 00000000..e37bbbfd --- /dev/null +++ b/rollbar-java-agent/README.md @@ -0,0 +1,176 @@ +# Rollbar Java Agent + +A zero-code-change Java instrumentation agent that automatically captures HTTP network errors (4xx and 5xx responses) as Rollbar telemetry events. + +It works by attaching to the JVM at startup via `-javaagent:` and using ByteBuddy to intercept HTTP calls across all major clients — no library dependencies or code changes are needed in your application. + +## Instrumented HTTP clients + +| Client | Condition | +|--------|-----------| +| `java.net.HttpURLConnection` | Always (JDK built-in) | +| `java.net.http.HttpClient` | Java 11+ only | +| Apache HttpClient 4.x (`org.apache.http`) | If present on classpath | +| Apache HttpClient 5.x (`org.apache.hc.client5`) | If present on classpath | + +Only 4xx and 5xx responses are recorded. Successful requests (< 400) produce no telemetry. + +## Requirements + +- Java 11 or higher +- `rollbar-java` on the application classpath (for `Rollbar.init(...)`) + +## Installation + +### 1. Build the agent JAR + +```bash +./gradlew :rollbar-java-agent:shadowJar +``` + +The fat JAR (with ByteBuddy bundled and relocated) is written to: + +``` +rollbar-java-agent/build/libs/rollbar-java-agent-.jar +``` + +### 2. Add the agent JVM flag + +Add `-javaagent:` to your JVM startup arguments, pointing at the JAR built above: + +``` +-javaagent:/path/to/rollbar-java-agent-.jar +``` + +**Gradle:** +```kotlin +jvmArgs("-javaagent:/path/to/rollbar-java-agent-.jar") +``` + +**Maven Surefire / Failsafe:** +```xml +-javaagent:/path/to/rollbar-java-agent-.jar +``` + +**Docker / environment variable:** +```bash +JAVA_TOOL_OPTIONS="-javaagent:/path/to/rollbar-java-agent-.jar" +``` + +### 3. Also add the JAR to your classpath + +The agent JAR must also be available on the regular application classpath so that your code can call `RollbarAgent.getTelemetryTracker()`: + +**Gradle:** +```kotlin +dependencies { + implementation(files("/path/to/rollbar-java-agent-.jar")) +} +``` + +**Maven:** +```xml + + com.rollbar + rollbar-java-agent + ${rollbar.version} + +``` + +### 4. Wire into your Rollbar configuration + +```java +import com.rollbar.agent.RollbarAgent; +import com.rollbar.notifier.Rollbar; + +import static com.rollbar.notifier.config.ConfigBuilder.withAccessToken; + +Rollbar rollbar = Rollbar.init( + withAccessToken("your-access-token") + .environment("production") + .telemetryEventTracker(RollbarAgent.getTelemetryTracker()) + .build() +); +``` + +That's it. All HTTP calls your application makes from that point on will automatically produce telemetry events in the Rollbar error report for any 4xx or 5xx response. + +## Behavior + +| Scenario | Action | +|----------|--------| +| Response status `< 400` | No telemetry recorded | +| Response status `>= 400` | Records a network telemetry event with `Level.CRITICAL` | +| Connection failure / I/O error | Records an error telemetry event | +| No Rollbar config wired | Events accumulate in the agent store (capacity 100); nothing is sent | + +## Security + +URLs can carry sensitive data in query parameters or basic-auth credentials. The agent **strips userinfo, query parameters, and the URL fragment** before recording. + +For example, a request to: +``` +https://user:secret@api.example.com/charge?token=sk_live_abc#section +``` +is recorded as: +``` +https://api.example.com/charge +``` + +## Testing + +### Automated tests + +```bash +./gradlew :rollbar-java-agent:test +``` + +This runs the full test suite (WireMock-backed integration tests for each instrumented client). + +### Manual smoke test + +1. Build the agent JAR: + ```bash + ./gradlew :rollbar-java-agent:shadowJar + ``` + +2. Write a small program that triggers a 4xx or 5xx: + ```java + import com.rollbar.agent.RollbarAgent; + import com.rollbar.notifier.Rollbar; + + import java.net.HttpURLConnection; + import java.net.URL; + + import static com.rollbar.notifier.config.ConfigBuilder.withAccessToken; + + public class SmokeTest { + public static void main(String[] args) throws Exception { + Rollbar rollbar = Rollbar.init( + withAccessToken("your-access-token") + .environment("test") + .telemetryEventTracker(RollbarAgent.getTelemetryTracker()) + .build() + ); + + // Trigger a 404 — captured as a telemetry event on the next error report + HttpURLConnection conn = (HttpURLConnection) new URL("https://httpstat.us/404").openConnection(); + int code = conn.getResponseCode(); + conn.disconnect(); + + System.out.println("Response: " + code); + + // Send an error to Rollbar — the 404 telemetry event will appear alongside it + rollbar.error(new RuntimeException("smoke test error")); + } + } + ``` + +3. Run with the agent: + ```bash + java -javaagent:rollbar-java-agent/build/libs/rollbar-java-agent-.jar \ + -cp "rollbar-java-agent/build/libs/rollbar-java-agent-.jar:your-app.jar" \ + SmokeTest + ``` + +4. Check your Rollbar dashboard — the error report for "smoke test error" should show a **Network** telemetry event for the 404 in the telemetry timeline. From ad0b2acf09032f3f56078a7031575956e7865cd3 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Tue, 26 May 2026 00:24:36 -0300 Subject: [PATCH 06/28] chore(java-agent): upgrade wiremock to org.wiremock:wiremock:3.13.2 --- rollbar-java-agent/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rollbar-java-agent/build.gradle.kts b/rollbar-java-agent/build.gradle.kts index 34facdef..dcdc4c7f 100644 --- a/rollbar-java-agent/build.gradle.kts +++ b/rollbar-java-agent/build.gradle.kts @@ -15,7 +15,7 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter") testRuntimeOnly("org.junit.platform:junit-platform-launcher") testImplementation("org.mockito:mockito-core:5.11.0") - testImplementation("com.github.tomakehurst:wiremock-jre8:2.35.2") + testImplementation("org.wiremock:wiremock:3.13.2") testImplementation("org.apache.httpcomponents:httpclient:4.5.14") testImplementation("org.apache.httpcomponents.client5:httpclient5:5.3.1") } From 64e7df9b7fcc0b45032ac52d38fcbae0b7d2150a Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Tue, 26 May 2026 00:48:50 -0300 Subject: [PATCH 07/28] refactor(java-agent): replace resetForTesting with injectable Provider in AgentTelemetryStore --- .../rollbar/agent/AgentTelemetryStore.java | 5 +++-- .../com/rollbar/agent/RollbarAgentTest.java | 22 ++++++++++++++++++- .../HttpURLConnectionInstrumentationTest.java | 2 +- .../JavaHttpClientInstrumentationTest.java | 2 +- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java index 35bf036f..cd6d6503 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java @@ -1,6 +1,7 @@ package com.rollbar.agent; import com.rollbar.api.payload.data.TelemetryEvent; +import com.rollbar.notifier.provider.Provider; import com.rollbar.notifier.telemetry.RollbarTelemetryEventTracker; import com.rollbar.notifier.telemetry.TelemetryEventTracker; @@ -21,7 +22,7 @@ public static List getAll() { return INSTANCE.getAll(); } - public static void resetForTesting() { - INSTANCE = new RollbarTelemetryEventTracker(System::currentTimeMillis, 100); + public static void init(Provider timestampProvider) { + INSTANCE = new RollbarTelemetryEventTracker(timestampProvider, 100); } } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java index 0efbc44c..cc6f82ee 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java @@ -1,16 +1,20 @@ package com.rollbar.agent; +import com.rollbar.api.payload.data.TelemetryEvent; import com.rollbar.notifier.telemetry.TelemetryEventTracker; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.List; + import static org.junit.jupiter.api.Assertions.*; + public class RollbarAgentTest { @BeforeEach public void setUp() { - AgentTelemetryStore.resetForTesting(); + AgentTelemetryStore.init(System::currentTimeMillis); } @Test @@ -20,6 +24,22 @@ public void getTelemetryTracker_returnsSingletonInstance() { assertSame(first, second); } + @Test + public void init_withCustomTimestamp_usesProvidedTimestamp() { + long fixedTime = 1_000_000L; + AgentTelemetryStore.init(() -> fixedTime); + + AgentTelemetryStore.getInstance().recordManualEventFor( + com.rollbar.api.payload.data.Level.WARNING, + com.rollbar.api.payload.data.Source.CLIENT, + "test" + ); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + assertEquals(fixedTime, events.get(0).asJson().get("timestamp_ms")); + } + @Test public void urlSanitizer_stripsQueryAndFragment() { String sanitized = UrlSanitizer.sanitize("https://api.example.com/path?token=secret#section"); diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java index 6044e832..03b91345 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java @@ -26,7 +26,7 @@ public class HttpURLConnectionInstrumentationTest { public void setUp() { server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); server.start(); - AgentTelemetryStore.resetForTesting(); + AgentTelemetryStore.init(System::currentTimeMillis); NetworkEventBridge.resetRecordedForTesting(); } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java index fa31cafd..785f04fe 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java @@ -29,7 +29,7 @@ public void setUp() { server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); server.start(); client = HttpClient.newHttpClient(); - AgentTelemetryStore.resetForTesting(); + AgentTelemetryStore.init(System::currentTimeMillis); NetworkEventBridge.resetRecordedForTesting(); } From 212ce90e7311cfeb347bdb06f47e421441f5637a Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Tue, 26 May 2026 01:07:17 -0300 Subject: [PATCH 08/28] fix(java-agent): resolve checkstyle violations in rollbar-java-agent --- .../com/rollbar/agent/NetworkEventBridge.java | 10 ++++++++ .../java/com/rollbar/agent/RollbarAgent.java | 3 +-- .../ApacheHttpClient4Instrumentation.java | 21 +++++++++++++--- .../ApacheHttpClient5Instrumentation.java | 22 ++++++++++++++--- .../HttpURLConnectionInstrumentation.java | 24 +++++++++++++++---- .../JavaHttpClientInstrumentation.java | 24 +++++++++++++++---- 6 files changed, 86 insertions(+), 18 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java index 947c5b34..332bcffb 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java @@ -37,6 +37,11 @@ public static boolean markAsRecorded(Object key) { return RECORDED.add(key); } + /** + * Records a network telemetry event for the given key if not already recorded. + * + *

Uses the key as a deduplication token — subsequent calls with the same key are ignored. + */ public static void recordNetworkEvent(Object key, String method, String url, String statusCode) { if (!markAsRecorded(key)) { return; // deduplicate re-entrant calls for the same connection @@ -50,6 +55,11 @@ public static void recordNetworkEvent(Object key, String method, String url, Str ); } + /** + * Records a manual error telemetry event with the given message. + * + *

Called when an HTTP request fails with an I/O exception rather than a status code. + */ public static void recordError(String message) { AgentTelemetryStore.getInstance().recordManualEventFor( Level.CRITICAL, diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java index 8eb04d68..fc3ef476 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java @@ -5,11 +5,10 @@ import com.rollbar.agent.instrumentation.HttpURLConnectionInstrumentation; import com.rollbar.agent.instrumentation.JavaHttpClientInstrumentation; import com.rollbar.notifier.telemetry.TelemetryEventTracker; +import java.lang.instrument.Instrumentation; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.matcher.ElementMatchers; -import java.lang.instrument.Instrumentation; - /** * Java agent entry point. Attach with {@code -javaagent:/path/to/rollbar-java-agent.jar}. * diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java index be57fabf..6a86d86e 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java @@ -4,18 +4,25 @@ import com.rollbar.agent.UrlSanitizer; import com.rollbar.api.payload.data.Level; import com.rollbar.api.payload.data.Source; +import java.lang.instrument.Instrumentation; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatchers; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpUriRequest; -import java.lang.instrument.Instrumentation; - +/** + * Installs ByteBuddy advice on Apache HttpClient 4.x to capture network errors. + */ public final class ApacheHttpClient4Instrumentation { private ApacheHttpClient4Instrumentation() {} + /** + * Instruments Apache HttpClient 4.x if present on the classpath. + * + *

Does nothing if {@code org.apache.http.impl.client.CloseableHttpClient} is not available. + */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { try { Class.forName("org.apache.http.impl.client.CloseableHttpClient"); @@ -41,6 +48,12 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst */ public static class ExecuteAdvice { + /** + * Fires after {@code execute()} returns or throws, recording 4xx/5xx responses as telemetry. + * + *

Apache HC 4.x runs in the application classloader, so Rollbar classes are referenced + * directly without the TCCL reflection bridge needed for JDK instrumentation. + */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( @Advice.Argument(0) HttpUriRequest request, @@ -49,10 +62,12 @@ public static void onExit( ) { try { if (thrown != null) { + String message = thrown.getMessage() != null + ? thrown.getMessage() : thrown.getClass().getName(); AgentTelemetryStore.getInstance().recordManualEventFor( Level.CRITICAL, Source.SERVER, - "Network error: " + (thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName()) + "Network error: " + message ); return; } diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java index 95d6e8c4..d2aaf927 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java @@ -4,18 +4,26 @@ import com.rollbar.agent.UrlSanitizer; import com.rollbar.api.payload.data.Level; import com.rollbar.api.payload.data.Source; +import java.lang.instrument.Instrumentation; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatchers; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; -import java.lang.instrument.Instrumentation; - +/** + * Installs ByteBuddy advice on Apache HttpClient 5.x to capture network errors. + */ public final class ApacheHttpClient5Instrumentation { private ApacheHttpClient5Instrumentation() {} + /** + * Instruments Apache HttpClient 5.x if present on the classpath. + * + *

Does nothing if {@code org.apache.hc.client5.http.impl.classic.CloseableHttpClient} + * is not available. + */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { try { Class.forName("org.apache.hc.client5.http.impl.classic.CloseableHttpClient"); @@ -41,6 +49,12 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst */ public static class ExecuteAdvice { + /** + * Fires after {@code execute()} returns or throws, recording 4xx/5xx responses as telemetry. + * + *

Apache HC 5.x runs in the application classloader, so Rollbar classes are referenced + * directly without the TCCL reflection bridge needed for JDK instrumentation. + */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( @Advice.Argument(0) ClassicHttpRequest request, @@ -49,10 +63,12 @@ public static void onExit( ) { try { if (thrown != null) { + String message = thrown.getMessage() != null + ? thrown.getMessage() : thrown.getClass().getName(); AgentTelemetryStore.getInstance().recordManualEventFor( Level.CRITICAL, Source.SERVER, - "Network error: " + (thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName()) + "Network error: " + message ); return; } diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java index b2a8c513..3f3e1447 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java @@ -1,15 +1,22 @@ package com.rollbar.agent.instrumentation; +import java.lang.instrument.Instrumentation; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatchers; -import java.lang.instrument.Instrumentation; - +/** + * Installs ByteBuddy advice on {@code java.net.HttpURLConnection} to capture network errors. + */ public final class HttpURLConnectionInstrumentation { private HttpURLConnectionInstrumentation() {} + /** + * Instruments {@code java.net.HttpURLConnection.getResponseCode()} to record 4xx/5xx responses. + * + *

Targets the declaring class directly to capture the method regardless of concrete subtype. + */ public static void install(AgentBuilder builder, Instrumentation inst) { builder .type(ElementMatchers.named("java.net.HttpURLConnection")) @@ -29,6 +36,12 @@ public static void install(AgentBuilder builder, Instrumentation inst) { */ public static class GetResponseCodeAdvice { + /** + * Fires after {@code getResponseCode()} returns or throws, recording 4xx/5xx as telemetry. + * + *

Uses the connection instance as a deduplication key since {@code getResponseCode()} is + * called re-entrantly up to 3 times per request internally. + */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( @Advice.This Object connection, @@ -46,7 +59,8 @@ public static void onExit( Boolean recorded = (Boolean) bridge .getMethod("markAsRecorded", Object.class).invoke(null, thrown); if (recorded) { - String msg = thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName(); + String msg = thrown.getMessage() != null + ? thrown.getMessage() : thrown.getClass().getName(); bridge.getMethod("recordError", String.class).invoke(null, msg); } return; @@ -55,8 +69,8 @@ public static void onExit( if (statusCode >= 400) { Object url = connection.getClass().getMethod("getURL").invoke(connection); String urlStr = url != null ? url.toString() : ""; - String method = (String) connection.getClass().getMethod("getRequestMethod").invoke(connection); - // connection instance as dedup key — same object across all re-entrant getResponseCode() calls + String method = (String) connection.getClass() + .getMethod("getRequestMethod").invoke(connection); bridge.getMethod("recordNetworkEvent", Object.class, String.class, String.class, String.class) .invoke(null, connection, method, urlStr, String.valueOf(statusCode)); diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java index 5247f832..5f15a92c 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java @@ -1,16 +1,23 @@ package com.rollbar.agent.instrumentation; +import java.lang.instrument.Instrumentation; +import java.net.http.HttpClient; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatchers; -import java.lang.instrument.Instrumentation; -import java.net.http.HttpClient; - +/** + * Installs ByteBuddy advice on {@code java.net.http.HttpClient} subtypes to capture network errors. + */ public final class JavaHttpClientInstrumentation { private JavaHttpClientInstrumentation() {} + /** + * Instruments {@code java.net.http.HttpClient} subtypes if available on the current JVM. + * + *

Does nothing if {@code java.net.http.HttpClient} is not present (i.e. below Java 11). + */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { try { Class.forName("java.net.http.HttpClient"); @@ -37,6 +44,12 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst */ public static class SendAdvice { + /** + * Fires after {@code send()} returns or throws, recording 4xx/5xx responses as telemetry. + * + *

Uses the response object as a deduplication key to avoid duplicate events when both + * {@code HttpClientFacade} and {@code HttpClientImpl} invoke this advice. + */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( @Advice.Argument(0) Object request, @@ -54,8 +67,9 @@ public static void onExit( Boolean recorded = (Boolean) bridge .getMethod("markAsRecorded", Object.class).invoke(null, thrown); if (recorded) { - String msg = thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName(); - bridge.getMethod("recordError", String.class).invoke(null, msg); + String message = thrown.getMessage() != null + ? thrown.getMessage() : thrown.getClass().getName(); + bridge.getMethod("recordError", String.class).invoke(null, message); } return; } From fe91c9af99f5a8ee63df54b275f6d5b2f058a032 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Tue, 26 May 2026 01:20:36 -0300 Subject: [PATCH 09/28] fix(java-agent): resolve checkstyle violations in rollbar-java-agent --- .../src/main/java/com/rollbar/agent/RollbarAgent.java | 4 ++-- ...rumentation.java => HttpUrlConnectionInstrumentation.java} | 4 ++-- ...ionTest.java => HttpUrlConnectionInstrumentationTest.java} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/{HttpURLConnectionInstrumentation.java => HttpUrlConnectionInstrumentation.java} (96%) rename rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/{HttpURLConnectionInstrumentationTest.java => HttpUrlConnectionInstrumentationTest.java} (98%) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java index fc3ef476..6377bb11 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java @@ -2,7 +2,7 @@ import com.rollbar.agent.instrumentation.ApacheHttpClient4Instrumentation; import com.rollbar.agent.instrumentation.ApacheHttpClient5Instrumentation; -import com.rollbar.agent.instrumentation.HttpURLConnectionInstrumentation; +import com.rollbar.agent.instrumentation.HttpUrlConnectionInstrumentation; import com.rollbar.agent.instrumentation.JavaHttpClientInstrumentation; import com.rollbar.notifier.telemetry.TelemetryEventTracker; import java.lang.instrument.Instrumentation; @@ -41,7 +41,7 @@ private static void installInstrumentation(Instrumentation inst) { .with(AgentBuilder.InitializationStrategy.NoOp.INSTANCE) .with(AgentBuilder.TypeStrategy.Default.REDEFINE); - HttpURLConnectionInstrumentation.install(builder, inst); + HttpUrlConnectionInstrumentation.install(builder, inst); JavaHttpClientInstrumentation.installIfAvailable(builder, inst); ApacheHttpClient4Instrumentation.installIfAvailable(builder, inst); ApacheHttpClient5Instrumentation.installIfAvailable(builder, inst); diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java similarity index 96% rename from rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java rename to rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java index 3f3e1447..5ee10ec6 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java @@ -8,9 +8,9 @@ /** * Installs ByteBuddy advice on {@code java.net.HttpURLConnection} to capture network errors. */ -public final class HttpURLConnectionInstrumentation { +public final class HttpUrlConnectionInstrumentation { - private HttpURLConnectionInstrumentation() {} + private HttpUrlConnectionInstrumentation() {} /** * Instruments {@code java.net.HttpURLConnection.getResponseCode()} to record 4xx/5xx responses. diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java similarity index 98% rename from rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java rename to rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java index 03b91345..319556ad 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpURLConnectionInstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java @@ -18,7 +18,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.*; import static org.junit.jupiter.api.Assertions.*; -public class HttpURLConnectionInstrumentationTest { +public class HttpUrlConnectionInstrumentationTest { private WireMockServer server; From 866b20b7ebb5821953761c368c28611ab55cf67b Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Tue, 26 May 2026 02:15:18 -0300 Subject: [PATCH 10/28] fix(rollbar-java-agent): declare explicit dependency on jar task in test --- rollbar-java-agent/build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/rollbar-java-agent/build.gradle.kts b/rollbar-java-agent/build.gradle.kts index dcdc4c7f..ff1119ad 100644 --- a/rollbar-java-agent/build.gradle.kts +++ b/rollbar-java-agent/build.gradle.kts @@ -52,4 +52,5 @@ tasks.test { // system classloader; mirrors production use where rollbar-java-agent is a Gradle/Maven dep classpath += files(agentJar) dependsOn(tasks.shadowJar) + dependsOn(tasks.jar) } From 6791575bf0ff5c603eedb85506db0a82711f556c Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 1 Jun 2026 02:27:02 -0300 Subject: [PATCH 11/28] feat(java-agent): instrument HttpClient.sendAsync() for async network telemetry --- .../com/rollbar/agent/NetworkEventBridge.java | 38 ++++++++++ .../JavaHttpClientInstrumentation.java | 56 +++++++++++++- .../JavaHttpClientInstrumentationTest.java | 73 +++++++++++++++++++ 3 files changed, 165 insertions(+), 2 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java index 332bcffb..c3a2bcac 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java @@ -55,6 +55,44 @@ public static void recordNetworkEvent(Object key, String method, String url, Str ); } + /** + * Returns a {@link java.util.function.BiConsumer} that records telemetry when an async + * HTTP response completes. Intended to be chained via {@code CompletableFuture.whenComplete}. + * + *

The callback is created here (in the app classloader) so it can reference Rollbar types + * directly, avoiding the reflection overhead that advice code needs to cross the classloader gap. + */ + public static java.util.function.BiConsumer createAsyncCallback( + Object request) { + return (response, thrown) -> { + try { + if (thrown != null) { + if (markAsRecorded(thrown)) { + String message = thrown.getMessage() != null + ? thrown.getMessage() : thrown.getClass().getName(); + recordError(message); + } + return; + } + if (response != null) { + // Look up methods via public interfaces, not the internal JDK implementation class. + // NetworkEventBridge runs in the app classloader (unnamed module) and cannot access + // jdk.internal.net.http.*; java.net.http.* is exported and accessible. + Class httpResponseIface = Class.forName("java.net.http.HttpResponse"); + Class httpRequestIface = Class.forName("java.net.http.HttpRequest"); + int statusCode = (Integer) httpResponseIface.getMethod("statusCode").invoke(response); + if (statusCode >= 400) { + Object uri = httpRequestIface.getMethod("uri").invoke(request); + String method = (String) httpRequestIface.getMethod("method").invoke(request); + recordNetworkEvent(response, method, uri.toString(), String.valueOf(statusCode)); + } + } + } catch (Throwable ignored) { + // Callback must never throw — swallow all errors + } + }; + } + /** * Records a manual error telemetry event with the given message. * diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java index 5f15a92c..f36b1971 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentation.java @@ -28,12 +28,64 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst builder .type(ElementMatchers.isSubTypeOf(HttpClient.class)) .transform((b, typeDescription, classLoader, module, protectionDomain) -> - b.visit(Advice.to(SendAdvice.class) - .on(ElementMatchers.named("send"))) + b.visit(Advice.to(SendAdvice.class).on(ElementMatchers.named("send"))) + .visit(Advice.to(SendAsyncAdvice.class).on(ElementMatchers.named("sendAsync"))) ) .installOn(inst); } + /** + * Advice inlined into JDK's HttpClient concrete implementation's sendAsync(). + * + *

At method exit, chains a {@code whenComplete} callback onto the returned + * {@code CompletableFuture}. The callback is created via the bridge (in the app classloader) + * so it can reference Rollbar types without further reflection at completion time. + * Deduplication is handled by the bridge using the response object as the key, which works the + * same way as for sync send(): both HttpClientFacade and HttpClientImpl contribute a callback, + * but only the first one to run with a given response object records an event. + */ + public static class SendAsyncAdvice { + + /** + * Fires after {@code sendAsync()} returns, chaining a telemetry callback on the future. + */ + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void onExit( + @Advice.Argument(0) Object request, + @Advice.Return Object future, + @Advice.Thrown Throwable thrown + ) { + try { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + if (cl == null) { + cl = ClassLoader.getSystemClassLoader(); + } + Class bridge = cl.loadClass("com.rollbar.agent.NetworkEventBridge"); + + if (thrown != null) { + Boolean recorded = (Boolean) bridge + .getMethod("markAsRecorded", Object.class).invoke(null, thrown); + if (recorded) { + String message = thrown.getMessage() != null + ? thrown.getMessage() : thrown.getClass().getName(); + bridge.getMethod("recordError", String.class).invoke(null, message); + } + return; + } + + if (future != null) { + Object callback = bridge + .getMethod("createAsyncCallback", Object.class).invoke(null, request); + future.getClass() + .getMethod("whenComplete", java.util.function.BiConsumer.class) + .invoke(future, callback); + } + } catch (Throwable ignored) { + // Advice must never throw — swallow all errors including Error subclasses + } + } + } + /** * Advice inlined into JDK's HttpClient concrete implementation's send(). * diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java index 785f04fe..fa5a0469 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java @@ -15,6 +15,8 @@ import java.net.http.HttpResponse; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; import static com.github.tomakehurst.wiremock.client.WireMock.*; import static org.junit.jupiter.api.Assertions.*; @@ -87,6 +89,77 @@ public void serverErrorResponse_recordsNetworkEvent() throws Exception { assertEquals("POST", body.get("method")); } + @Test + public void sendAsync_successResponse_doesNotRecordEvent() throws Exception { + server.stubFor(get(urlEqualTo("/ok-async")).willReturn(aResponse().withStatus(200))); + + client.sendAsync( + HttpRequest.newBuilder(URI.create(server.baseUrl() + "/ok-async")).GET().build(), + HttpResponse.BodyHandlers.discarding() + ).get(5, TimeUnit.SECONDS); + + // whenComplete callbacks fire in the HTTP thread; no event expected for 2xx + Thread.sleep(50); + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void sendAsync_clientErrorResponse_recordsNetworkEvent() throws Exception { + server.stubFor(get(urlEqualTo("/not-found-async")).willReturn(aResponse().withStatus(404))); + + client.sendAsync( + HttpRequest.newBuilder(URI.create(server.baseUrl() + "/not-found-async")).GET().build(), + HttpResponse.BodyHandlers.discarding() + ).get(5, TimeUnit.SECONDS); + + List events = awaitEvents(() -> AgentTelemetryStore.getInstance().getAll(), 1, 1000); + assertEquals(1, events.size()); + Map json = events.get(0).asJson(); + assertEquals("network", json.get("type")); + Map body = (Map) json.get("body"); + assertEquals("404", body.get("status_code")); + assertEquals("GET", body.get("method")); + assertTrue(body.get("url").toString().contains("/not-found-async")); + } + + @Test + public void sendAsync_serverErrorResponse_recordsNetworkEvent() throws Exception { + server.stubFor(post(urlEqualTo("/error-async")).willReturn(aResponse().withStatus(500))); + + client.sendAsync( + HttpRequest.newBuilder(URI.create(server.baseUrl() + "/error-async")) + .POST(HttpRequest.BodyPublishers.noBody()) + .build(), + HttpResponse.BodyHandlers.discarding() + ).get(5, TimeUnit.SECONDS); + + List events = awaitEvents(() -> AgentTelemetryStore.getInstance().getAll(), 1, 1000); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("500", body.get("status_code")); + assertEquals("POST", body.get("method")); + } + + /** + * Polls until the supplier returns a list with at least {@code minCount} elements or + * {@code timeoutMs} elapses. The {@code whenComplete} callbacks from async advice fire in + * the HTTP-client thread, so they may arrive a few milliseconds after {@code get()} returns. + */ + private static List awaitEvents( + Supplier> supplier, int minCount, long timeoutMs) + throws InterruptedException { + long deadline = System.currentTimeMillis() + timeoutMs; + List events; + do { + events = supplier.get(); + if (events.size() >= minCount) { + return events; + } + Thread.sleep(5); + } while (System.currentTimeMillis() < deadline); + return events; + } + @Test public void urlSanitization_stripsQuery() throws Exception { server.stubFor(get(anyUrl()).willReturn(aResponse().withStatus(500))); From ba9f0d84917cbf4d4d1039ad42f8f9544e3c0833 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 1 Jun 2026 03:09:45 -0300 Subject: [PATCH 12/28] fix(java-agent): fix Apache HC4 transformer to handle all execute() overloads safely --- .../ApacheHttpClient4Instrumentation.java | 40 +++++-- .../ApacheHttpClient4InstrumentationTest.java | 108 ++++++++++++++++++ 2 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java index 6a86d86e..dda8b23c 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java @@ -24,20 +24,34 @@ private ApacheHttpClient4Instrumentation() {} *

Does nothing if {@code org.apache.http.impl.client.CloseableHttpClient} is not available. */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { - try { - Class.forName("org.apache.http.impl.client.CloseableHttpClient"); - } catch (ClassNotFoundException e) { + // Use a resource check rather than Class.forName: execute(HttpUriRequest, HttpContext) is a + // concrete method defined in CloseableHttpClient itself, so loading that class before the + // ByteBuddy transformer is installed prevents the method from ever being instrumented (no + // RedefinitionStrategy is configured). getResource checks classpath availability without + // triggering class loading. + if (ClassLoader.getSystemClassLoader().getResource( + "org/apache/http/impl/client/CloseableHttpClient.class") == null) { return; } builder - .type(ElementMatchers.hasSuperType( - ElementMatchers.named("org.apache.http.impl.client.CloseableHttpClient"))) + .type(ElementMatchers.named("org.apache.http.impl.client.CloseableHttpClient")) .transform((b, typeDescription, classLoader, module, protectionDomain) -> b.visit(Advice.to(ExecuteAdvice.class) .on(ElementMatchers.named("execute") + // Target only execute(HttpUriRequest, HttpContext) — the single concrete + // method that all other execute() overloads delegate to before calling + // doExecute(). This ensures exactly one advice invocation per HTTP request + // regardless of which public overload the caller uses (including ResponseHandler + // variants). The bridge method for this overload is excluded to avoid a second + // firing; the 1 arg execute(HttpUriRequest) is excluded because it has no arg + // at index 1, and ResponseHandler overloads are excluded because their second + // arg is ResponseHandler, not HttpContext. + .and(ElementMatchers.not(ElementMatchers.isBridge())) .and(ElementMatchers.takesArgument(0, - ElementMatchers.named("org.apache.http.client.methods.HttpUriRequest"))))) + ElementMatchers.named("org.apache.http.client.methods.HttpUriRequest"))) + .and(ElementMatchers.takesArgument(1, + ElementMatchers.named("org.apache.http.protocol.HttpContext"))))) ) .installOn(inst); } @@ -45,14 +59,17 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst /** * Apache HC 4.x runs in the application classloader, so we can reference Rollbar classes * directly without the TCCL reflection bridge. + * + *

String concatenation in the advice body must use {@link String#concat} or + * {@link StringBuilder} rather than the {@code +} operator. Apache HC 4.x jars are compiled at + * class-file version 50 (Java 6); the Java 9+ compiler emits {@code invokedynamic} for {@code +} + * concatenation, which ByteBuddy cannot inline into a Java 6 class file. */ public static class ExecuteAdvice { /** - * Fires after {@code execute()} returns or throws, recording 4xx/5xx responses as telemetry. - * - *

Apache HC 4.x runs in the application classloader, so Rollbar classes are referenced - * directly without the TCCL reflection bridge needed for JDK instrumentation. + * Fires after {@code execute(HttpUriRequest, HttpContext)} returns or throws, recording + * 4xx/5xx responses as telemetry. */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( @@ -64,10 +81,11 @@ public static void onExit( if (thrown != null) { String message = thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName(); + // Use String.concat instead of + to avoid invokedynamic (unsupported in Java 6 class files) AgentTelemetryStore.getInstance().recordManualEventFor( Level.CRITICAL, Source.SERVER, - "Network error: " + message + "Network error: ".concat(message) ); return; } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java new file mode 100644 index 00000000..b8f3663e --- /dev/null +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java @@ -0,0 +1,108 @@ +package com.rollbar.agent.instrumentation; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.rollbar.agent.AgentTelemetryStore; +import com.rollbar.agent.NetworkEventBridge; +import com.rollbar.api.payload.data.TelemetryEvent; +import org.apache.http.client.ResponseHandler; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static org.junit.jupiter.api.Assertions.*; + +public class ApacheHttpClient4InstrumentationTest { + + private WireMockServer server; + private CloseableHttpClient client; + + @BeforeEach + public void setUp() { + server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); + server.start(); + client = HttpClients.createDefault(); + AgentTelemetryStore.init(System::currentTimeMillis); + NetworkEventBridge.resetRecordedForTesting(); + } + + @AfterEach + public void tearDown() throws Exception { + client.close(); + server.stop(); + } + + @Test + public void successResponse_doesNotRecordEvent() throws Exception { + server.stubFor(get(urlEqualTo("/ok")).willReturn(aResponse().withStatus(200))); + + client.execute(new HttpGet(server.baseUrl() + "/ok")).close(); + + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void clientErrorResponse_recordsNetworkEvent() throws Exception { + server.stubFor(get(urlEqualTo("/not-found")).willReturn(aResponse().withStatus(404))); + + client.execute(new HttpGet(server.baseUrl() + "/not-found")).close(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map json = events.get(0).asJson(); + assertEquals("network", json.get("type")); + Map body = (Map) json.get("body"); + assertEquals("404", body.get("status_code")); + assertEquals("GET", body.get("method")); + assertTrue(body.get("url").toString().contains("/not-found")); + } + + @Test + public void serverErrorResponse_recordsNetworkEvent() throws Exception { + server.stubFor(post(urlEqualTo("/error")).willReturn(aResponse().withStatus(500))); + + client.execute(new HttpPost(server.baseUrl() + "/error")).close(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("500", body.get("status_code")); + assertEquals("POST", body.get("method")); + } + + @Test + public void responseHandlerOverload_doesNotBreakInstrumentation() throws Exception { + // execute(HttpUriRequest, ResponseHandler) routes through execute(HttpHost, HttpRequest, ...) + // — a separate call chain that does not pass through execute(HttpUriRequest, HttpContext). + // No telemetry event is produced for this path, but the call must succeed without error. + server.stubFor(get(urlEqualTo("/handler")).willReturn(aResponse().withStatus(404))); + + ResponseHandler handler = response -> response.getStatusLine().getStatusCode(); + int status = client.execute(new HttpGet(server.baseUrl() + "/handler"), handler); + + assertEquals(404, status); + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void urlSanitization_stripsQuery() throws Exception { + server.stubFor(get(anyUrl()).willReturn(aResponse().withStatus(500))); + + client.execute(new HttpGet(server.baseUrl() + "/path?token=secret")).close(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + String url = body.get("url").toString(); + assertTrue(url.contains("/path")); + assertFalse(url.contains("secret")); + } +} From 02fe6504ee67f9f899860e6c764c9823f5e9eedd Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 1 Jun 2026 03:22:55 -0300 Subject: [PATCH 13/28] fix(java-agent): fix Apache HC5 transformer to handle all execute() overloads safely --- .../ApacheHttpClient5Instrumentation.java | 30 +++-- .../ApacheHttpClient5InstrumentationTest.java | 122 ++++++++++++++++++ 2 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java index d2aaf927..9c2be56c 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java @@ -25,20 +25,35 @@ private ApacheHttpClient5Instrumentation() {} * is not available. */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { - try { - Class.forName("org.apache.hc.client5.http.impl.classic.CloseableHttpClient"); - } catch (ClassNotFoundException e) { + // Use a resource check rather than Class.forName: execute() overloads are concrete methods + // defined in CloseableHttpClient itself, so loading that class before the ByteBuddy transformer + // is installed prevents the methods from ever being instrumented (no RedefinitionStrategy is + // configured). getResource checks classpath availability without triggering class loading. + if (ClassLoader.getSystemClassLoader().getResource( + "org/apache/hc/client5/http/impl/classic/CloseableHttpClient.class") == null) { return; } builder - .type(ElementMatchers.hasSuperType( - ElementMatchers.named("org.apache.hc.client5.http.impl.classic.CloseableHttpClient"))) + .type(ElementMatchers.named("org.apache.hc.client5.http.impl.classic.CloseableHttpClient")) .transform((b, typeDescription, classLoader, module, protectionDomain) -> b.visit(Advice.to(ExecuteAdvice.class) .on(ElementMatchers.named("execute") + // Exclude bridge methods generated for covariant return-type overrides — they + // return HttpResponse (supertype), which @Advice.Return ClassicHttpResponse + // cannot safely bind via downcast. + .and(ElementMatchers.not(ElementMatchers.isBridge())) .and(ElementMatchers.takesArgument(0, - ElementMatchers.named("org.apache.hc.core5.http.ClassicHttpRequest"))))) + ElementMatchers.named("org.apache.hc.core5.http.ClassicHttpRequest"))) + // Exclude HttpClientResponseHandler overloads: they return generic T erased to + // Object, which cannot be bound to @Advice.Return ClassicHttpResponse and would + // cause ByteBuddy to fail the transformation for the whole CloseableHttpClient. + // HC5 has both 2-arg (arg1=handler) and 3-arg (arg1=context, arg2=handler) + // variants; both are excluded here. + .and(ElementMatchers.not(ElementMatchers.takesArgument(1, + ElementMatchers.named("org.apache.hc.core5.http.io.HttpClientResponseHandler")))) + .and(ElementMatchers.not(ElementMatchers.takesArgument(2, + ElementMatchers.named("org.apache.hc.core5.http.io.HttpClientResponseHandler")))))) ) .installOn(inst); } @@ -51,9 +66,6 @@ public static class ExecuteAdvice { /** * Fires after {@code execute()} returns or throws, recording 4xx/5xx responses as telemetry. - * - *

Apache HC 5.x runs in the application classloader, so Rollbar classes are referenced - * directly without the TCCL reflection bridge needed for JDK instrumentation. */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java new file mode 100644 index 00000000..87027b6f --- /dev/null +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java @@ -0,0 +1,122 @@ +package com.rollbar.agent.instrumentation; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.rollbar.agent.AgentTelemetryStore; +import com.rollbar.agent.NetworkEventBridge; +import com.rollbar.api.payload.data.TelemetryEvent; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.core5.http.io.HttpClientResponseHandler; +import org.apache.hc.core5.http.message.BasicClassicHttpRequest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static org.junit.jupiter.api.Assertions.*; + +public class ApacheHttpClient5InstrumentationTest { + + private WireMockServer server; + private CloseableHttpClient client; + + @BeforeEach + public void setUp() { + server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); + server.start(); + client = HttpClients.createDefault(); + AgentTelemetryStore.init(System::currentTimeMillis); + NetworkEventBridge.resetRecordedForTesting(); + } + + @AfterEach + public void tearDown() throws Exception { + client.close(); + server.stop(); + } + + @Test + public void successResponse_doesNotRecordEvent() throws Exception { + server.stubFor(get(urlEqualTo("/ok")).willReturn(aResponse().withStatus(200))); + + try (CloseableHttpResponse r = client.execute( + new BasicClassicHttpRequest("GET", server.baseUrl() + "/ok"))) { + // consume response + } + + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void clientErrorResponse_recordsNetworkEvent() throws Exception { + server.stubFor(get(urlEqualTo("/not-found")).willReturn(aResponse().withStatus(404))); + + try (CloseableHttpResponse r = client.execute( + new BasicClassicHttpRequest("GET", server.baseUrl() + "/not-found"))) { + // consume response + } + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map json = events.get(0).asJson(); + assertEquals("network", json.get("type")); + Map body = (Map) json.get("body"); + assertEquals("404", body.get("status_code")); + assertEquals("GET", body.get("method")); + assertTrue(body.get("url").toString().contains("/not-found")); + } + + @Test + public void serverErrorResponse_recordsNetworkEvent() throws Exception { + server.stubFor(post(urlEqualTo("/error")).willReturn(aResponse().withStatus(500))); + + try (CloseableHttpResponse r = client.execute( + new BasicClassicHttpRequest("POST", server.baseUrl() + "/error"))) { + // consume response + } + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("500", body.get("status_code")); + assertEquals("POST", body.get("method")); + } + + @Test + public void responseHandlerOverload_doesNotBreakInstrumentation() throws Exception { + // execute(ClassicHttpRequest, HttpClientResponseHandler) routes through a separate call chain + // (HttpHost-based) that does not pass through the instrumented execute(ClassicHttpRequest) + // or execute(ClassicHttpRequest, HttpContext) overloads. No telemetry event is produced, + // but the call must succeed without error. + server.stubFor(get(urlEqualTo("/handler")).willReturn(aResponse().withStatus(404))); + + HttpClientResponseHandler handler = response -> response.getCode(); + int status = client.execute( + new BasicClassicHttpRequest("GET", server.baseUrl() + "/handler"), handler); + + assertEquals(404, status); + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void urlSanitization_stripsQuery() throws Exception { + server.stubFor(get(anyUrl()).willReturn(aResponse().withStatus(500))); + + try (CloseableHttpResponse r = client.execute( + new BasicClassicHttpRequest("GET", server.baseUrl() + "/path?token=secret"))) { + // consume response + } + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + String url = body.get("url").toString(); + assertTrue(url.contains("/path")); + assertFalse(url.contains("secret")); + } +} From ebf0c39a1bbd85c8239d6e66d16bfdff9500da1a Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 1 Jun 2026 03:24:34 -0300 Subject: [PATCH 14/28] docs: update readme of rollbar-java-agent --- rollbar-java-agent/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rollbar-java-agent/README.md b/rollbar-java-agent/README.md index e37bbbfd..d2702e17 100644 --- a/rollbar-java-agent/README.md +++ b/rollbar-java-agent/README.md @@ -9,12 +9,14 @@ It works by attaching to the JVM at startup via `-javaagent:` and using ByteBudd | Client | Condition | |--------|-----------| | `java.net.HttpURLConnection` | Always (JDK built-in) | -| `java.net.http.HttpClient` | Java 11+ only | +| `java.net.http.HttpClient` — `send()` and `sendAsync()` | Java 11+ only | | Apache HttpClient 4.x (`org.apache.http`) | If present on classpath | | Apache HttpClient 5.x (`org.apache.hc.client5`) | If present on classpath | Only 4xx and 5xx responses are recorded. Successful requests (< 400) produce no telemetry. +> **Apache HC4/HC5 note:** `execute(request, responseHandler)` overloads route through a separate internal call chain and are not instrumented. Use the standard `execute(request)` / `execute(request, context)` overloads to get telemetry. + ## Requirements - Java 11 or higher From 5259919da1e888004b61a3818b4f77fc176af595 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 1 Jun 2026 16:21:42 -0300 Subject: [PATCH 15/28] style(java-agent): fix checkstyle line length violations in HC4 and HC5 instrumentation --- .../instrumentation/ApacheHttpClient4Instrumentation.java | 6 ++++-- .../instrumentation/ApacheHttpClient5Instrumentation.java | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java index dda8b23c..e2a8782f 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java @@ -42,7 +42,8 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst // Target only execute(HttpUriRequest, HttpContext) — the single concrete // method that all other execute() overloads delegate to before calling // doExecute(). This ensures exactly one advice invocation per HTTP request - // regardless of which public overload the caller uses (including ResponseHandler + // regardless of which public overload the caller uses (including + // ResponseHandler // variants). The bridge method for this overload is excluded to avoid a second // firing; the 1 arg execute(HttpUriRequest) is excluded because it has no arg // at index 1, and ResponseHandler overloads are excluded because their second @@ -81,7 +82,8 @@ public static void onExit( if (thrown != null) { String message = thrown.getMessage() != null ? thrown.getMessage() : thrown.getClass().getName(); - // Use String.concat instead of + to avoid invokedynamic (unsupported in Java 6 class files) + // Use String.concat instead of + to avoid invokedynamic + // (unsupported in Java 6 class files) AgentTelemetryStore.getInstance().recordManualEventFor( Level.CRITICAL, Source.SERVER, diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java index 9c2be56c..da394bd5 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java @@ -51,9 +51,11 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst // HC5 has both 2-arg (arg1=handler) and 3-arg (arg1=context, arg2=handler) // variants; both are excluded here. .and(ElementMatchers.not(ElementMatchers.takesArgument(1, - ElementMatchers.named("org.apache.hc.core5.http.io.HttpClientResponseHandler")))) + ElementMatchers.named( + "org.apache.hc.core5.http.io.HttpClientResponseHandler")))) .and(ElementMatchers.not(ElementMatchers.takesArgument(2, - ElementMatchers.named("org.apache.hc.core5.http.io.HttpClientResponseHandler")))))) + ElementMatchers.named( + "org.apache.hc.core5.http.io.HttpClientResponseHandler")))))) ) .installOn(inst); } From d9237e4a375bf76662ca6b9b67d93609d8339161 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Fri, 12 Jun 2026 02:48:29 -0300 Subject: [PATCH 16/28] fix(java-agent): record full URL in HC5 instrumentation --- .../ApacheHttpClient5Instrumentation.java | 10 +++++++++- .../ApacheHttpClient5InstrumentationTest.java | 11 ++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java index da394bd5..919f3bc8 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java @@ -5,6 +5,8 @@ import com.rollbar.api.payload.data.Level; import com.rollbar.api.payload.data.Source; import java.lang.instrument.Instrumentation; +import java.net.URI; + import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatchers; @@ -90,7 +92,13 @@ public static void onExit( if (response != null) { int statusCode = response.getCode(); if (statusCode >= 400) { - String url = request.getRequestUri() != null ? request.getRequestUri() : ""; + String url; + try { + URI uri = request.getUri(); + url = uri != null ? uri.toString() : ""; + } catch (java.net.URISyntaxException ignored) { + url = request.getRequestUri() != null ? request.getRequestUri() : ""; + } AgentTelemetryStore.getInstance().recordNetworkEventFor( Level.CRITICAL, Source.SERVER, diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java index 87027b6f..74f810d9 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java @@ -68,7 +68,10 @@ public void clientErrorResponse_recordsNetworkEvent() throws Exception { Map body = (Map) json.get("body"); assertEquals("404", body.get("status_code")); assertEquals("GET", body.get("method")); - assertTrue(body.get("url").toString().contains("/not-found")); + String url = body.get("url").toString(); + assertTrue(url.startsWith("http://"), "URL should include scheme: " + url); + assertTrue(url.contains("localhost"), "URL should include host: " + url); + assertTrue(url.contains("/not-found"), "URL should include path: " + url); } @Test @@ -116,7 +119,9 @@ public void urlSanitization_stripsQuery() throws Exception { assertEquals(1, events.size()); Map body = (Map) events.get(0).asJson().get("body"); String url = body.get("url").toString(); - assertTrue(url.contains("/path")); - assertFalse(url.contains("secret")); + assertTrue(url.startsWith("http://"), "URL should include scheme: " + url); + assertTrue(url.contains("localhost"), "URL should include host: " + url); + assertTrue(url.contains("/path"), "URL should include path: " + url); + assertFalse(url.contains("secret"), "URL should not contain query params: " + url); } } From 9510ed84aceb2f6131088a023e527f56bcce42eb Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Fri, 12 Jun 2026 03:06:50 -0300 Subject: [PATCH 17/28] fix(java-agent): decouple getTelemetryTracker() from INSTANCE identity --- .../java/com/rollbar/agent/RollbarAgent.java | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java index 6377bb11..19b17855 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/RollbarAgent.java @@ -4,8 +4,12 @@ import com.rollbar.agent.instrumentation.ApacheHttpClient5Instrumentation; import com.rollbar.agent.instrumentation.HttpUrlConnectionInstrumentation; import com.rollbar.agent.instrumentation.JavaHttpClientInstrumentation; +import com.rollbar.api.payload.data.Level; +import com.rollbar.api.payload.data.Source; +import com.rollbar.api.payload.data.TelemetryEvent; import com.rollbar.notifier.telemetry.TelemetryEventTracker; import java.lang.instrument.Instrumentation; +import java.util.List; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.matcher.ElementMatchers; @@ -48,6 +52,40 @@ private static void installInstrumentation(Instrumentation inst) { } public static TelemetryEventTracker getTelemetryTracker() { - return AgentTelemetryStore.getInstance(); + return DelegatingTracker.INSTANCE; + } + + private static final class DelegatingTracker implements TelemetryEventTracker { + + static final DelegatingTracker INSTANCE = new DelegatingTracker(); + + private DelegatingTracker() {} + + @Override + public List getAll() { + return AgentTelemetryStore.getInstance().getAll(); + } + + @Override + public void recordLogEventFor(Level level, Source source, String message) { + AgentTelemetryStore.getInstance().recordLogEventFor(level, source, message); + } + + @Override + public void recordManualEventFor(Level level, Source source, String message) { + AgentTelemetryStore.getInstance().recordManualEventFor(level, source, message); + } + + @Override + public void recordNavigationEventFor(Level level, Source source, String from, String to) { + AgentTelemetryStore.getInstance().recordNavigationEventFor(level, source, from, to); + } + + @Override + public void recordNetworkEventFor( + Level level, Source source, String method, String url, String statusCode) { + AgentTelemetryStore.getInstance().recordNetworkEventFor( + level, source, method, url, statusCode); + } } } From 439a3d233eafa62402d704feafb933a41833fc15 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Fri, 12 Jun 2026 03:10:41 -0300 Subject: [PATCH 18/28] refactor(java-agent): rename AgentTelemetryStore.init() to initForTesting() --- rollbar-java-agent/README.md | 4 ++++ .../src/main/java/com/rollbar/agent/AgentTelemetryStore.java | 2 +- .../src/test/java/com/rollbar/agent/RollbarAgentTest.java | 4 ++-- .../instrumentation/ApacheHttpClient4InstrumentationTest.java | 2 +- .../instrumentation/ApacheHttpClient5InstrumentationTest.java | 2 +- .../instrumentation/HttpUrlConnectionInstrumentationTest.java | 2 +- .../instrumentation/JavaHttpClientInstrumentationTest.java | 2 +- 7 files changed, 11 insertions(+), 7 deletions(-) diff --git a/rollbar-java-agent/README.md b/rollbar-java-agent/README.md index d2702e17..98bf0ec4 100644 --- a/rollbar-java-agent/README.md +++ b/rollbar-java-agent/README.md @@ -119,6 +119,10 @@ is recorded as: https://api.example.com/charge ``` +## Internal API + +`AgentTelemetryStore.initForTesting()` is intended for use in tests only (it replaces the internal tracker instance). Do not call it in production code — use `RollbarAgent.getTelemetryTracker()` as shown above. + ## Testing ### Automated tests diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java index cd6d6503..3406cee2 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/AgentTelemetryStore.java @@ -22,7 +22,7 @@ public static List getAll() { return INSTANCE.getAll(); } - public static void init(Provider timestampProvider) { + public static void initForTesting(Provider timestampProvider) { INSTANCE = new RollbarTelemetryEventTracker(timestampProvider, 100); } } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java index cc6f82ee..b1a9366a 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/RollbarAgentTest.java @@ -14,7 +14,7 @@ public class RollbarAgentTest { @BeforeEach public void setUp() { - AgentTelemetryStore.init(System::currentTimeMillis); + AgentTelemetryStore.initForTesting(System::currentTimeMillis); } @Test @@ -27,7 +27,7 @@ public void getTelemetryTracker_returnsSingletonInstance() { @Test public void init_withCustomTimestamp_usesProvidedTimestamp() { long fixedTime = 1_000_000L; - AgentTelemetryStore.init(() -> fixedTime); + AgentTelemetryStore.initForTesting(() -> fixedTime); AgentTelemetryStore.getInstance().recordManualEventFor( com.rollbar.api.payload.data.Level.WARNING, diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java index b8f3663e..8be9532c 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java @@ -30,7 +30,7 @@ public void setUp() { server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); server.start(); client = HttpClients.createDefault(); - AgentTelemetryStore.init(System::currentTimeMillis); + AgentTelemetryStore.initForTesting(System::currentTimeMillis); NetworkEventBridge.resetRecordedForTesting(); } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java index 74f810d9..f041850e 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java @@ -30,7 +30,7 @@ public void setUp() { server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); server.start(); client = HttpClients.createDefault(); - AgentTelemetryStore.init(System::currentTimeMillis); + AgentTelemetryStore.initForTesting(System::currentTimeMillis); NetworkEventBridge.resetRecordedForTesting(); } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java index 319556ad..1d403699 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java @@ -26,7 +26,7 @@ public class HttpUrlConnectionInstrumentationTest { public void setUp() { server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); server.start(); - AgentTelemetryStore.init(System::currentTimeMillis); + AgentTelemetryStore.initForTesting(System::currentTimeMillis); NetworkEventBridge.resetRecordedForTesting(); } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java index fa5a0469..de0874d1 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/JavaHttpClientInstrumentationTest.java @@ -31,7 +31,7 @@ public void setUp() { server = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); server.start(); client = HttpClient.newHttpClient(); - AgentTelemetryStore.init(System::currentTimeMillis); + AgentTelemetryStore.initForTesting(System::currentTimeMillis); NetworkEventBridge.resetRecordedForTesting(); } From 9b106d0f2169b697b3100d574e3e6ba60acb71fc Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 29 Jun 2026 18:54:47 -0300 Subject: [PATCH 19/28] fix(java-agent): disable plain jar to prevent overwriting shaded artifact --- rollbar-java-agent/build.gradle.kts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rollbar-java-agent/build.gradle.kts b/rollbar-java-agent/build.gradle.kts index ff1119ad..7e89e822 100644 --- a/rollbar-java-agent/build.gradle.kts +++ b/rollbar-java-agent/build.gradle.kts @@ -21,6 +21,11 @@ dependencies { } tasks.jar { + enabled = false +} + +tasks.shadowJar { + archiveClassifier.set("") manifest { attributes( "Premain-Class" to "com.rollbar.agent.RollbarAgent", @@ -29,10 +34,6 @@ tasks.jar { "Can-Retransform-Classes" to "true" ) } -} - -tasks.shadowJar { - archiveClassifier.set("") relocate("net.bytebuddy", "com.rollbar.agent.shaded.bytebuddy") mergeServiceFiles() } @@ -52,5 +53,4 @@ tasks.test { // system classloader; mirrors production use where rollbar-java-agent is a Gradle/Maven dep classpath += files(agentJar) dependsOn(tasks.shadowJar) - dependsOn(tasks.jar) } From ab969f19eb1b2a0e77576f56811a48aaab4f4382 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Sun, 5 Jul 2026 19:05:53 -0300 Subject: [PATCH 20/28] fix(java-agent): always install HC4/HC5 transformers regardless of classloader --- .../ApacheHttpClient4Instrumentation.java | 20 +++++++----------- .../ApacheHttpClient5Instrumentation.java | 21 ++++++++----------- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java index e2a8782f..af2c527e 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java @@ -19,21 +19,17 @@ public final class ApacheHttpClient4Instrumentation { private ApacheHttpClient4Instrumentation() {} /** - * Instruments Apache HttpClient 4.x if present on the classpath. + * Installs a ByteBuddy transformer for Apache HttpClient 4.x. * - *

Does nothing if {@code org.apache.http.impl.client.CloseableHttpClient} is not available. + *

The transformer fires only if {@code org.apache.http.impl.client.CloseableHttpClient} is + * loaded at runtime; if HC4 is absent the registered matcher simply never matches. */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { - // Use a resource check rather than Class.forName: execute(HttpUriRequest, HttpContext) is a - // concrete method defined in CloseableHttpClient itself, so loading that class before the - // ByteBuddy transformer is installed prevents the method from ever being instrumented (no - // RedefinitionStrategy is configured). getResource checks classpath availability without - // triggering class loading. - if (ClassLoader.getSystemClassLoader().getResource( - "org/apache/http/impl/client/CloseableHttpClient.class") == null) { - return; - } - + // Always install — ByteBuddy intercepts class loading at the JVM level regardless of which + // classloader (system, app, or child) eventually loads CloseableHttpClient. If HC4 is absent + // the transformer simply never fires. We must not use Class.forName here: loading + // CloseableHttpClient before the transformer is installed prevents instrumentation because no + // RedefinitionStrategy is configured. builder .type(ElementMatchers.named("org.apache.http.impl.client.CloseableHttpClient")) .transform((b, typeDescription, classLoader, module, protectionDomain) -> diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java index 919f3bc8..3938382c 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java @@ -21,21 +21,18 @@ public final class ApacheHttpClient5Instrumentation { private ApacheHttpClient5Instrumentation() {} /** - * Instruments Apache HttpClient 5.x if present on the classpath. + * Installs a ByteBuddy transformer for Apache HttpClient 5.x. * - *

Does nothing if {@code org.apache.hc.client5.http.impl.classic.CloseableHttpClient} - * is not available. + *

The transformer fires only if + * {@code org.apache.hc.client5.http.impl.classic.CloseableHttpClient} is loaded at runtime; + * if HC5 is absent the registered matcher simply never matches. */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { - // Use a resource check rather than Class.forName: execute() overloads are concrete methods - // defined in CloseableHttpClient itself, so loading that class before the ByteBuddy transformer - // is installed prevents the methods from ever being instrumented (no RedefinitionStrategy is - // configured). getResource checks classpath availability without triggering class loading. - if (ClassLoader.getSystemClassLoader().getResource( - "org/apache/hc/client5/http/impl/classic/CloseableHttpClient.class") == null) { - return; - } - + // Always install — ByteBuddy intercepts class loading at the JVM level regardless of which + // classloader (system, app, or child) eventually loads CloseableHttpClient. If HC5 is absent + // the transformer simply never fires. We must not use Class.forName here: loading + // CloseableHttpClient before the transformer is installed prevents instrumentation because no + // RedefinitionStrategy is configured. builder .type(ElementMatchers.named("org.apache.hc.client5.http.impl.classic.CloseableHttpClient")) .transform((b, typeDescription, classLoader, module, protectionDomain) -> From efd4c208b393a1c29ce397597e160b0ff8d89ed7 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Sun, 12 Jul 2026 22:57:34 -0300 Subject: [PATCH 21/28] feat(java-agent): capture HttpURLConnection requests that skip getResponseCode --- .../HttpUrlConnectionInstrumentation.java | 85 ++++++++++++++++++- .../HttpUrlConnectionInstrumentationTest.java | 52 ++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java index 5ee10ec6..fe203c9c 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java @@ -13,9 +13,22 @@ public final class HttpUrlConnectionInstrumentation { private HttpUrlConnectionInstrumentation() {} /** - * Instruments {@code java.net.HttpURLConnection.getResponseCode()} to record 4xx/5xx responses. + * Instruments {@code HttpURLConnection} to record 4xx/5xx responses and network errors. * - *

Targets the declaring class directly to capture the method regardless of concrete subtype. + *

Three entry points are covered: + *

    + *
  • {@code getResponseCode()} on the base class — catches callers that check the code + * explicitly.
    • + *
  • {@code getInputStream()} on concrete subclasses — catches the common pattern where the + * caller reads the body directly and only sees the IOException on 4xx.
    • + *
  • {@code getErrorStream()} on concrete subclasses — catches callers that check for an error + * stream after {@code connect()} or after catching the IOException from + * {@code getInputStream()}.
    • + *
+ * + *

{@code getInputStream} and {@code getErrorStream} advice simply invoke + * {@code getResponseCode()} to trigger the base-class advice; deduplication in + * {@link com.rollbar.agent.NetworkEventBridge} ensures only one event is emitted per connection. */ public static void install(AgentBuilder builder, Instrumentation inst) { builder @@ -25,6 +38,74 @@ public static void install(AgentBuilder builder, Instrumentation inst) { .on(ElementMatchers.named("getResponseCode"))) ) .installOn(inst); + + // getInputStream() and getErrorStream() are overridden in concrete subclasses, so we must + // target subtypes rather than java.net.HttpURLConnection itself. + builder + .type(ElementMatchers.isSubTypeOf(java.net.HttpURLConnection.class) + .and(ElementMatchers.not(ElementMatchers.named("java.net.HttpURLConnection")))) + .transform((b, typeDescription, classLoader, module, protectionDomain) -> + b.visit(Advice.to(GetInputStreamAdvice.class) + .on(ElementMatchers.named("getInputStream") + .and(ElementMatchers.not(ElementMatchers.isAbstract())))) + .visit(Advice.to(GetErrorStreamAdvice.class) + .on(ElementMatchers.named("getErrorStream") + .and(ElementMatchers.not(ElementMatchers.isAbstract())))) + ) + .installOn(inst); + } + + /** + * Advice inlined into concrete {@code HttpURLConnection.getInputStream()}. + * + *

When {@code getInputStream()} throws (4xx/5xx response), invokes {@code getResponseCode()} + * so that {@link GetResponseCodeAdvice} records the event. Deduplication in + * {@link com.rollbar.agent.NetworkEventBridge} prevents double-recording if the caller also calls + * {@code getResponseCode()} or {@code getErrorStream()} afterwards. + */ + public static class GetInputStreamAdvice { + + /** Fires when {@code getInputStream()} throws, ensuring the failed request is recorded. */ + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void onExit( + @Advice.This Object connection, + @Advice.Thrown Throwable thrown + ) { + if (thrown == null) { + return; + } + try { + connection.getClass().getMethod("getResponseCode").invoke(connection); + } catch (Throwable ignored) { + // Advice must never throw + } + } + } + + /** + * Advice inlined into concrete {@code HttpURLConnection.getErrorStream()}. + * + *

A non-null return means the server sent a 4xx/5xx response. Invokes + * {@code getResponseCode()} so that {@link GetResponseCodeAdvice} records the event. + * Deduplication in {@link com.rollbar.agent.NetworkEventBridge} prevents double-recording. + */ + public static class GetErrorStreamAdvice { + + /** Fires when {@code getErrorStream()} returns a non-null stream. */ + @Advice.OnMethodExit + public static void onExit( + @Advice.This Object connection, + @Advice.Return Object errorStream + ) { + if (errorStream == null) { + return; + } + try { + connection.getClass().getMethod("getResponseCode").invoke(connection); + } catch (Throwable ignored) { + // Advice must never throw + } + } } /** diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java index 1d403699..c7de6cba 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java @@ -104,6 +104,58 @@ public void urlSanitization_stripsQueryAndCredentials() throws IOException { assertFalse(url.contains("token")); } + @Test + public void getInputStream_on4xx_recordsNetworkEvent() throws IOException { + server.stubFor(get(urlEqualTo("/not-found")).willReturn(aResponse().withStatus(404))); + + HttpURLConnection conn = (HttpURLConnection) new URL(server.baseUrl() + "/not-found") + .openConnection(); + conn.setRequestMethod("GET"); + try { + conn.getInputStream(); + } catch (IOException ignored) { + // expected for 4xx + } + conn.disconnect(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("404", body.get("status_code")); + assertEquals("GET", body.get("method")); + } + + @Test + public void getInputStream_on2xx_doesNotRecordEvent() throws IOException { + server.stubFor(get(urlEqualTo("/ok")).willReturn(aResponse().withStatus(200))); + + HttpURLConnection conn = (HttpURLConnection) new URL(server.baseUrl() + "/ok") + .openConnection(); + conn.setRequestMethod("GET"); + conn.getInputStream().close(); + conn.disconnect(); + + assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + } + + @Test + public void getInputStream_thenGetErrorStream_doesNotDoubleRecord() throws IOException { + server.stubFor(get(urlEqualTo("/not-found")).willReturn(aResponse().withStatus(404))); + + HttpURLConnection conn = (HttpURLConnection) new URL(server.baseUrl() + "/not-found") + .openConnection(); + conn.setRequestMethod("GET"); + try { + conn.getInputStream(); + } catch (IOException ignored) { + // expected for 4xx + } + conn.getErrorStream(); + conn.disconnect(); + + assertEquals(1, AgentTelemetryStore.getInstance().getAll().size()); + } + private void makeRequest(String method, String path) throws IOException { HttpURLConnection conn = (HttpURLConnection) new URL(server.baseUrl() + path).openConnection(); conn.setRequestMethod(method); From b46aa7b5349048ce214b59d859e6aa041e9aacee Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Sun, 12 Jul 2026 23:08:58 -0300 Subject: [PATCH 22/28] fix(java-agent): strip query and userinfo in UrlSanitizer fallback path --- .../java/com/rollbar/agent/UrlSanitizer.java | 28 ++++++++- .../com/rollbar/agent/UrlSanitizerTest.java | 59 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 rollbar-java-agent/src/test/java/com/rollbar/agent/UrlSanitizerTest.java diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java index d11ae48d..b162e93c 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java @@ -27,7 +27,33 @@ public static String sanitize(String rawUrl) { null ).toString(); } catch (URISyntaxException e) { - return rawUrl; + return fallbackSanitize(rawUrl); } } + + // URI rejected the URL (e.g. unescaped space, bad percent-escape). Strip query, fragment, and + // userinfo with plain string ops rather than failing open with the raw URL. + private static String fallbackSanitize(String rawUrl) { + // Drop query string and fragment — take everything before the first '?' or '#'. + int end = rawUrl.length(); + int query = rawUrl.indexOf('?'); + int header = rawUrl.indexOf('#'); + if (query >= 0) { + end = query; + } + if (header >= 0) { + end = Math.min(end, header); + } + String result = rawUrl.substring(0, end); + + // Drop userinfo: scheme://user:pass@host/path → scheme://host/path + int schemeEnd = result.indexOf("://"); + if (schemeEnd >= 0) { + int atSign = result.indexOf('@', schemeEnd + 3); + if (atSign >= 0) { + result = result.substring(0, schemeEnd + 3) + result.substring(atSign + 1); + } + } + return result; + } } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/UrlSanitizerTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/UrlSanitizerTest.java new file mode 100644 index 00000000..99b5b2da --- /dev/null +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/UrlSanitizerTest.java @@ -0,0 +1,59 @@ +package com.rollbar.agent; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class UrlSanitizerTest { + + @Test + public void normalUrl_stripsQueryAndFragment() { + assertEquals( + "https://example.com/path", + UrlSanitizer.sanitize("https://example.com/path?token=secret&key=value") + ); + } + + @Test + public void normalUrl_stripsUserinfo() { + assertEquals( + "https://example.com/path", + UrlSanitizer.sanitize("https://user:pass@example.com/path") + ); + } + + @Test + public void normalUrl_stripsFragment() { + assertEquals( + "https://example.com/path", + UrlSanitizer.sanitize("https://example.com/path#section") + ); + } + + @Test + public void urlWithUnescapedSpace_fallback_stripsQuery() { + // URI rejects unescaped spaces; fallback must still strip the query string. + String result = UrlSanitizer.sanitize("http://example.com/path with spaces?token=secret"); + assertFalse(result.contains("token"), "query must be stripped even on parse failure"); + assertFalse(result.contains("secret"), "secret value must be stripped even on parse failure"); + assertTrue(result.contains("example.com"), "host should be preserved"); + } + + @Test + public void urlWithUnescapedSpace_fallback_stripsUserinfo() { + String result = UrlSanitizer.sanitize("http://user:pass@example.com/path with spaces"); + assertFalse(result.contains("pass"), "userinfo must be stripped even on parse failure"); + assertTrue(result.contains("example.com"), "host should be preserved"); + } + + @Test + public void urlWithBadPercentEncoding_fallback_stripsQuery() { + String result = UrlSanitizer.sanitize("http://example.com/path%zz?secret=abc"); + assertFalse(result.contains("secret"), "query must be stripped even on parse failure"); + } + + @Test + public void nullUrl_returnsNull() { + assertNull(UrlSanitizer.sanitize(null)); + } +} From 7aadaaaa502939059bbaf5246cea04cc377decf0 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 13 Jul 2026 00:02:38 -0300 Subject: [PATCH 23/28] fix(java-agent): make shadowJar the sole artifact, prevent thin jar from overwriting it --- rollbar-java-agent/build.gradle.kts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rollbar-java-agent/build.gradle.kts b/rollbar-java-agent/build.gradle.kts index 7e89e822..bee58efe 100644 --- a/rollbar-java-agent/build.gradle.kts +++ b/rollbar-java-agent/build.gradle.kts @@ -24,6 +24,15 @@ tasks.jar { enabled = false } +// java-library wires tasks.jar into apiElements/runtimeElements; replace it with shadowJar so +// Gradle's variant system and vanniktech publishing both see the fat jar as the primary artifact. +listOf(configurations.apiElements, configurations.runtimeElements).forEach { cfg -> + cfg.configure { + outgoing.artifacts.clear() + outgoing.artifact(tasks.shadowJar) + } +} + tasks.shadowJar { archiveClassifier.set("") manifest { From 97c3ac3b2646e1093535357f52d1977ca88387a5 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 13 Jul 2026 01:02:16 -0300 Subject: [PATCH 24/28] fix(java-agent): preserve host in UrlSanitizer for underscore hostnames and @-in-path URLs --- .../java/com/rollbar/agent/UrlSanitizer.java | 41 +++++++++++++------ .../com/rollbar/agent/UrlSanitizerTest.java | 26 ++++++++++++ 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java index b162e93c..d5a793d5 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/UrlSanitizer.java @@ -17,15 +17,18 @@ public static String sanitize(String rawUrl) { } try { URI uri = new URI(rawUrl); - return new URI( - uri.getScheme(), - null, - uri.getHost(), - uri.getPort(), - uri.getPath(), - null, - null - ).toString(); + // Use getAuthority() rather than getHost(): URI parses authorities that fail RFC 2396 + // server-based grammar (e.g. underscores in Kubernetes/AD internal DNS) in registry-based + // mode, where getHost() returns null and the host would be silently dropped. Strip userinfo + // from the authority manually. + String authority = uri.getAuthority(); + if (authority != null) { + int at = authority.indexOf('@'); + if (at >= 0) { + authority = authority.substring(at + 1); + } + } + return new URI(uri.getScheme(), authority, uri.getPath(), null, null).toString(); } catch (URISyntaxException e) { return fallbackSanitize(rawUrl); } @@ -46,12 +49,24 @@ private static String fallbackSanitize(String rawUrl) { } String result = rawUrl.substring(0, end); - // Drop userinfo: scheme://user:pass@host/path → scheme://host/path + // Drop userinfo: scheme://user:pass@host/path → scheme://host/path. Bound the '@' search to + // the authority component (before the first '/', '?', or '#') so an '@' inside the path — e.g. + // /@handle or /@scope/pkg — is not mistaken for the userinfo separator, which would delete the + // real host and promote a path segment to host. int schemeEnd = result.indexOf("://"); if (schemeEnd >= 0) { - int atSign = result.indexOf('@', schemeEnd + 3); - if (atSign >= 0) { - result = result.substring(0, schemeEnd + 3) + result.substring(atSign + 1); + int authorityStart = schemeEnd + 3; + int authorityEnd = result.length(); + for (int i = authorityStart; i < result.length(); i++) { + char c = result.charAt(i); + if (c == '/' || c == '?' || c == '#') { + authorityEnd = i; + break; + } + } + int atSign = result.indexOf('@', authorityStart); + if (atSign >= 0 && atSign < authorityEnd) { + result = result.substring(0, authorityStart) + result.substring(atSign + 1); } } return result; diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/UrlSanitizerTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/UrlSanitizerTest.java index 99b5b2da..2d656312 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/UrlSanitizerTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/UrlSanitizerTest.java @@ -52,6 +52,32 @@ public void urlWithBadPercentEncoding_fallback_stripsQuery() { assertFalse(result.contains("secret"), "query must be stripped even on parse failure"); } + @Test + public void underscoreHostname_preservesHost() { + // Underscore hostnames (Kubernetes service DNS, Windows/AD internal DNS) parse in URI's + // registry-based mode, where getHost() returns null. The host must not be dropped. + assertEquals( + "http://s3_bucket.example.com/path", + UrlSanitizer.sanitize("http://s3_bucket.example.com/path?token=secret") + ); + } + + @Test + public void underscoreHostname_withUserinfoAndPort_stripsUserinfoKeepsHost() { + assertEquals( + "http://my_svc.internal:8080/v1", + UrlSanitizer.sanitize("http://user:pass@my_svc.internal:8080/v1?x=1") + ); + } + + @Test + public void atSignInPath_fallback_doesNotPromotePathToHost() { + // Unescaped space forces the fallback; the '@' inside the path must not be mistaken for the + // userinfo separator (which would delete the host and promote 'johndoe' to host). + String result = UrlSanitizer.sanitize("http://example.com/@johndoe/messages?bad space"); + assertEquals("http://example.com/@johndoe/messages", result); + } + @Test public void nullUrl_returnsNull() { assertNull(UrlSanitizer.sanitize(null)); From ac57d625d10ca22b1347bd43a390306d08ebdf5e Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 13 Jul 2026 01:13:28 -0300 Subject: [PATCH 25/28] fix(java-agent): prevent getInputStream advice recursion on connection-level failures --- .../com/rollbar/agent/NetworkEventBridge.java | 28 ++++++++++++++++ .../HttpUrlConnectionInstrumentation.java | 19 ++++++++++- .../HttpUrlConnectionInstrumentationTest.java | 33 +++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java index c3a2bcac..75f307e1 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java @@ -23,10 +23,38 @@ public final class NetworkEventBridge { Collections.synchronizedMap(new WeakHashMap<>()) ); + // Re-entry guard for the getInputStream/getErrorStream advice: invoking getResponseCode() to + // trigger recording can, on connection-level failures (responseCode stays -1), cause the JDK's + // getResponseCode() to call getInputStream() again — re-firing the advice and recursing until a + // StackOverflowError. This ThreadLocal breaks that loop. + private static final ThreadLocal TRIGGERING_RESPONSE_CODE = + ThreadLocal.withInitial(() -> Boolean.FALSE); + private NetworkEventBridge() {} public static void resetRecordedForTesting() { RECORDED.clear(); + TRIGGERING_RESPONSE_CODE.remove(); + } + + /** + * Returns {@code true} if the caller may proceed to trigger {@code getResponseCode()}; + * {@code false} if a trigger is already in progress on this thread (re-entrant call). + * + *

The caller that receives {@code true} must call {@link #exitResponseCodeTrigger()} in a + * {@code finally} block. A re-entrant caller receives {@code false} and must not call exit. + */ + public static boolean enterResponseCodeTrigger() { + if (TRIGGERING_RESPONSE_CODE.get()) { + return false; + } + TRIGGERING_RESPONSE_CODE.set(Boolean.TRUE); + return true; + } + + /** Clears the re-entry guard set by {@link #enterResponseCodeTrigger()}. */ + public static void exitResponseCodeTrigger() { + TRIGGERING_RESPONSE_CODE.remove(); } /** diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java index fe203c9c..1102cfb9 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java @@ -75,7 +75,24 @@ public static void onExit( return; } try { - connection.getClass().getMethod("getResponseCode").invoke(connection); + ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + if (classLoader == null) { + classLoader = ClassLoader.getSystemClassLoader(); + } + Class bridge = classLoader.loadClass("com.rollbar.agent.NetworkEventBridge"); + + // Re-entry guard: on connection-level failures (responseCode == -1) the JDK's + // getResponseCode() calls getInputStream() again, which re-fires this advice. Without the + // guard that recurses until a StackOverflowError, recording nothing. + Boolean entered = (Boolean) bridge.getMethod("enterResponseCodeTrigger").invoke(null); + if (entered == null || !entered) { + return; + } + try { + connection.getClass().getMethod("getResponseCode").invoke(connection); + } finally { + bridge.getMethod("exitResponseCodeTrigger").invoke(null); + } } catch (Throwable ignored) { // Advice must never throw } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java index c7de6cba..7254b872 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentationTest.java @@ -11,6 +11,7 @@ import java.io.IOException; import java.net.HttpURLConnection; +import java.net.ServerSocket; import java.net.URL; import java.util.List; import java.util.Map; @@ -156,6 +157,38 @@ public void getInputStream_thenGetErrorStream_doesNotDoubleRecord() throws IOExc assertEquals(1, AgentTelemetryStore.getInstance().getAll().size()); } + @Test + public void getInputStream_connectionRefused_recordsErrorWithoutRecursion() throws IOException { + // A connection-level failure leaves responseCode == -1, so the JDK's getResponseCode() calls + // getInputStream() again. Before the re-entry guard this recursed until a StackOverflowError + // that the advice swallowed, recording nothing. Now it must record a single error event and + // return promptly. + int closedPort; + try (ServerSocket socket = new ServerSocket(0)) { + closedPort = socket.getLocalPort(); + } // socket closed here → connections to closedPort are refused + + HttpURLConnection conn = (HttpURLConnection) new URL( + "http://127.0.0.1:" + closedPort + "/x").openConnection(); + conn.setConnectTimeout(1000); + conn.setReadTimeout(1000); + try { + conn.getInputStream(); + fail("expected connection to be refused"); + } catch (IOException expected) { + // expected: nothing is listening on the port + } + conn.disconnect(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size(), "connection failure should record exactly one event"); + Map json = events.get(0).asJson(); + assertEquals("manual", json.get("type")); + Map body = (Map) json.get("body"); + assertTrue(body.get("message").toString().contains("Network error"), + "error event should carry a network-error message"); + } + private void makeRequest(String method, String path) throws IOException { HttpURLConnection conn = (HttpURLConnection) new URL(server.baseUrl() + path).openConnection(); conn.setRequestMethod(method); From af9cdee4d1ceb9fe6b94b69db60ac140a1ee6ab7 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 13 Jul 2026 01:17:15 -0300 Subject: [PATCH 26/28] fix: lint --- .../main/java/com/rollbar/agent/NetworkEventBridge.java | 4 +++- .../instrumentation/HttpUrlConnectionInstrumentation.java | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java index 75f307e1..d018e35d 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java @@ -52,7 +52,9 @@ public static boolean enterResponseCodeTrigger() { return true; } - /** Clears the re-entry guard set by {@link #enterResponseCodeTrigger()}. */ + /** + * Clears the re-entry guard set by {@link #enterResponseCodeTrigger()}. + */ public static void exitResponseCodeTrigger() { TRIGGERING_RESPONSE_CODE.remove(); } diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java index 1102cfb9..41316bc6 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/HttpUrlConnectionInstrumentation.java @@ -65,7 +65,9 @@ public static void install(AgentBuilder builder, Instrumentation inst) { */ public static class GetInputStreamAdvice { - /** Fires when {@code getInputStream()} throws, ensuring the failed request is recorded. */ + /** + * Fires when {@code getInputStream()} throws, ensuring the failed request is recorded. + */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( @Advice.This Object connection, @@ -108,7 +110,9 @@ public static void onExit( */ public static class GetErrorStreamAdvice { - /** Fires when {@code getErrorStream()} returns a non-null stream. */ + /** + * Fires when {@code getErrorStream()} returns a non-null stream. + */ @Advice.OnMethodExit public static void onExit( @Advice.This Object connection, From 69fc32a924e1a4a3572e788d111d553b03fb1a74 Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 13 Jul 2026 01:56:58 -0300 Subject: [PATCH 27/28] chore: update readme --- rollbar-java-agent/README.md | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/rollbar-java-agent/README.md b/rollbar-java-agent/README.md index 98bf0ec4..f3f4efe6 100644 --- a/rollbar-java-agent/README.md +++ b/rollbar-java-agent/README.md @@ -13,10 +13,22 @@ It works by attaching to the JVM at startup via `-javaagent:` and using ByteBudd | Apache HttpClient 4.x (`org.apache.http`) | If present on classpath | | Apache HttpClient 5.x (`org.apache.hc.client5`) | If present on classpath | -Only 4xx and 5xx responses are recorded. Successful requests (< 400) produce no telemetry. +Only 4xx and 5xx responses are recorded, along with requests that fail before a response arrives (connection refused, DNS failure, timeout). Successful requests (< 400) produce no telemetry. > **Apache HC4/HC5 note:** `execute(request, responseHandler)` overloads route through a separate internal call chain and are not instrumented. Use the standard `execute(request)` / `execute(request, context)` overloads to get telemetry. +### HttpURLConnection entry points + +`HttpURLConnection` is captured through three entry points, so a failed request is recorded regardless of how your code consumes the response: + +| Entry point | Why it is covered | +|-------------|-------------------| +| `getResponseCode()` | The caller checks the status code explicitly. | +| `getInputStream()` | The caller reads the body directly and only ever sees the `IOException` that a 4xx/5xx throws. | +| `getErrorStream()` | The caller inspects the error stream after `connect()`, or after catching the `IOException` from `getInputStream()`. | + +Exactly one event is recorded per connection, even when your code hits several of these entry points (for example `getInputStream()` throwing and then `getErrorStream()` being read) — the agent deduplicates on the connection instance. + ## Requirements - Java 11 or higher @@ -36,6 +48,8 @@ The fat JAR (with ByteBuddy bundled and relocated) is written to: rollbar-java-agent/build/libs/rollbar-java-agent-.jar ``` +This fat JAR is the module's only artifact — the thin `jar` task is disabled, and the shaded JAR is what Gradle consumers and the published Maven artifact resolve to. So the JAR you pass to `-javaagent:` and the JAR you put on the classpath (steps 2 and 3) are always the same file. + ### 2. Add the agent JVM flag Add `-javaagent:` to your JVM startup arguments, pointing at the JAR built above: @@ -103,9 +117,12 @@ That's it. All HTTP calls your application makes from that point on will automat |----------|--------| | Response status `< 400` | No telemetry recorded | | Response status `>= 400` | Records a network telemetry event with `Level.CRITICAL` | -| Connection failure / I/O error | Records an error telemetry event | +| Connection failure / I/O error (connection refused, DNS failure, timeout) | Records a `Network error: ` telemetry event with `Level.CRITICAL` | +| The same request seen through several entry points | Deduplicated — one event per request | | No Rollbar config wired | Events accumulate in the agent store (capacity 100); nothing is sent | +The agent never throws into your application: every advice body swallows all errors, so a failure inside the instrumentation cannot break an HTTP call. + ## Security URLs can carry sensitive data in query parameters or basic-auth credentials. The agent **strips userinfo, query parameters, and the URL fragment** before recording. @@ -121,7 +138,10 @@ https://api.example.com/charge ## Internal API -`AgentTelemetryStore.initForTesting()` is intended for use in tests only (it replaces the internal tracker instance). Do not call it in production code — use `RollbarAgent.getTelemetryTracker()` as shown above. +Two methods exist for tests only. Do not call them in production code — use `RollbarAgent.getTelemetryTracker()` as shown above. + +- `AgentTelemetryStore.initForTesting(Provider timestampProvider)` — replaces the internal tracker with one backed by the given timestamp provider, so tests can assert on event timestamps. +- `NetworkEventBridge.resetRecordedForTesting()` — clears the deduplication state, so events from a previous test do not suppress recording in the next one. ## Testing From cbab9d5943f9202aae9ca9c41755c05ef2e9c68b Mon Sep 17 00:00:00 2001 From: buongarzoni Date: Mon, 13 Jul 2026 02:21:12 -0300 Subject: [PATCH 28/28] fix(java-agent): instrument Apache HC doExecute() to cover all execute() overloads --- rollbar-java-agent/README.md | 2 +- .../com/rollbar/agent/NetworkEventBridge.java | 31 ++++++ .../ApacheHttpClient4Instrumentation.java | 100 +++++++++-------- .../ApacheHttpClient5Instrumentation.java | 104 ++++++++++-------- .../rollbar/agent/NetworkEventBridgeTest.java | 51 +++++++++ .../ApacheHttpClient4InstrumentationTest.java | 47 +++++++- .../ApacheHttpClient5InstrumentationTest.java | 50 ++++++++- 7 files changed, 285 insertions(+), 100 deletions(-) create mode 100644 rollbar-java-agent/src/test/java/com/rollbar/agent/NetworkEventBridgeTest.java diff --git a/rollbar-java-agent/README.md b/rollbar-java-agent/README.md index f3f4efe6..9948d587 100644 --- a/rollbar-java-agent/README.md +++ b/rollbar-java-agent/README.md @@ -15,7 +15,7 @@ It works by attaching to the JVM at startup via `-javaagent:` and using ByteBudd Only 4xx and 5xx responses are recorded, along with requests that fail before a response arrives (connection refused, DNS failure, timeout). Successful requests (< 400) produce no telemetry. -> **Apache HC4/HC5 note:** `execute(request, responseHandler)` overloads route through a separate internal call chain and are not instrumented. Use the standard `execute(request)` / `execute(request, context)` overloads to get telemetry. +**Apache HC4/HC5:** every `execute(...)` overload is covered — the request-only forms, the target-host forms (`execute(HttpHost, request)`), and the response-handler forms. The agent instruments the protected `doExecute(HttpHost, request, context)` method that all of them converge on, rather than any individual `execute()` overload, so no dispatch path is missed. Requests issued through a target-host overload carry only a path, so the agent rejoins the host from the `HttpHost` argument to record a complete URL. ### HttpURLConnection entry points diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java index d018e35d..2023f587 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/NetworkEventBridge.java @@ -123,6 +123,37 @@ public static java.util.function.BiConsumer createAsyncCallba }; } + /** + * Joins a base URI with a request URI, for clients that dispatch a target host separately from a + * request whose URI may be relative. + * + *

Apache HC's {@code doExecute(HttpHost, request, context)} receives the target host as its + * own argument, so a request issued through the host-based {@code execute(HttpHost, request)} + * overloads carries only a path (e.g. {@code /charge}). Rejoining the two is what keeps the host + * in the recorded URL. A request URI that is already absolute is returned untouched, and a null + * base (HC leaves the target null for a relative URI it could not resolve) degrades to the path + * alone. + * + * @param baseUri the target host as a URI (e.g. {@code https://api.example.com}), or null + * @param requestUri the request URI, absolute or relative, or null + * @return the joined URL — never null, so the caller always has something to sanitize + */ + public static String composeUrl(String baseUri, String requestUri) { + if (requestUri == null || requestUri.isEmpty()) { + return baseUri != null ? baseUri : ""; + } + if (requestUri.contains("://")) { + return requestUri; + } + if (baseUri == null) { + return requestUri; + } + if (requestUri.startsWith("/")) { + return baseUri.concat(requestUri); + } + return baseUri.concat("/").concat(requestUri); + } + /** * Records a manual error telemetry event with the given message. * diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java index af2c527e..6a6207b1 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient4Instrumentation.java @@ -1,15 +1,13 @@ package com.rollbar.agent.instrumentation; -import com.rollbar.agent.AgentTelemetryStore; -import com.rollbar.agent.UrlSanitizer; -import com.rollbar.api.payload.data.Level; -import com.rollbar.api.payload.data.Source; +import com.rollbar.agent.NetworkEventBridge; import java.lang.instrument.Instrumentation; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatchers; +import org.apache.http.HttpHost; +import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; -import org.apache.http.client.methods.HttpUriRequest; /** * Installs ByteBuddy advice on Apache HttpClient 4.x to capture network errors. @@ -21,33 +19,44 @@ private ApacheHttpClient4Instrumentation() {} /** * Installs a ByteBuddy transformer for Apache HttpClient 4.x. * - *

The transformer fires only if {@code org.apache.http.impl.client.CloseableHttpClient} is - * loaded at runtime; if HC4 is absent the registered matcher simply never matches. + *

The transformer fires only if a subtype of + * {@code org.apache.http.impl.client.CloseableHttpClient} is loaded at runtime; if HC4 is absent + * the registered matcher simply never matches. */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { // Always install — ByteBuddy intercepts class loading at the JVM level regardless of which - // classloader (system, app, or child) eventually loads CloseableHttpClient. If HC4 is absent - // the transformer simply never fires. We must not use Class.forName here: loading + // classloader (system, app, or child) eventually loads the client. If HC4 is absent the + // transformer simply never fires. We must not use Class.forName here: loading // CloseableHttpClient before the transformer is installed prevents instrumentation because no // RedefinitionStrategy is configured. builder - .type(ElementMatchers.named("org.apache.http.impl.client.CloseableHttpClient")) + // doExecute() is declared abstract on CloseableHttpClient and implemented by its subclasses + // (InternalHttpClient, MinimalHttpClient, third-party wrappers), so we match subtypes. + // The name filters keep the (relatively costly) hierarchy walk off the JDK classes that + // reach this matcher because RollbarAgent un-ignores java.* for the JDK HTTP clients. + .type(ElementMatchers.not(ElementMatchers.nameStartsWith("java.")) + .and(ElementMatchers.not(ElementMatchers.nameStartsWith("javax."))) + .and(ElementMatchers.not(ElementMatchers.nameStartsWith("jdk."))) + .and(ElementMatchers.not(ElementMatchers.nameStartsWith("sun."))) + .and(ElementMatchers.hasSuperType( + ElementMatchers.named("org.apache.http.impl.client.CloseableHttpClient")))) .transform((b, typeDescription, classLoader, module, protectionDomain) -> - b.visit(Advice.to(ExecuteAdvice.class) - .on(ElementMatchers.named("execute") - // Target only execute(HttpUriRequest, HttpContext) — the single concrete - // method that all other execute() overloads delegate to before calling - // doExecute(). This ensures exactly one advice invocation per HTTP request - // regardless of which public overload the caller uses (including - // ResponseHandler - // variants). The bridge method for this overload is excluded to avoid a second - // firing; the 1 arg execute(HttpUriRequest) is excluded because it has no arg - // at index 1, and ResponseHandler overloads are excluded because their second - // arg is ResponseHandler, not HttpContext. + b.visit(Advice.to(DoExecuteAdvice.class) + // Target doExecute(HttpHost, HttpRequest, HttpContext) — the real convergence point + // of every dispatch path. Verified against httpclient-4.5.14 bytecode: the + // HttpUriRequest overloads reach it via determineTarget(), the HttpHost overloads + // invoke it directly, and the ResponseHandler overloads route through + // execute(HttpHost, HttpRequest, HttpContext). Instrumenting any public execute() + // overload instead would miss the paths that bypass it. Exactly one advice + // invocation per request, whichever overload the caller used. + .on(ElementMatchers.named("doExecute") + .and(ElementMatchers.not(ElementMatchers.isAbstract())) .and(ElementMatchers.not(ElementMatchers.isBridge())) .and(ElementMatchers.takesArgument(0, - ElementMatchers.named("org.apache.http.client.methods.HttpUriRequest"))) + ElementMatchers.named("org.apache.http.HttpHost"))) .and(ElementMatchers.takesArgument(1, + ElementMatchers.named("org.apache.http.HttpRequest"))) + .and(ElementMatchers.takesArgument(2, ElementMatchers.named("org.apache.http.protocol.HttpContext"))))) ) .installOn(inst); @@ -60,43 +69,48 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst *

String concatenation in the advice body must use {@link String#concat} or * {@link StringBuilder} rather than the {@code +} operator. Apache HC 4.x jars are compiled at * class-file version 50 (Java 6); the Java 9+ compiler emits {@code invokedynamic} for {@code +} - * concatenation, which ByteBuddy cannot inline into a Java 6 class file. + * concatenation, which ByteBuddy cannot inline into a Java 6 class file. Delegating to + * {@link NetworkEventBridge} keeps concatenation out of the inlined code entirely. */ - public static class ExecuteAdvice { + public static class DoExecuteAdvice { /** - * Fires after {@code execute(HttpUriRequest, HttpContext)} returns or throws, recording - * 4xx/5xx responses as telemetry. + * Fires after {@code doExecute(HttpHost, HttpRequest, HttpContext)} returns or throws, + * recording 4xx/5xx responses as telemetry. + * + *

The response (or the thrown exception) is the deduplication key, so a client that wraps + * another {@code CloseableHttpClient} — where the outer and inner {@code doExecute} both fire + * for one request — still records a single event. */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( - @Advice.Argument(0) HttpUriRequest request, + @Advice.Argument(0) HttpHost target, + @Advice.Argument(1) HttpRequest request, @Advice.Return HttpResponse response, @Advice.Thrown Throwable thrown ) { try { if (thrown != null) { - String message = thrown.getMessage() != null - ? thrown.getMessage() : thrown.getClass().getName(); - // Use String.concat instead of + to avoid invokedynamic - // (unsupported in Java 6 class files) - AgentTelemetryStore.getInstance().recordManualEventFor( - Level.CRITICAL, - Source.SERVER, - "Network error: ".concat(message) - ); + if (NetworkEventBridge.markAsRecorded(thrown)) { + String message = thrown.getMessage() != null + ? thrown.getMessage() : thrown.getClass().getName(); + NetworkEventBridge.recordError(message); + } return; } - if (response != null) { + if (response != null && request != null) { int statusCode = response.getStatusLine().getStatusCode(); if (statusCode >= 400) { - String url = request.getURI() != null ? request.getURI().toString() : ""; - AgentTelemetryStore.getInstance().recordNetworkEventFor( - Level.CRITICAL, - Source.SERVER, - request.getMethod(), - UrlSanitizer.sanitize(url), + // The host-based overloads carry the target separately from a request whose URI may be + // just a path, so rejoin the two rather than reading the request URI alone. + String base = target != null ? target.toURI() : null; + String requestUri = request.getRequestLine() != null + ? request.getRequestLine().getUri() : null; + NetworkEventBridge.recordNetworkEvent( + response, + request.getRequestLine().getMethod(), + NetworkEventBridge.composeUrl(base, requestUri), String.valueOf(statusCode) ); } diff --git a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java index 3938382c..8f21ceac 100644 --- a/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java +++ b/rollbar-java-agent/src/main/java/com/rollbar/agent/instrumentation/ApacheHttpClient5Instrumentation.java @@ -1,17 +1,16 @@ package com.rollbar.agent.instrumentation; -import com.rollbar.agent.AgentTelemetryStore; -import com.rollbar.agent.UrlSanitizer; -import com.rollbar.api.payload.data.Level; -import com.rollbar.api.payload.data.Source; +import com.rollbar.agent.NetworkEventBridge; import java.lang.instrument.Instrumentation; import java.net.URI; +import java.net.URISyntaxException; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatchers; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.HttpHost; /** * Installs ByteBuddy advice on Apache HttpClient 5.x to capture network errors. @@ -23,38 +22,47 @@ private ApacheHttpClient5Instrumentation() {} /** * Installs a ByteBuddy transformer for Apache HttpClient 5.x. * - *

The transformer fires only if - * {@code org.apache.hc.client5.http.impl.classic.CloseableHttpClient} is loaded at runtime; - * if HC5 is absent the registered matcher simply never matches. + *

The transformer fires only if a subtype of + * {@code org.apache.hc.client5.http.impl.classic.CloseableHttpClient} is loaded at runtime; if + * HC5 is absent the registered matcher simply never matches. */ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst) { // Always install — ByteBuddy intercepts class loading at the JVM level regardless of which - // classloader (system, app, or child) eventually loads CloseableHttpClient. If HC5 is absent - // the transformer simply never fires. We must not use Class.forName here: loading + // classloader (system, app, or child) eventually loads the client. If HC5 is absent the + // transformer simply never fires. We must not use Class.forName here: loading // CloseableHttpClient before the transformer is installed prevents instrumentation because no // RedefinitionStrategy is configured. builder - .type(ElementMatchers.named("org.apache.hc.client5.http.impl.classic.CloseableHttpClient")) + // doExecute() is declared abstract on CloseableHttpClient and implemented by its subclasses + // (InternalHttpClient, MinimalHttpClient, third-party wrappers), so we match subtypes. + // The name filters keep the (relatively costly) hierarchy walk off the JDK classes that + // reach this matcher because RollbarAgent un-ignores java.* for the JDK HTTP clients. + .type(ElementMatchers.not(ElementMatchers.nameStartsWith("java.")) + .and(ElementMatchers.not(ElementMatchers.nameStartsWith("javax."))) + .and(ElementMatchers.not(ElementMatchers.nameStartsWith("jdk."))) + .and(ElementMatchers.not(ElementMatchers.nameStartsWith("sun."))) + .and(ElementMatchers.hasSuperType(ElementMatchers.named( + "org.apache.hc.client5.http.impl.classic.CloseableHttpClient")))) .transform((b, typeDescription, classLoader, module, protectionDomain) -> - b.visit(Advice.to(ExecuteAdvice.class) - .on(ElementMatchers.named("execute") - // Exclude bridge methods generated for covariant return-type overrides — they - // return HttpResponse (supertype), which @Advice.Return ClassicHttpResponse - // cannot safely bind via downcast. + b.visit(Advice.to(DoExecuteAdvice.class) + // Target doExecute(HttpHost, ClassicHttpRequest, HttpContext) — the real + // convergence point of every dispatch path. Verified against httpclient5-5.3.1 + // bytecode: the request-only overloads reach it via determineTarget(), the HttpHost + // overloads invoke it directly, and the HttpClientResponseHandler overloads route + // through execute(HttpHost, ClassicHttpRequest, HttpContext, handler), which calls + // it too. Instrumenting any public execute() overload instead would miss the paths + // that bypass it, and the handler overloads erase their return type to Object, + // which cannot bind to @Advice.Return. doExecute() has a single concrete signature, + // so both problems disappear. + .on(ElementMatchers.named("doExecute") + .and(ElementMatchers.not(ElementMatchers.isAbstract())) .and(ElementMatchers.not(ElementMatchers.isBridge())) .and(ElementMatchers.takesArgument(0, + ElementMatchers.named("org.apache.hc.core5.http.HttpHost"))) + .and(ElementMatchers.takesArgument(1, ElementMatchers.named("org.apache.hc.core5.http.ClassicHttpRequest"))) - // Exclude HttpClientResponseHandler overloads: they return generic T erased to - // Object, which cannot be bound to @Advice.Return ClassicHttpResponse and would - // cause ByteBuddy to fail the transformation for the whole CloseableHttpClient. - // HC5 has both 2-arg (arg1=handler) and 3-arg (arg1=context, arg2=handler) - // variants; both are excluded here. - .and(ElementMatchers.not(ElementMatchers.takesArgument(1, - ElementMatchers.named( - "org.apache.hc.core5.http.io.HttpClientResponseHandler")))) - .and(ElementMatchers.not(ElementMatchers.takesArgument(2, - ElementMatchers.named( - "org.apache.hc.core5.http.io.HttpClientResponseHandler")))))) + .and(ElementMatchers.takesArgument(2, + ElementMatchers.named("org.apache.hc.core5.http.protocol.HttpContext"))))) ) .installOn(inst); } @@ -63,44 +71,50 @@ public static void installIfAvailable(AgentBuilder builder, Instrumentation inst * Apache HC 5.x runs in the application classloader, so we can reference Rollbar classes * directly without the TCCL reflection bridge. */ - public static class ExecuteAdvice { + public static class DoExecuteAdvice { /** - * Fires after {@code execute()} returns or throws, recording 4xx/5xx responses as telemetry. + * Fires after {@code doExecute(HttpHost, ClassicHttpRequest, HttpContext)} returns or throws, + * recording 4xx/5xx responses as telemetry. + * + *

The response (or the thrown exception) is the deduplication key, so a client that wraps + * another {@code CloseableHttpClient} — where the outer and inner {@code doExecute} both fire + * for one request — still records a single event. */ @Advice.OnMethodExit(onThrowable = Throwable.class) public static void onExit( - @Advice.Argument(0) ClassicHttpRequest request, + @Advice.Argument(0) HttpHost target, + @Advice.Argument(1) ClassicHttpRequest request, @Advice.Return ClassicHttpResponse response, @Advice.Thrown Throwable thrown ) { try { if (thrown != null) { - String message = thrown.getMessage() != null - ? thrown.getMessage() : thrown.getClass().getName(); - AgentTelemetryStore.getInstance().recordManualEventFor( - Level.CRITICAL, - Source.SERVER, - "Network error: " + message - ); + if (NetworkEventBridge.markAsRecorded(thrown)) { + String message = thrown.getMessage() != null + ? thrown.getMessage() : thrown.getClass().getName(); + NetworkEventBridge.recordError(message); + } return; } - if (response != null) { + if (response != null && request != null) { int statusCode = response.getCode(); if (statusCode >= 400) { - String url; + String requestUri; try { URI uri = request.getUri(); - url = uri != null ? uri.toString() : ""; - } catch (java.net.URISyntaxException ignored) { - url = request.getRequestUri() != null ? request.getRequestUri() : ""; + requestUri = uri != null ? uri.toString() : null; + } catch (URISyntaxException ignored) { + requestUri = request.getRequestUri(); } - AgentTelemetryStore.getInstance().recordNetworkEventFor( - Level.CRITICAL, - Source.SERVER, + // The host-based overloads carry the target separately from a request whose URI may be + // just a path, so rejoin the two rather than reading the request URI alone. + String base = target != null ? target.toURI() : null; + NetworkEventBridge.recordNetworkEvent( + response, request.getMethod(), - UrlSanitizer.sanitize(url), + NetworkEventBridge.composeUrl(base, requestUri), String.valueOf(statusCode) ); } diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/NetworkEventBridgeTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/NetworkEventBridgeTest.java new file mode 100644 index 00000000..5f4c9212 --- /dev/null +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/NetworkEventBridgeTest.java @@ -0,0 +1,51 @@ +package com.rollbar.agent; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; + +public class NetworkEventBridgeTest { + + @Test + public void composeUrl_joinsHostWithRelativePath() { + assertEquals("https://api.example.com/charge", + NetworkEventBridge.composeUrl("https://api.example.com", "/charge")); + } + + @Test + public void composeUrl_joinsHostWithPortAndRelativePath() { + assertEquals("http://localhost:8080/charge", + NetworkEventBridge.composeUrl("http://localhost:8080", "/charge")); + } + + @Test + public void composeUrl_insertsSeparatorForPathWithoutLeadingSlash() { + assertEquals("https://api.example.com/charge", + NetworkEventBridge.composeUrl("https://api.example.com", "charge")); + } + + @Test + public void composeUrl_leavesAbsoluteRequestUriUntouched() { + assertEquals("https://other.example.com/charge", + NetworkEventBridge.composeUrl( + "https://api.example.com", "https://other.example.com/charge")); + } + + @Test + public void composeUrl_withoutBase_returnsRequestUri() { + assertEquals("/charge", NetworkEventBridge.composeUrl(null, "/charge")); + } + + @Test + public void composeUrl_withoutRequestUri_returnsBase() { + assertEquals("https://api.example.com", + NetworkEventBridge.composeUrl("https://api.example.com", null)); + assertEquals("https://api.example.com", + NetworkEventBridge.composeUrl("https://api.example.com", "")); + } + + @Test + public void composeUrl_withNeither_returnsEmptyString() { + assertEquals("", NetworkEventBridge.composeUrl(null, null)); + } +} diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java index 8be9532c..5d2784c8 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient4InstrumentationTest.java @@ -5,11 +5,14 @@ import com.rollbar.agent.AgentTelemetryStore; import com.rollbar.agent.NetworkEventBridge; import com.rollbar.api.payload.data.TelemetryEvent; +import org.apache.http.HttpHost; import org.apache.http.client.ResponseHandler; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; +import org.apache.http.message.BasicHttpRequest; +import org.apache.http.protocol.BasicHttpContext; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -79,17 +82,51 @@ public void serverErrorResponse_recordsNetworkEvent() throws Exception { } @Test - public void responseHandlerOverload_doesNotBreakInstrumentation() throws Exception { - // execute(HttpUriRequest, ResponseHandler) routes through execute(HttpHost, HttpRequest, ...) - // — a separate call chain that does not pass through execute(HttpUriRequest, HttpContext). - // No telemetry event is produced for this path, but the call must succeed without error. + public void responseHandlerOverload_recordsNetworkEvent() throws Exception { + // execute(HttpUriRequest, ResponseHandler) routes through execute(HttpHost, HttpRequest, ...), + // bypassing the single-request overloads. Instrumenting doExecute() — which every dispatch + // path converges on — is what makes this path visible. server.stubFor(get(urlEqualTo("/handler")).willReturn(aResponse().withStatus(404))); ResponseHandler handler = response -> response.getStatusLine().getStatusCode(); int status = client.execute(new HttpGet(server.baseUrl() + "/handler"), handler); assertEquals(404, status); - assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("404", body.get("status_code").toString()); + assertTrue(body.get("url").toString().endsWith("/handler")); + } + + @Test + public void hostBasedOverload_recordsNetworkEventWithFullUrl() throws Exception { + // execute(HttpHost, HttpRequest) invokes doExecute() directly. The request carries only a path, + // so the host must be rejoined from the HttpHost argument for the URL to be usable. + server.stubFor(get(urlEqualTo("/charge")).willReturn(aResponse().withStatus(500))); + + HttpHost target = new HttpHost("localhost", server.port(), "http"); + client.execute(target, new BasicHttpRequest("GET", "/charge")).close(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("500", body.get("status_code").toString()); + assertEquals(server.baseUrl() + "/charge", body.get("url").toString()); + } + + @Test + public void hostBasedOverloadWithContext_recordsNetworkEvent() throws Exception { + server.stubFor(get(urlEqualTo("/charge")).willReturn(aResponse().withStatus(503))); + + HttpHost target = new HttpHost("localhost", server.port(), "http"); + client.execute(target, new BasicHttpRequest("GET", "/charge"), new BasicHttpContext()).close(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("503", body.get("status_code").toString()); + assertEquals(server.baseUrl() + "/charge", body.get("url").toString()); } @Test diff --git a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java index f041850e..c49d6401 100644 --- a/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java +++ b/rollbar-java-agent/src/test/java/com/rollbar/agent/instrumentation/ApacheHttpClient5InstrumentationTest.java @@ -8,8 +8,10 @@ import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.io.HttpClientResponseHandler; import org.apache.hc.core5.http.message.BasicClassicHttpRequest; +import org.apache.hc.core5.http.protocol.BasicHttpContext; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -91,11 +93,10 @@ public void serverErrorResponse_recordsNetworkEvent() throws Exception { } @Test - public void responseHandlerOverload_doesNotBreakInstrumentation() throws Exception { - // execute(ClassicHttpRequest, HttpClientResponseHandler) routes through a separate call chain - // (HttpHost-based) that does not pass through the instrumented execute(ClassicHttpRequest) - // or execute(ClassicHttpRequest, HttpContext) overloads. No telemetry event is produced, - // but the call must succeed without error. + public void responseHandlerOverload_recordsNetworkEvent() throws Exception { + // execute(ClassicHttpRequest, HttpClientResponseHandler) routes through the HttpHost-based + // chain, bypassing the single-request overloads. Instrumenting doExecute() — which every + // dispatch path converges on — is what makes this path visible. server.stubFor(get(urlEqualTo("/handler")).willReturn(aResponse().withStatus(404))); HttpClientResponseHandler handler = response -> response.getCode(); @@ -103,7 +104,44 @@ public void responseHandlerOverload_doesNotBreakInstrumentation() throws Excepti new BasicClassicHttpRequest("GET", server.baseUrl() + "/handler"), handler); assertEquals(404, status); - assertTrue(AgentTelemetryStore.getInstance().getAll().isEmpty()); + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("404", body.get("status_code").toString()); + assertTrue(body.get("url").toString().endsWith("/handler")); + } + + @Test + public void hostBasedOverload_recordsNetworkEventWithFullUrl() throws Exception { + // execute(HttpHost, ClassicHttpRequest) invokes doExecute() directly. The request carries only + // a path, so the host must be rejoined from the HttpHost argument for the URL to be usable. + server.stubFor(get(urlEqualTo("/charge")).willReturn(aResponse().withStatus(500))); + + HttpHost target = new HttpHost("http", "localhost", server.port()); + client.execute(target, new BasicClassicHttpRequest("GET", "/charge")).close(); + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("500", body.get("status_code").toString()); + assertEquals(server.baseUrl() + "/charge", body.get("url").toString()); + } + + @Test + public void hostBasedOverloadWithContext_recordsNetworkEvent() throws Exception { + server.stubFor(get(urlEqualTo("/charge")).willReturn(aResponse().withStatus(503))); + + HttpHost target = new HttpHost("http", "localhost", server.port()); + try (CloseableHttpResponse response = client.execute( + target, new BasicClassicHttpRequest("GET", "/charge"), new BasicHttpContext())) { + assertEquals(503, response.getCode()); + } + + List events = AgentTelemetryStore.getInstance().getAll(); + assertEquals(1, events.size()); + Map body = (Map) events.get(0).asJson().get("body"); + assertEquals("503", body.get("status_code").toString()); + assertEquals(server.baseUrl() + "/charge", body.get("url").toString()); } @Test