Make HttpURLConnection connection inactivity timeout configurable and add test for harvester code

instrumentation/httpurlconnection/README.md

@@ -20,7 +20,7 @@ A span associated with a given request is concluded in the following scenarios:
 - When the getInputStream()/getErrorStream() APIs are called, the span concludes after the stream is fully read, an IOException is encountered, or the stream is closed.
 - When the disconnect API is called.
 
-Spans won't be automatically ended and reported otherwise. If any of your URLConnection requests do not call the span concluding APIs mentioned above, refer the section entitled ["Scheduling Harvester Thread"](#scheduling-harvester-thread). This section provides guidance on setting up a recurring thread that identifies unreported, idle connections (those that have been read from but have been inactive for more than 10 seconds) and concludes any open spans associated with them.
+Spans won't be automatically ended and reported otherwise. If any of your URLConnection requests do not call the span concluding APIs mentioned above, refer the section entitled ["Scheduling Harvester Thread"](#scheduling-harvester-thread). This section provides guidance on setting up a recurring thread that identifies unreported, idle connections and ends/reports any open spans associated with them. Idle connections are those that have been read from previously but have been inactive for a particular configurable time interval (defaults to 10 seconds).
 
 > The minimum supported Android SDK version is 21, though it will also instrument APIs added in the Android SDK version 24 when running on devices with API level 24 and above.
 
