(chore) cleanup from review

open-telemetry · Nov 19, 2024 · 55aa9c5 · 55aa9c5
1 parent 948e5ba
commit 55aa9c5
Show file tree

Hide file tree

Showing 12 changed files with 88 additions and 163 deletions.
diff --git a/.../java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackForkedHttpClientTest.java b/.../java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackForkedHttpClientTest.java
@@ -5,29 +5,14 @@
 
 package io.opentelemetry.javaagent.instrumentation.ratpack;
 
-import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS;
-import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT;
-
-import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackForkedHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
-import java.net.URI;
-import java.util.HashSet;
-import java.util.Set;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 class RatpackForkedHttpClientTest extends AbstractRatpackForkedHttpClientTest {
 
   @RegisterExtension
   static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent();
 
-  @Override
-  protected Set<AttributeKey<?>> computeHttpAttributes(URI uri) {
-    Set<AttributeKey<?>> attributes = new HashSet<>(super.computeHttpAttributes(uri));
-    // underlying netty instrumentation does not provide these
-    attributes.remove(SERVER_ADDRESS);
-    attributes.remove(SERVER_PORT);
-    return attributes;
-  }
 }
diff --git a/...c/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackHttpClientTest.java b/...c/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackHttpClientTest.java
@@ -5,29 +5,14 @@
 
 package io.opentelemetry.javaagent.instrumentation.ratpack;
 
-import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS;
-import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT;
-
-import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
-import java.net.URI;
-import java.util.HashSet;
-import java.util.Set;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 class RatpackHttpClientTest extends AbstractRatpackHttpClientTest {
 
   @RegisterExtension
   static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent();
 
-  @Override
-  protected Set<AttributeKey<?>> computeHttpAttributes(URI uri) {
-    Set<AttributeKey<?>> attributes = new HashSet<>(super.computeHttpAttributes(uri));
-    // underlying netty instrumentation does not provide these
-    attributes.remove(SERVER_ADDRESS);
-    attributes.remove(SERVER_PORT);
-    return attributes;
-  }
 }
diff --git a/.../java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackPooledHttpClientTest.java b/.../java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackPooledHttpClientTest.java
@@ -5,29 +5,14 @@
 
 package io.opentelemetry.javaagent.instrumentation.ratpack;
 
-import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS;
-import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT;
-
-import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackPooledHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
-import java.net.URI;
-import java.util.HashSet;
-import java.util.Set;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 class RatpackPooledHttpClientTest extends AbstractRatpackPooledHttpClientTest {
 
   @RegisterExtension
   static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent();
 
-  @Override
-  protected Set<AttributeKey<?>> computeHttpAttributes(URI uri) {
-    Set<AttributeKey<?>> attributes = new HashSet<>(super.computeHttpAttributes(uri));
-    // underlying netty instrumentation does not provide these
-    attributes.remove(SERVER_ADDRESS);
-    attributes.remove(SERVER_PORT);
-    return attributes;
-  }
 }
diff --git a/...n/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java b/...n/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java
@@ -5,15 +5,20 @@
 
 package io.opentelemetry.instrumentation.ratpack.client;
 
+import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS;
+import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT;
+
 import io.netty.channel.ConnectTimeoutException;
 import io.netty.handler.timeout.ReadTimeoutException;
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
+import io.opentelemetry.semconv.NetworkAttributes;
 import java.net.URI;
 import java.time.Duration;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import org.junit.jupiter.api.AfterAll;
@@ -122,32 +127,13 @@ protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
                   .getValueOrThrow();
             });
 
-    optionsBuilder.setExpectedClientSpanNameMapper(
-        (uri, method) -> {
-          switch (uri.toString()) {
-            case "http://localhost:61/": // unopened port
-            case "https://192.0.2.1/": // non routable address
-              return "CONNECT";
-            default:
-              return HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER.apply(
-                  uri, method);
-          }
-        });
+    if (useNettyClientAttributes()) {
+      optionsBuilder.setExpectedClientSpanNameMapper(
+          AbstractRatpackHttpClientTest::nettyExpectedClientSpanNameMapper);
+    }
 
     optionsBuilder.setClientSpanErrorMapper(
-        (uri, exception) -> {
-          if (uri.toString().equals("https://192.0.2.1/")) {
-            return new ConnectTimeoutException(
-                "connection timed out"
-                    + (Boolean.getBoolean("testLatestDeps") ? " after 2000 ms" : "")
-                    + ": /192.0.2.1:443");
-          } else if (OS.WINDOWS.isCurrentOs() && uri.toString().equals("http://localhost:61/")) {
-            return new ConnectTimeoutException("connection timed out: localhost/127.0.0.1:61");
-          } else if (uri.getPath().equals("/read-timeout")) {
-            return ReadTimeoutException.INSTANCE;
-          }
-          return exception;
-        });
+        AbstractRatpackHttpClientTest::nettyClientSpanErrorMapper);
 
     optionsBuilder.setHttpAttributes(this::computeHttpAttributes);
 
