Enable grizzly instrumentation by default (#6049)

open-telemetry · May 17, 2022 · 6c44340 · 6c44340
1 parent 528360d
commit 6c44340
Show file tree

Hide file tree

Showing 8 changed files with 16 additions and 23 deletions.
diff --git a/docs/supported-libraries.md b/docs/supported-libraries.md
@@ -12,7 +12,6 @@ or [contributing](../CONTRIBUTING.md).
   * [Application Servers](#application-servers)
   * [JVMs and Operating Systems](#jvms-and-operating-systems)
   * [Disabled instrumentations](#disabled-instrumentations)
-    + [Grizzly instrumentation](#grizzly-instrumentation)
 
 ## Libraries / Frameworks
 
@@ -152,13 +151,3 @@ For this reason, the following instrumentations are disabled by default:
 
 To enable them, add the `otel.instrumentation.<name>.enabled` system property:
 `-Dotel.instrumentation.jdbc-datasource.enabled=true`
-
-### Grizzly instrumentation
-
-When you use
-[Grizzly](https://javaee.github.io/grizzly/httpserverframework.html) for
-Servlet-based applications, you get better experience from Servlet-specific
-support. As these two instrumentations conflict with each other, more generic
-instrumentation for Grizzly HTTP server is disabled by default. If needed,
-you can enable it by adding the following system property:
-`-Dotel.instrumentation.grizzly.enabled=true`
diff --git a/instrumentation/grizzly-2.0/javaagent/build.gradle.kts b/instrumentation/grizzly-2.0/javaagent/build.gradle.kts
@@ -14,6 +14,8 @@ muzzle {
 dependencies {
   compileOnly("org.glassfish.grizzly:grizzly-http:2.0")
 
+  bootstrap(project(":instrumentation:servlet:servlet-common:bootstrap"))
+
   testImplementation("javax.xml.bind:jaxb-api:2.2.3")
   testImplementation("javax.ws.rs:javax.ws.rs-api:2.0")
   testLibrary("org.glassfish.jersey.containers:jersey-container-grizzly2-http:2.0")

diff --git a/...io/opentelemetry/javaagent/instrumentation/grizzly/DefaultFilterChainInstrumentation.java b/...io/opentelemetry/javaagent/instrumentation/grizzly/DefaultFilterChainInstrumentation.java
@@ -12,6 +12,7 @@
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 
 import io.opentelemetry.context.Context;
+import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import net.bytebuddy.asm.Advice;
@@ -48,6 +49,9 @@ public static void onFail(
       HttpRequestPacket request = GrizzlyStateStorage.removeRequest(ctx);
       if (context != null && request != null) {
         Throwable error = GrizzlyErrorHolder.getOrDefault(context, throwable);
+        if (error == null) {
+          error = AppServerBridge.getException(context);
+        }
         instrumenter().end(context, request, null, error);
       }
     }

diff --git a/...java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyInstrumentationModule.java b/...java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyInstrumentationModule.java
@@ -27,9 +27,4 @@ public List<TypeInstrumentation> typeInstrumentations() {
         new HttpServerFilterInstrumentation(),
         new HttpHandlerInstrumentation());
   }
-
-  @Override
-  public boolean defaultEnabled() {
-    return false;
-  }
 }
diff --git a/...t/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlySingletons.java b/...t/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlySingletons.java
@@ -13,6 +13,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
+import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge;
 import org.glassfish.grizzly.http.HttpRequestPacket;
 import org.glassfish.grizzly.http.HttpResponsePacket;
 
@@ -33,6 +34,9 @@ public final class GrizzlySingletons {
             .addAttributesExtractor(HttpServerAttributesExtractor.create(httpAttributesGetter))
             .addAttributesExtractor(NetServerAttributesExtractor.create(netAttributesGetter))
             .addOperationMetrics(HttpServerMetrics.get())
+            .addContextCustomizer(
+                (context, request, attributes) ->
+                    new AppServerBridge.Builder().recordException().init(context))
             .addContextCustomizer(
                 (context, httpRequestPacket, startAttributes) -> GrizzlyErrorHolder.init(context))
             .addContextCustomizer(HttpRouteHolder.get())

diff --git a/...a/io/opentelemetry/javaagent/instrumentation/grizzly/HttpServerFilterInstrumentation.java b/...a/io/opentelemetry/javaagent/instrumentation/grizzly/HttpServerFilterInstrumentation.java
@@ -11,6 +11,7 @@
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 
 import io.opentelemetry.context.Context;
+import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import net.bytebuddy.asm.Advice;
@@ -50,6 +51,9 @@ public static void onExit(
       HttpRequestPacket request = GrizzlyStateStorage.removeRequest(ctx);
       if (context != null && request != null) {
         Throwable error = GrizzlyErrorHolder.getOrDefault(context, null);
+        if (error == null) {
+          error = AppServerBridge.getException(context);
+        }
         instrumenter().end(context, request, response, error);
       }
     }

diff --git a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/AppServerTest.groovy b/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/AppServerTest.groovy
@@ -265,10 +265,10 @@ abstract class AppServerTest extends SmokeTest {
     traces.countFilteredAttributes("http.target", "/app/exception") == 1
 
     and: "Number of spans tagged with current otel library version"
-    traces.countFilteredResourceAttributes("telemetry.auto.version", currentAgentVersion) == 1
+    traces.countFilteredResourceAttributes("telemetry.auto.version", currentAgentVersion) == traces.countSpans()
 
     and: "Number of spans tagged with expected OS type"
-    traces.countFilteredResourceAttributes(OS_TYPE.key, isWindows ? WINDOWS : LINUX) == 1
+    traces.countFilteredResourceAttributes(OS_TYPE.key, isWindows ? WINDOWS : LINUX) == traces.countSpans()
 
     where:
     [appServer, jdk, isWindows] << getTestParams()

diff --git a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/PayaraSmokeTest.groovy b/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/PayaraSmokeTest.groovy
@@ -36,11 +36,6 @@ abstract class PayaraSmokeTest extends AppServerTest {
     }
     return super.getSpanName(path)
   }
-
-  @Override
-  boolean testRequestWebInfWebXml() {
-    false
-  }
 }
 
 @AppServer(version = "5.2020.6", jdk = "8")