address comments

Author: SentryMan

instrumentation/java-http-server/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpserver/JdkHttpServerInstrumentationModule.java renamed to instrumentation/java-http-server/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpserver/JavaHttpServerInstrumentationModule.java

@@ -5,21 +5,20 @@
 
 package io.opentelemetry.javaagent.instrumentation.httpserver;
 
-import static java.util.Arrays.asList;
-
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import java.util.Collections;
 import java.util.List;
 
 @AutoService(InstrumentationModule.class)
-public class JdkHttpServerInstrumentationModule extends InstrumentationModule {
-  public JdkHttpServerInstrumentationModule() {
+public class JavaHttpServerInstrumentationModule extends InstrumentationModule {
+  public JavaHttpServerInstrumentationModule() {
     super("java-http-server");
   }
 
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
-    return asList(new JdkServerContextInstrumentation());
+    return Collections.singletonList(new JavaServerContextInstrumentation());
   }
 }

instrumentation/java-http-server/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpserver/JdkServerContextInstrumentation.java renamed to instrumentation/java-http-server/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpserver/JavaServerContextInstrumentation.java

@@ -5,37 +5,38 @@
 
 package io.opentelemetry.javaagent.instrumentation.httpserver;
 
+import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 
 import com.sun.net.httpserver.HttpContext;
-import com.sun.net.httpserver.HttpServer;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 
-public class JdkServerContextInstrumentation implements TypeInstrumentation {
+public class JavaServerContextInstrumentation implements TypeInstrumentation {
 
   @Override
   public ElementMatcher<TypeDescription> typeMatcher() {
-    return named(HttpServer.class.getName());
+    return extendsClass(named("com.sun.net.httpserver.HttpServer"));
   }
 
   @Override
   public void transform(TypeTransformer transformer) {
     transformer.applyAdviceToMethod(
         isMethod().and(isPublic()).and(named("createContext")),
-        this.getClass().getName() + "$BuildAdvice");
+        JavaServerContextInstrumentation.class.getName() + "$BuildAdvice");
   }
 
+  @SuppressWarnings("unused")
   public static class BuildAdvice {
 
     @Advice.OnMethodExit
     public static void onExit(@Advice.Return HttpContext ctx) {
-      ctx.getFilters().addAll(JdkSingletons.SERVER_DECORATOR);
+      ctx.getFilters().addAll(JavaSingletons.FILTERS);
     }
   }
 }

instrumentation/java-http-server/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpserver/JavaSingletons.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.httpserver;
+
+import com.sun.net.httpserver.Filter;
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.instrumentation.api.incubator.config.internal.CommonConfig;
+import io.opentelemetry.instrumentation.httpserver.JavaServerTelemetry;
+import io.opentelemetry.instrumentation.httpserver.JavaServerTelemetryBuilder;
+import io.opentelemetry.instrumentation.httpserver.internal.JavaInstrumenterBuilderUtil;
+import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig;
+import java.util.Arrays;
+import java.util.List;
+
+// Holds singleton references to decorators to match against during suppression.
+// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/903
+public final class JavaSingletons {
+
+  public static final List<Filter> FILTERS;
+
+  static {
+    CommonConfig config = AgentCommonConfig.get();
+
+    JavaServerTelemetryBuilder serverBuilder =
+        JavaServerTelemetry.builder(GlobalOpenTelemetry.get());
+    JavaInstrumenterBuilderUtil.getServerBuilderExtractor().apply(serverBuilder).configure(config);
+    JavaServerTelemetry serverTelemetry = serverBuilder.build();
+
+    FILTERS = Arrays.asList(serverTelemetry.otelFilter(), new ResponseCustomizingFilter());
+  }
+
+  private JavaSingletons() {}
+}

instrumentation/java-http-server/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpserver/JdkSingletons.java

@@ -19,6 +19,7 @@ class JdkHttpServerTest extends AbstractJdkHttpServerTest {
   @Override
   protected void configure(HttpServerTestOptions options) {
     super.configure(options);
-    options.setTestHttpPipelining(false);
+    // library instrumentation does not create a span at all
+    options.disableTestNonStandardHttpMethod();
   }
 }

instrumentation/java-http-server/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/httpserver/JdkHttpServerTest.java

@@ -10,26 +10,26 @@
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 
-/** Entrypoint for instrumenting Armeria services. */
-public final class JdkServerTelemetry {
+/** Entrypoint for instrumenting the jdk.httpserver services. */
+public final class JavaServerTelemetry {
 
-  /** Returns a new {@link JdkServerTelemetry} configured with the given {@link OpenTelemetry}. */
-  public static JdkServerTelemetry create(OpenTelemetry openTelemetry) {
+  /** Returns a new {@link JavaServerTelemetry} configured with the given {@link OpenTelemetry}. */
+  public static JavaServerTelemetry create(OpenTelemetry openTelemetry) {
     return builder(openTelemetry).build();
   }
 
-  public static JdkServerTelemetryBuilder builder(OpenTelemetry openTelemetry) {
-    return new JdkServerTelemetryBuilder(openTelemetry);
+  public static JavaServerTelemetryBuilder builder(OpenTelemetry openTelemetry) {
+    return new JavaServerTelemetryBuilder(openTelemetry);
   }
 
   private final Instrumenter<HttpExchange, HttpExchange> instrumenter;
 
-  JdkServerTelemetry(Instrumenter<HttpExchange, HttpExchange> instrumenter) {
+  JavaServerTelemetry(Instrumenter<HttpExchange, HttpExchange> instrumenter) {
     this.instrumenter = instrumenter;
   }
 
   /** Returns a new {@link Filter} for telemetry usage */
   public Filter otelFilter() {
-    return new OpenTelemetryService(instrumenter);
+    return new OpenTelemetryFilter(instrumenter);
   }
 }

instrumentation/java-http-server/library/src/main/java/io/opentelemetry/instrumentation/httpserver/JdkServerTelemetry.java renamed to instrumentation/java-http-server/library/src/main/java/io/opentelemetry/instrumentation/httpserver/JavaServerTelemetry.java

@@ -13,26 +13,26 @@
 import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpServerAttributesExtractorBuilder;
-import io.opentelemetry.instrumentation.httpserver.internal.JdkInstrumenterBuilderFactory;
-import io.opentelemetry.instrumentation.httpserver.internal.JdkInstrumenterBuilderUtil;
+import io.opentelemetry.instrumentation.httpserver.internal.JavaInstrumenterBuilderFactory;
+import io.opentelemetry.instrumentation.httpserver.internal.JavaInstrumenterBuilderUtil;
 import java.util.Collection;
 import java.util.function.Function;
 
-public final class JdkServerTelemetryBuilder {
+public final class JavaServerTelemetryBuilder {
 
   private final DefaultHttpServerInstrumenterBuilder<HttpExchange, HttpExchange> builder;
 
   static {
-    JdkInstrumenterBuilderUtil.setServerBuilderExtractor(builder -> builder.builder);
+    JavaInstrumenterBuilderUtil.setServerBuilderExtractor(builder -> builder.builder);
   }
 
-  JdkServerTelemetryBuilder(OpenTelemetry openTelemetry) {
-    builder = JdkInstrumenterBuilderFactory.getServerBuilder(openTelemetry);
+  JavaServerTelemetryBuilder(OpenTelemetry openTelemetry) {
+    builder = JavaInstrumenterBuilderFactory.getServerBuilder(openTelemetry);
   }
 
   /** Sets the status extractor for server spans. */
   @CanIgnoreReturnValue
-  public JdkServerTelemetryBuilder setStatusExtractor(
+  public JavaServerTelemetryBuilder setStatusExtractor(
       Function<
               SpanStatusExtractor<? super HttpExchange, ? super HttpExchange>,
               ? extends SpanStatusExtractor<? super HttpExchange, ? super HttpExchange>>
@@ -46,7 +46,7 @@ public JdkServerTelemetryBuilder setStatusExtractor(
    * The {@link AttributesExtractor} will be executed after all default extractors.
    */
   @CanIgnoreReturnValue
-  public JdkServerTelemetryBuilder addAttributesExtractor(
+  public JavaServerTelemetryBuilder addAttributesExtractor(
       AttributesExtractor<HttpExchange, HttpExchange> attributesExtractor) {
     builder.addAttributesExtractor(attributesExtractor);
     return this;
@@ -58,7 +58,7 @@ public JdkServerTelemetryBuilder addAttributesExtractor(
    * @param requestHeaders A list of HTTP header names.
    */
   @CanIgnoreReturnValue
-  public JdkServerTelemetryBuilder setCapturedRequestHeaders(Collection<String> requestHeaders) {
+  public JavaServerTelemetryBuilder setCapturedRequestHeaders(Collection<String> requestHeaders) {
     builder.setCapturedRequestHeaders(requestHeaders);
     return this;
   }
@@ -69,7 +69,7 @@ public JdkServerTelemetryBuilder setCapturedRequestHeaders(Collection<String> re
    * @param responseHeaders A list of HTTP header names.
    */
   @CanIgnoreReturnValue
-  public JdkServerTelemetryBuilder setCapturedResponseHeaders(Collection<String> responseHeaders) {
+  public JavaServerTelemetryBuilder setCapturedResponseHeaders(Collection<String> responseHeaders) {
     builder.setCapturedResponseHeaders(responseHeaders);
     return this;
   }
@@ -88,14 +88,14 @@ public JdkServerTelemetryBuilder setCapturedResponseHeaders(Collection<String> r
    * @see HttpServerAttributesExtractorBuilder#setKnownMethods(Collection)
    */
   @CanIgnoreReturnValue
-  public JdkServerTelemetryBuilder setKnownMethods(Collection<String> knownMethods) {
+  public JavaServerTelemetryBuilder setKnownMethods(Collection<String> knownMethods) {
     builder.setKnownMethods(knownMethods);
     return this;
   }
 
   /** Sets custom server {@link SpanNameExtractor} via transform function. */
   @CanIgnoreReturnValue
-  public JdkServerTelemetryBuilder setSpanNameExtractor(
+  public JavaServerTelemetryBuilder setSpanNameExtractor(
       Function<
               SpanNameExtractor<? super HttpExchange>,
               ? extends SpanNameExtractor<? super HttpExchange>>
@@ -104,7 +104,7 @@ public JdkServerTelemetryBuilder setSpanNameExtractor(
     return this;
   }
 
-  public JdkServerTelemetry build() {
-    return new JdkServerTelemetry(builder.build());
+  public JavaServerTelemetry build() {
+    return new JavaServerTelemetry(builder.build());
   }
 }

instrumentation/java-http-server/library/src/main/java/io/opentelemetry/instrumentation/httpserver/JdkServerTelemetryBuilder.java renamed to instrumentation/java-http-server/library/src/main/java/io/opentelemetry/instrumentation/httpserver/JavaServerTelemetryBuilder.java

@@ -13,11 +13,11 @@
 import java.io.IOException;
 
 /** Decorates an {@link HttpServer} to trace inbound {@link HttpExchange}s. */
-final class OpenTelemetryService extends Filter {
+final class OpenTelemetryFilter extends Filter {
 
   private final Instrumenter<HttpExchange, HttpExchange> instrumenter;
 
-  OpenTelemetryService(Instrumenter<HttpExchange, HttpExchange> instrumenter) {
+  OpenTelemetryFilter(Instrumenter<HttpExchange, HttpExchange> instrumenter) {
 
     this.instrumenter = instrumenter;
   }
@@ -30,13 +30,16 @@ public void doFilter(HttpExchange exchange, Chain chain) throws IOException {
       chain.doFilter(exchange);
       return;
     }
-
+    Throwable error = null;
     Context context = instrumenter.start(parentContext, exchange);
 
     try (Scope ignored = context.makeCurrent()) {
       chain.doFilter(exchange);
+    } catch (Throwable t) {
+      error = t;
+      throw t;
     } finally {
-      instrumenter.end(context, exchange, null, null);
+      instrumenter.end(context, exchange, exchange, error);
     }
   }
 

instrumentation/java-http-server/library/src/main/java/io/opentelemetry/instrumentation/httpserver/OpenTelemetryService.java renamed to instrumentation/java-http-server/library/src/main/java/io/opentelemetry/instrumentation/httpserver/OpenTelemetryFilter.java

@@ -7,13 +7,14 @@
 
 import com.sun.net.httpserver.HttpExchange;
 import com.sun.net.httpserver.HttpsExchange;
+import io.opentelemetry.instrumentation.api.internal.HttpProtocolUtil;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpServerAttributesGetter;
 import java.net.InetSocketAddress;
 import java.util.Collections;
 import java.util.List;
 import javax.annotation.Nullable;
 
-enum JdkHttpServerAttributesGetter
+enum JavaHttpServerAttributesGetter
     implements HttpServerAttributesGetter<HttpExchange, HttpExchange> {
   INSTANCE;
 
@@ -29,17 +30,13 @@ public String getUrlScheme(HttpExchange exchange) {
 
   @Override
   public String getUrlPath(HttpExchange exchange) {
-    String fullPath = exchange.getRequestURI().toString();
-    int separatorPos = fullPath.indexOf('?');
-    return separatorPos == -1 ? fullPath : fullPath.substring(0, separatorPos);
+    return exchange.getRequestURI().getPath();
   }
 
   @Nullable
   @Override
   public String getUrlQuery(HttpExchange exchange) {
-    String fullPath = exchange.getRequestURI().toString();
-    int separatorPos = fullPath.indexOf('?');
-    return separatorPos == -1 ? null : fullPath.substring(separatorPos + 1);
+    return exchange.getRequestURI().getQuery();
   }
 
   @Override
@@ -71,13 +68,13 @@ public String getHttpRoute(HttpExchange exchange) {
 
   @Override
   public String getNetworkProtocolName(HttpExchange exchange, @Nullable HttpExchange res) {
-    return exchange instanceof HttpsExchange ? "https" : "http";
+    return HttpProtocolUtil.getProtocol(exchange.getProtocol());
   }
 
   @Override
   public String getNetworkProtocolVersion(HttpExchange exchange, @Nullable HttpExchange res) {
 
-    return "1.1";
+    return HttpProtocolUtil.getVersion(exchange.getProtocol());
   }
 
   @Override

instrumentation/java-http-server/library/src/main/java/io/opentelemetry/instrumentation/httpserver/internal/JdkHttpServerAttributesGetter.java renamed to instrumentation/java-http-server/library/src/main/java/io/opentelemetry/instrumentation/httpserver/internal/JavaHttpServerAttributesGetter.java

@@ -13,8 +13,8 @@
  * This class is internal and is hence not for public use. Its APIs are unstable and can change at
  * any time.
  */
-public final class JdkInstrumenterBuilderFactory {
-  private JdkInstrumenterBuilderFactory() {}
+public final class JavaInstrumenterBuilderFactory {
+  private JavaInstrumenterBuilderFactory() {}
 
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.java-http-server";
 
@@ -23,7 +23,7 @@ public static DefaultHttpServerInstrumenterBuilder<HttpExchange, HttpExchange> g
     return DefaultHttpServerInstrumenterBuilder.create(
         INSTRUMENTATION_NAME,
         openTelemetry,
-        JdkHttpServerAttributesGetter.INSTANCE,
+        JavaHttpServerAttributesGetter.INSTANCE,
         ExchangeContextGetter.INSTANCE);
   }
 }