@@ -164,7 +150,45 @@ protected Set<AttributeKey<?>> computeHttpAttributes(URI uri) {
       case "https://192.0.2.1/": // non routable address
         return Collections.emptySet();
       default:
-        return HttpClientTestOptions.DEFAULT_HTTP_ATTRIBUTES;
+        HashSet<AttributeKey<?>> attributes =
+            new HashSet<>(HttpClientTestOptions.DEFAULT_HTTP_ATTRIBUTES);
+        if (useNettyClientAttributes()) {
+          // underlying netty instrumentation does not provide these
+          attributes.remove(SERVER_ADDRESS);
+          attributes.remove(SERVER_PORT);
+        } else {
+          // ratpack client instrumentation does not provide this
+          attributes.remove(NetworkAttributes.NETWORK_PROTOCOL_VERSION);
+        }
+        return attributes;
+    }
+  }
+
+  protected boolean useNettyClientAttributes() {
+    return true;
+  }
+
+  private static Throwable nettyClientSpanErrorMapper(URI uri, Throwable exception) {
+    if (uri.toString().equals("https://192.0.2.1/")) {
+      return new ConnectTimeoutException(
+          "connection timed out"
+              + (Boolean.getBoolean("testLatestDeps") ? " after 2000 ms" : "")
+              + ": /192.0.2.1:443");
+    } else if (OS.WINDOWS.isCurrentOs() && uri.toString().equals("http://localhost:61/")) {
+      return new ConnectTimeoutException("connection timed out: localhost/127.0.0.1:61");
+    } else if (uri.getPath().equals("/read-timeout")) {
+      return ReadTimeoutException.INSTANCE;
+    }
+    return exception;
+  }
+
+  private static String nettyExpectedClientSpanNameMapper(URI uri, String method) {
+    switch (uri.toString()) {
+      case "http://localhost:61/": // unopened port
+      case "https://192.0.2.1/": // non routable address
+        return "CONNECT";
+      default:
+        return HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER.apply(uri, method);
     }
   }
 }
diff --git a/.../main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackSingletons.java b/.../main/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackSingletons.java
@@ -11,9 +11,7 @@
 import io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys;
 import io.opentelemetry.instrumentation.ratpack.v1_7.internal.ContextHolder;
 import io.opentelemetry.instrumentation.ratpack.v1_7.internal.OpenTelemetryHttpClient;
-import io.opentelemetry.instrumentation.ratpack.v1_7.internal.OpenTelemetryServerHandler;
 import io.opentelemetry.instrumentation.ratpack.v1_7.internal.RatpackClientInstrumenterBuilderFactory;
-import io.opentelemetry.instrumentation.ratpack.v1_7.internal.RatpackServerInstrumenterBuilderFactory;
 import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig;
 import ratpack.exec.Execution;
 
@@ -26,25 +24,14 @@ public final class RatpackSingletons {
                     "io.opentelemetry.ratpack-1.7", GlobalOpenTelemetry.get())
                 .configure(AgentCommonConfig.get())
                 .build());
-    SERVER_HANDLER =
-        new OpenTelemetryServerHandler(
-            RatpackServerInstrumenterBuilderFactory.create(
-                    "io.opentelemetry.ratpack-1.7", GlobalOpenTelemetry.get())
-                .configure(AgentCommonConfig.get())
-                .build());
   }
 
   private static final OpenTelemetryHttpClient HTTP_CLIENT;
-  private static final OpenTelemetryServerHandler SERVER_HANDLER;
 
   public static OpenTelemetryHttpClient httpClient() {
     return HTTP_CLIENT;
   }
 
