Addressed PR suggestions

open-telemetry · Mar 22, 2021 · eca4adc · eca4adc
1 parent baed675
commit eca4adc
Show file tree

Hide file tree

Showing 16 changed files with 45 additions and 34 deletions.
diff --git a/...o/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2InstrumentationModule.java b/...o/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2InstrumentationModule.java
@@ -10,7 +10,7 @@
 import static net.bytebuddy.matcher.ElementMatchers.not;
 
 import com.google.auto.service.AutoService;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.service.ServletAndFilterInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation;
 import io.opentelemetry.javaagent.tooling.InstrumentationModule;
 import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
 import java.util.Arrays;

diff --git a/...o/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java b/...o/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InstrumentationModule.java
@@ -8,8 +8,8 @@
 import static java.util.Arrays.asList;
 
 import com.google.auto.service.AutoService;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.async.AsyncContextInstrumentation;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.service.ServletAndFilterInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncContextInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation;
 import io.opentelemetry.javaagent.tooling.InstrumentationModule;
 import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
 import java.util.List;

diff --git a/...telemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java b/...telemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java
@@ -8,10 +8,10 @@
 import static java.util.Collections.singletonMap;
 
 import com.google.auto.service.AutoService;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.async.AsyncContextInstrumentation;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher.RequestDispatcherInstrumentation;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.response.HttpServletResponseInstrumentation;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.service.ServletAndFilterInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncContextInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher.RequestDispatcherInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation;
 import io.opentelemetry.javaagent.tooling.InstrumentationModule;
 import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
 import java.util.Arrays;

diff --git a/...ntelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherAdvice.java b/...ntelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherAdvice.java
@@ -10,6 +10,7 @@
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher.RequestDispatcherAdviceHelper;
 import jakarta.servlet.RequestDispatcher;
 import jakarta.servlet.ServletRequest;
 import java.lang.reflect.Method;

diff --git a/.../io/opentelemetry/javaagent/instrumentation/servlet/v5_0/response/ResponseSendAdvice.java b/.../io/opentelemetry/javaagent/instrumentation/servlet/v5_0/response/ResponseSendAdvice.java
@@ -12,6 +12,7 @@
 import io.opentelemetry.javaagent.instrumentation.api.CallDepth;
 import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
 import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseAdviceHelper;
 import jakarta.servlet.http.HttpServletResponse;
 import java.lang.reflect.Method;
 import net.bytebuddy.asm.Advice;
@@ -39,7 +40,6 @@ public static void stopSpan(
       @Advice.Local("otelContext") Context context,
       @Advice.Local("otelScope") Scope scope,
       @Advice.Local("otelCallDepth") CallDepth callDepth) {
-    HttpServletResponseAdviceHelper.stopSpan(
-        tracer(), HttpServletResponse.class, throwable, context, scope, callDepth);
+    HttpServletResponseAdviceHelper.stopSpan(tracer(), throwable, context, scope, callDepth);
   }
 }
diff --git a/..._0/async/AsyncContextInstrumentation.java → ...on/async/AsyncContextInstrumentation.java b/..._0/async/AsyncContextInstrumentation.java → ...on/async/AsyncContextInstrumentation.java
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.async;
+package io.opentelemetry.javaagent.instrumentation.servlet.common.async;
 
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface;
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher.hasClassesNamed;

