Capture response headers for HttpUrlConnection (#4417)

Author: trask

instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java

@@ -74,6 +74,11 @@ public static void methodEnter(
         // top-level HttpURLConnection calls
         return;
       }
+      // double increment on first entry is used to prevent infinite recursion in case end()
+      // captures response headers due to HttpUrlConnection.getHeaderField() calling
+      // HttpUrlConnection.getInputStream() which then enters this advice again
+      callDepth.getAndIncrement();
+
       Context parentContext = currentContext();
       if (!instrumenter().shouldStart(parentContext, connection)) {
         return;
@@ -108,35 +113,45 @@ public static void methodExit(
         @Advice.Local("otelHttpUrlState") HttpUrlState httpUrlState,
         @Advice.Local("otelScope") Scope scope,
         @Advice.Local("otelCallDepth") CallDepth callDepth) {
-      if (callDepth.decrementAndGet() > 0) {
+      // checking against 1 instead of against 0, because of the double increment which is used to
+      // prevent infinite recursion in case end() captures response headers
+      if (callDepth.decrementAndGet() > 1) {
         return;
       }
       if (scope == null) {
+        // need to perform the second decrement here since bailing out early
+        callDepth.decrementAndGet();
         return;
       }
-      scope.close();
-
-      if (throwable != null) {
-        if (responseCode >= 400) {
-          // HttpURLConnection unnecessarily throws exception on error response.
-          // None of the other http clients do this, so not recording the exception on the span
-          // to be consistent with the telemetry for other http clients.
+      try {
+        scope.close();
+
+        if (throwable != null) {
+          if (responseCode >= 400) {
+            // HttpURLConnection unnecessarily throws exception on error response.
+            // None of the other http clients do this, so not recording the exception on the span
+            // to be consistent with the telemetry for other http clients.
+            instrumenter().end(httpUrlState.context, connection, responseCode, null);
+          } else {
+            instrumenter()
+                .end(
+                    httpUrlState.context,
+                    connection,
+                    responseCode > 0 ? responseCode : null,
+                    throwable);
+          }
+          httpUrlState.finished = true;
+        } else if (methodName.equals("getInputStream") && responseCode > 0) {
+          // responseCode field is sometimes not populated.
+          // We can't call getResponseCode() due to some unwanted side-effects
+          // (e.g. breaks getOutputStream).
           instrumenter().end(httpUrlState.context, connection, responseCode, null);
-        } else {
-          instrumenter()
-              .end(
-                  httpUrlState.context,
-                  connection,
-                  responseCode > 0 ? responseCode : null,
-                  throwable);
+          httpUrlState.finished = true;
         }
-        httpUrlState.finished = true;
-      } else if (methodName.equals("getInputStream") && responseCode > 0) {
-        // responseCode field is sometimes not populated.
-        // We can't call getResponseCode() due to some unwanted side-effects
-        // (e.g. breaks getOutputStream).
-        instrumenter().end(httpUrlState.context, connection, responseCode, null);
-        httpUrlState.finished = true;
+      } finally {
+        // double increment is used to prevent infinite recursion in case end() captures response
+        // headers
+        callDepth.decrementAndGet();
       }
     }
   }

instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlHttpAttributesExtractor.java

@@ -72,6 +72,7 @@ protected Long responseContentLengthUncompressed(
   @Override
   protected List<String> responseHeader(
       HttpURLConnection connection, Integer statusCode, String name) {
-    return emptyList();
+    String value = connection.getHeaderField(name);
+    return value == null ? emptyList() : singletonList(value);
   }
 }