-  public static OpenTelemetryServerHandler serverHandler() {
-    return SERVER_HANDLER;
-  }
-
   public static void propagateContextToChannel(Execution execution, Channel channel) {
     Context parentContext =
         execution

diff --git a/...telemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java b/...telemetry/javaagent/instrumentation/ratpack/v1_7/RequestActionSupportInstrumentation.java
@@ -77,7 +77,7 @@ public static class ContextAdvice {
     public static Scope injectChannelAttribute(
         @Advice.FieldValue("execution") Execution execution) {
 
-      // Capture the CLIENT span and make it current before cally Netty layer
+      // Capture the CLIENT span and make it current before calling Netty layer
       return execution
           .maybeGet(ContextHolder.class)
           .map(ContextHolder::context)

diff --git a/.../io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java b/.../io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackForkedHttpClientTest.java
@@ -5,48 +5,25 @@
 
 package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7;
 
-import io.netty.channel.ConnectTimeoutException;
-import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackForkedHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
-import io.opentelemetry.semconv.NetworkAttributes;
-import java.net.URI;
-import java.util.HashSet;
-import java.util.Set;
-import org.junit.jupiter.api.condition.OS;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import ratpack.http.client.HttpClientReadTimeoutException;
 
 class RatpackForkedHttpClientTest extends AbstractRatpackForkedHttpClientTest {
 
   @RegisterExtension
   static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent();
 
   @Override
-  protected Set<AttributeKey<?>> computeHttpAttributes(URI uri) {
-    Set<AttributeKey<?>> attributes = new HashSet<>(super.computeHttpAttributes(uri));
-    attributes.remove(NetworkAttributes.NETWORK_PROTOCOL_VERSION);
-    return attributes;
+  protected boolean useNettyClientAttributes() {
+    return false;
   }
 
   @Override
   protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
     super.configure(optionsBuilder);
-    optionsBuilder.setExpectedClientSpanNameMapper(
-        HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER);
-    optionsBuilder.setClientSpanErrorMapper(
-        (uri, exception) -> {
-          if (uri.toString().equals("https://192.0.2.1/")) {
-            return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri);
-          } else if (OS.WINDOWS.isCurrentOs() && uri.toString().equals("http://localhost:61/")) {
-            return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri);
-          } else if (uri.getPath().equals("/read-timeout")) {
-            return new HttpClientReadTimeoutException(
-                "Read timeout (PT2S) waiting on HTTP server at " + uri);
-          }
-          return exception;
-        });
+    optionsBuilder.setClientSpanErrorMapper(RatpackTestSingletons::ratpackClientSpanErrorMapper);
   }
 }
diff --git a/...t/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java b/...t/java/io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackHttpClientTest.java
@@ -5,48 +5,25 @@
 
 package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7;
 
-import io.netty.channel.ConnectTimeoutException;
-import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
-import io.opentelemetry.semconv.NetworkAttributes;
-import java.net.URI;
-import java.util.HashSet;
-import java.util.Set;
-import org.junit.jupiter.api.condition.OS;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import ratpack.http.client.HttpClientReadTimeoutException;
 
 class RatpackHttpClientTest extends AbstractRatpackHttpClientTest {
 
   @RegisterExtension
   static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent();
 
   @Override
-  protected Set<AttributeKey<?>> computeHttpAttributes(URI uri) {
-    Set<AttributeKey<?>> attributes = new HashSet<>(super.computeHttpAttributes(uri));
-    attributes.remove(NetworkAttributes.NETWORK_PROTOCOL_VERSION);
-    return attributes;
+  protected boolean useNettyClientAttributes() {
+    return false;
   }
 
   @Override
   protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
     super.configure(optionsBuilder);
-    optionsBuilder.setExpectedClientSpanNameMapper(
-        HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER);
-    optionsBuilder.setClientSpanErrorMapper(
-        (uri, exception) -> {
-          if (uri.toString().equals("https://192.0.2.1/")) {
-            return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri);
-          } else if (OS.WINDOWS.isCurrentOs() && uri.toString().equals("http://localhost:61/")) {
-            return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri);
-          } else if (uri.getPath().equals("/read-timeout")) {
-            return new HttpClientReadTimeoutException(
-                "Read timeout (PT2S) waiting on HTTP server at " + uri);
-          }
-          return exception;
-        });
+    optionsBuilder.setClientSpanErrorMapper(RatpackTestSingletons::ratpackClientSpanErrorMapper);
   }
 }
diff --git a/.../io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java b/.../io/opentelemetry/javaagent/instrumentation/ratpack/v1_7/RatpackPooledHttpClientTest.java
@@ -5,47 +5,25 @@
 
 package io.opentelemetry.javaagent.instrumentation.ratpack.v1_7;
 
-import io.netty.channel.ConnectTimeoutException;
-import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackPooledHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
-import io.opentelemetry.semconv.NetworkAttributes;
-import java.net.URI;
-import java.util.HashSet;
-import java.util.Set;
-import org.junit.jupiter.api.condition.OS;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import ratpack.http.client.HttpClientReadTimeoutException;
 
 class RatpackPooledHttpClientTest extends AbstractRatpackPooledHttpClientTest {
 
   @RegisterExtension
   static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent();
 
   @Override
-  protected Set<AttributeKey<?>> computeHttpAttributes(URI uri) {
-    Set<AttributeKey<?>> attributes = new HashSet<>(super.computeHttpAttributes(uri));
-    attributes.remove(NetworkAttributes.NETWORK_PROTOCOL_VERSION);
-    return attributes;
+  protected boolean useNettyClientAttributes() {
+    return false;
   }
 
   @Override
   protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
     super.configure(optionsBuilder);
-    optionsBuilder.setExpectedClientSpanNameMapper(
-        HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER);
-    optionsBuilder.setClientSpanErrorMapper(
-        (uri, exception) -> {
-          if (uri.toString().equals("https://192.0.2.1/")
-              || (OS.WINDOWS.isCurrentOs() && uri.toString().equals("http://localhost:61/"))) {
-            return new ConnectTimeoutException("Connect timeout (PT2S) connecting to " + uri);
-          } else if (uri.getPath().equals("/read-timeout")) {
-            return new HttpClientReadTimeoutException(
-                "Read timeout (PT2S) waiting on HTTP server at " + uri);
-          }
-          return exception;
-        });
+    optionsBuilder.setClientSpanErrorMapper(RatpackTestSingletons::ratpackClientSpanErrorMapper);
   }
 }