diff --git a/...atcher/RequestDispatcherAdviceHelper.java → ...atcher/RequestDispatcherAdviceHelper.java b/...atcher/RequestDispatcherAdviceHelper.java → ...atcher/RequestDispatcherAdviceHelper.java
@@ -3,29 +3,37 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher;
+package io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher;
 
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.context.Context;
-import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
+import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer;
 
 public class RequestDispatcherAdviceHelper {
+  /**
+   * Determines if the advice for {@link RequestDispatcherInstrumentation} should create a new span
+   * and provides the context in which that span should be created.
+   *
+   * @param servletContextObject Value of the {@link HttpServerTracer#CONTEXT_ATTRIBUTE} attribute
+   *     of the servlet request.
+   * @return The context in which the advice should create the dispatcher span in. Returns <code>
+   *     null</code> in case a new span should not be created.
+   */
   public static Context getStartParentContext(Object servletContextObject) {
     Context parentContext = Context.current();
 
     Context servletContext =
         servletContextObject instanceof Context ? (Context) servletContextObject : null;
 
-    Span parentSpan = Java8BytecodeBridge.spanFromContext(parentContext);
+    Span parentSpan = Span.fromContext(parentContext);
     SpanContext parentSpanContext = parentSpan.getSpanContext();
     if (!parentSpanContext.isValid() && servletContext == null) {
       // Don't want to generate a new top-level span
       return null;
     }
 
-    Span servletSpan =
-        servletContext != null ? Java8BytecodeBridge.spanFromContext(servletContext) : null;
+    Span servletSpan = servletContext != null ? Span.fromContext(servletContext) : null;
     Context parent;
     if (servletContext == null
         || (parentSpanContext.isValid()

diff --git a/...her/RequestDispatcherInstrumentation.java → ...her/RequestDispatcherInstrumentation.java b/...her/RequestDispatcherInstrumentation.java → ...her/RequestDispatcherInstrumentation.java
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher;
+package io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher;
 
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface;
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher.hasClassesNamed;

diff --git a/...onse/HttpServletResponseAdviceHelper.java → ...onse/HttpServletResponseAdviceHelper.java b/...onse/HttpServletResponseAdviceHelper.java → ...onse/HttpServletResponseAdviceHelper.java
@@ -3,24 +3,18 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.response;
+package io.opentelemetry.javaagent.instrumentation.servlet.common.response;
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
 import io.opentelemetry.javaagent.instrumentation.api.CallDepth;
-import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
 
 public class HttpServletResponseAdviceHelper {
   public static void stopSpan(
-      BaseTracer tracer,
-      Class<?> responseClass,
-      Throwable throwable,
-      Context context,
-      Scope scope,
-      CallDepth callDepth) {
+      BaseTracer tracer, Throwable throwable, Context context, Scope scope, CallDepth callDepth) {
     if (callDepth.decrementAndGet() == 0 && context != null) {
-      CallDepthThreadLocalMap.reset(responseClass);
+      callDepth.reset();
 
       scope.close();
 

diff --git a/...e/HttpServletResponseInstrumentation.java → ...e/HttpServletResponseInstrumentation.java b/...e/HttpServletResponseInstrumentation.java → ...e/HttpServletResponseInstrumentation.java
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.response;
+package io.opentelemetry.javaagent.instrumentation.servlet.common.response;
 
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface;
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher.hasClassesNamed;

diff --git a/...vice/ServletAndFilterInstrumentation.java → ...vice/ServletAndFilterInstrumentation.java b/...vice/ServletAndFilterInstrumentation.java → ...vice/ServletAndFilterInstrumentation.java
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.service;
+package io.opentelemetry.javaagent.instrumentation.servlet.common.service;
 
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType;
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher.hasClassesNamed;

diff --git a/...ommon/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletAccessor.java b/...ommon/library/src/main/java/io/opentelemetry/instrumentation/servlet/ServletAccessor.java
@@ -7,6 +7,15 @@
 
 import java.security.Principal;
 
+/**
+ * This interface is used to access methods of HttpServletRequest and HttpServletResponse classes in
+ * shared code that is used for both jakarta.servlet and javax.servlet versions of those classes. A
+ * wrapper class with extra information attached may be used as well in cases where the class itself
+ * does not provide some field (such as response status for Servlet API 2.2).
+ *
+ * @param <RequestT> HttpServletRequest class (or a wrapper)
+ * @param <ResponseT> HttpServletResponse class (or a wrapper)
+ */
 public interface ServletAccessor<RequestT, ResponseT> {
   String getRequestContextPath(RequestT request);
 

diff --git a/...ntelemetry/javaagent/instrumentation/servlet/javax/JavaxServletInstrumentationModule.java b/...ntelemetry/javaagent/instrumentation/servlet/javax/JavaxServletInstrumentationModule.java
@@ -8,8 +8,8 @@
 import static java.util.Collections.singletonMap;
 
 import com.google.auto.service.AutoService;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher.RequestDispatcherInstrumentation;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.response.HttpServletResponseInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher.RequestDispatcherInstrumentation;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseInstrumentation;
 import io.opentelemetry.javaagent.tooling.InstrumentationModule;
 import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
 import java.util.Arrays;

diff --git a/...telemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherAdvice.java b/...telemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherAdvice.java
@@ -10,7 +10,7 @@
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher.RequestDispatcherAdviceHelper;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher.RequestDispatcherAdviceHelper;
 import java.lang.reflect.Method;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletRequest;

diff --git a/...io/opentelemetry/javaagent/instrumentation/servlet/javax/response/ResponseSendAdvice.java b/...io/opentelemetry/javaagent/instrumentation/servlet/javax/response/ResponseSendAdvice.java
@@ -12,7 +12,7 @@
 import io.opentelemetry.javaagent.instrumentation.api.CallDepth;
 import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
 import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
-import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.response.HttpServletResponseAdviceHelper;
+import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseAdviceHelper;
 import java.lang.reflect.Method;
 import javax.servlet.http.HttpServletResponse;
 import net.bytebuddy.asm.Advice;
@@ -40,7 +40,6 @@ public static void stopSpan(
       @Advice.Local("otelContext") Context context,
       @Advice.Local("otelScope") Scope scope,
       @Advice.Local("otelCallDepth") CallDepth callDepth) {
-    HttpServletResponseAdviceHelper.stopSpan(
-        tracer(), HttpServletResponse.class, throwable, context, scope, callDepth);
+    HttpServletResponseAdviceHelper.stopSpan(tracer(), throwable, context, scope, callDepth);
   }
 }
diff --git a/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/CallDepth.java b/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/CallDepth.java
@@ -28,7 +28,7 @@ public int get() {
     return depth;
   }
 
-  void reset() {
+  public void reset() {
     depth = 0;
   }
 }