@@ -62,12 +62,15 @@ byteBuddy("io.opentelemetry.android:httpurlconnection-agent:AUTO_HTTP_URL_INSTRU
 
 #### Scheduling Harvester Thread
 
-To schedule a periodically running thread to conclude spans on any unreported, idle connections, add the below code in the function where your application starts ( that could be onCreate() method of first Activity/Fragment/Service):
+To schedule a periodically running thread to conclude/report spans on any unreported, idle connections, add the below code in the function where your application starts ( that could be onCreate() method of first Activity/Fragment/Service):
 ```Java
+HttpUrlInstrumentationConfig.setConnectionInactivityTimeoutMs(customTimeoutValue); //This is optional. Replace customTimeoutValue with a long data type value which denotes the connection inactivity timeout in milli seconds. Defaults to 10000ms
 Executors.newSingleThreadScheduledExecutor().scheduleWithFixedDelay(HttpUrlInstrumentationConfig.getReportIdleConnectionRunnable(), 0, HttpUrlInstrumentationConfig.getReportIdleConnectionInterval(), TimeUnit.MILLISECONDS);
 ```
 
-`HttpUrlInstrumentationConfig.getReportIdleConnectionRunnable()` is the API to get the runnable. `HttpUrlInstrumentationConfig.getReportIdleConnectionInterval()` is the API to get the fixed interval (10s) in milli seconds.
+- `HttpUrlInstrumentationConfig.getReportIdleConnectionRunnable()` is the API to get the runnable.
+- By default, connections inactive for 10000ms are reported. To optionally change the connection inactivity timeout call `HttpUrlInstrumentationConfig.setConnectionInactivityTimeoutMs(customTimeoutValue)` API as shown in above code snippet.
+- It is efficient to run the harvester thread at the same time interval as the connection inactivity timeout used to identify the connections to be reported. `HttpUrlInstrumentationConfig.getReportIdleConnectionInterval()` is the API to get the same connection inactivity timeout interval (milliseconds) you have configured or the default value of 10000ms if not configured.
 
 #### Other Optional Configurations
 You can optionally configure the automatic instrumentation by using the setters in [HttpUrlInstrumentationConfig](library/src/main/java/io/opentelemetry/instrumentation/library/httpurlconnection/HttpUrlInstrumentationConfig.java).

instrumentation/httpurlconnection/library/src/main/java/io/opentelemetry/instrumentation/library/httpurlconnection/HttpUrlInstrumentationConfig.java

@@ -24,7 +24,7 @@ public final class HttpUrlInstrumentationConfig {
 
     // Time (ms) to wait before assuming that an idle connection is no longer
     // in use and should be reported.
-    private static final long CONNECTION_INACTIVITY_TIMEOUT_MS = 10000;
+    private static long connectionInactivityTimeoutMs = 10000;
 
     private HttpUrlInstrumentationConfig() {}
 
@@ -119,20 +119,53 @@ public static boolean emitExperimentalHttpClientMetrics() {
         return emitExperimentalHttpClientMetrics;
     }
 
+    /**
+     * Configures the connection inactivity timeout in milliseconds.
+     *
+     * <p>This timeout defines the time that should have elapsed since the connection was last
+     * active. It is used by the idle connection harvester thread to find idle connections that
+     * should be reported. To schedule the idle connection harvester follow instructions defined in
+     * the <a
+     * href="https://github.com/open-telemetry/opentelemetry-android/blob/96ea4aa9fe709838a91811deeb85b0f1baceb8cd/instrumentation/httpurlconnection/README.md#scheduling-harvester-thread">
+     * README</a>. If the specified timeout is negative, an {@code IllegalArgumentException} will be
+     * thrown.
+     *
+     * <p>Default value: "10000"
+     *
+     * @param timeoutMs the timeout period in milliseconds. Must be non-negative.
+     * @throws IllegalArgumentException if {@code timeoutMs} is negative.
+     */
+    public static void setConnectionInactivityTimeoutMs(long timeoutMs) {
+        if (timeoutMs < 0) {
+            throw new IllegalArgumentException("timeoutMs must be non-negative");
+        }
+        HttpUrlInstrumentationConfig.connectionInactivityTimeoutMs = timeoutMs;
+    }
+
+    /**
+     * Configures the connection inactivity timeout in milliseconds for testing purposes only.
+     *
+     * <p>This test only API allows you to set negative values for timeout. For production
+     * workflows, {@code setConnectionInactivityTimeoutMs} API should be used.
+     *
+     * @param timeoutMsForTesting the timeout period in milliseconds.
+     */
+    public static void setConnectionInactivityTimeoutMsForTesting(long timeoutMsForTesting) {
+        HttpUrlInstrumentationConfig.connectionInactivityTimeoutMs = timeoutMsForTesting;
+    }
+
     /**
      * Returns a runnable that can be scheduled to run periodically at a fixed interval to close
-     * open spans if connection is left idle for CONNECTION_INACTIVITY_TIMEOUT duration. Runnable
-     * interval is same as CONNECTION_INACTIVITY_TIMEOUT. CONNECTION_INACTIVITY_TIMEOUT in milli
-     * seconds can be obtained from getReportIdleConnectionInterval() API.
+     * open spans if connection is left idle for connectionInactivityTimeoutMs duration.
+     * connectionInactivityTimeoutMs can be obtained via getReportIdleConnectionInterval() API.
      *
      * @return The idle connection reporting runnable
      */
     public static Runnable getReportIdleConnectionRunnable() {
         return new Runnable() {
             @Override
             public void run() {
-                HttpUrlReplacements.reportIdleConnectionsOlderThan(
-                        CONNECTION_INACTIVITY_TIMEOUT_MS);
+                HttpUrlReplacements.reportIdleConnectionsOlderThan(connectionInactivityTimeoutMs);
             }
 
             @Override
@@ -143,12 +176,12 @@ public String toString() {
     }
 
     /**
-     * The fixed interval duration in milli seconds that the runnable from
+     * The interval duration in milli seconds that the runnable from
      * getReportIdleConnectionRunnable() API should be scheduled to periodically run at.
      *
-     * @return The fixed interval duration in ms
+     * @return The interval duration in ms
      */
     public static long getReportIdleConnectionInterval() {
-        return CONNECTION_INACTIVITY_TIMEOUT_MS;
+        return connectionInactivityTimeoutMs;
     }
 }

instrumentation/httpurlconnection/testing/build.gradle.kts

@@ -3,6 +3,10 @@ plugins {
     id("net.bytebuddy.byte-buddy-gradle-plugin")
 }
 
+android {
+    namespace = "io.opentelemetry.android.httpurlconnection.test"
+}
+
 dependencies {
     byteBuddy(project(":instrumentation:httpurlconnection:agent"))
     implementation(project(":instrumentation:httpurlconnection:library"))

instrumentation/httpurlconnection/testing/src/androidTest/kotlin/io/opentelemetry/instrumentation/library/httpurlconnection/InstrumentationTest.kt

@@ -96,4 +96,18 @@ class InstrumentationTest {
 
         assertThat(inMemorySpanExporter.finishedSpanItems.size).isEqualTo(2)
     }
+
+    @Test
+    fun testHttpUrlConnectionGetRequest_WhenNoStreamFetchedAndNoDisconnectCalledButHarvesterScheduled_ShouldBeTraced() {
+        executeGet("http://httpbin.org/get", false, false)
+        // setting a -1ms connection inactivity timeout for testing to ensure harvester sees it as 1ms elapsed
+        // and we don't have to include any wait timers in the test. 0ms does not work as the time difference
+        // between last connection activity and harvester time elapsed check is much lesser than 1ms due to
+        // our high speed modern CPUs.
+        HttpUrlInstrumentationConfig.setConnectionInactivityTimeoutMsForTesting(-1)
+        // Running the harvester runnable once instead of scheduling it to run periodically,
+        // so we can synchronously assert instead of waiting for another threads execution to finish
+        HttpUrlInstrumentationConfig.getReportIdleConnectionRunnable().run()
+        assertThat(inMemorySpanExporter.finishedSpanItems.size).isEqualTo(1)
+    }
 }