Convert struts-2.3 to instrumenter api (#3835)

* Convert struts-2.3 to instrumenter api

* Apply suggestions from code review

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/ActionInvocationInstrumentation.java

@@ -5,9 +5,10 @@
 
 package io.opentelemetry.javaagent.instrumentation.struts2;
 
+import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER;
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
-import static io.opentelemetry.javaagent.instrumentation.struts2.Struts2Tracer.tracer;
+import static io.opentelemetry.javaagent.instrumentation.struts2.StrutsSingletons.instrumenter;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -15,6 +16,7 @@ import static net.bytebuddy.matcher.ElementMatchers.named;
 import com.opensymphony.xwork2.ActionInvocation;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
@@ -51,25 +53,32 @@ public class ActionInvocationInstrumentation implements TypeInstrumentation {
         @Advice.Local("otelScope") Scope scope) {
       Context parentContext = Java8BytecodeBridge.currentContext();
 
-      context = tracer().startSpan(parentContext, actionInvocation);
+      ServerSpanNaming.updateServerSpanName(
-      scope = context.makeCurrent();
+          parentContext,
+          CONTROLLER,
+          StrutsServerSpanNaming.getServerSpanNameSupplier(
+              parentContext, actionInvocation.getProxy()));
 
-      tracer().updateServerSpanName(parentContext, actionInvocation.getProxy());
+      if (!instrumenter().shouldStart(parentContext, actionInvocation)) {
+        return;
+      }
+
+      context = instrumenter().start(parentContext, actionInvocation);
+      scope = context.makeCurrent();
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
         @Advice.Thrown Throwable throwable,
+        @Advice.This ActionInvocation actionInvocation,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
-      if (scope != null) {
+      if (scope == null) {
-        scope.close();
+        return;
-      }
-      if (throwable != null) {
-        tracer().endExceptionally(context, throwable);
-      } else {
-        tracer().end(context);
       }
+      scope.close();
+
+      instrumenter().end(context, actionInvocation, null, throwable);
     }
   }
 }

instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/Struts2Tracer.java

@@ -1,77 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.struts2;
-
-import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
-import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER;
-
-import com.opensymphony.xwork2.ActionInvocation;
-import com.opensymphony.xwork2.ActionProxy;
-import io.opentelemetry.api.trace.SpanBuilder;
-import io.opentelemetry.context.Context;
-import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
-import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
-import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
-import io.opentelemetry.instrumentation.api.tracer.SpanNames;
-import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
-
-public class Struts2Tracer extends BaseTracer {
-
-  private static final Struts2Tracer TRACER = new Struts2Tracer();
-
-  public static Struts2Tracer tracer() {
-    return TRACER;
-  }
-
-  public Context startSpan(Context parentContext, ActionInvocation actionInvocation) {
-    Object action = actionInvocation.getAction();
-    Class<?> actionClass = action.getClass();
-
-    String method = actionInvocation.getProxy().getMethod();
-    String spanName = SpanNames.fromMethod(actionClass, method);
-
-    SpanBuilder strutsSpan = spanBuilder(parentContext, spanName, INTERNAL);
-
-    strutsSpan.setAttribute(SemanticAttributes.CODE_NAMESPACE, actionClass.getName());
-    if (method != null) {
-      strutsSpan.setAttribute(SemanticAttributes.CODE_FUNCTION, method);
-    }
-
-    return parentContext.with(strutsSpan.startSpan());
-  }
-
-  // Handle cases where action parameters are encoded into URL path
-  public void updateServerSpanName(Context context, ActionProxy actionProxy) {
-    ServerSpanNaming.updateServerSpanName(
-        context, CONTROLLER, () -> getServerSpanName(context, actionProxy));
-  }
-
-  private static String getServerSpanName(Context context, ActionProxy actionProxy) {
-    // We take name from the config, because it contains the path pattern from the
-    // configuration.
-    String result = actionProxy.getConfig().getName();
-
-    String actionNamespace = actionProxy.getNamespace();
-    if (actionNamespace != null && !actionNamespace.isEmpty()) {
-      if (actionNamespace.endsWith("/") || result.startsWith("/")) {
-        result = actionNamespace + result;
-      } else {
-        result = actionNamespace + "/" + result;
-      }
-    }
-
-    if (!result.startsWith("/")) {
-      result = "/" + result;
-    }
-
-    return ServletContextPath.prepend(context, result);
-  }
-
-  @Override
-  protected String getInstrumentationName() {
-    return "io.opentelemetry.struts-2.3";
-  }
-}

instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/StrutsCodeAttributesExtractor.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.struts2;
+
+import com.opensymphony.xwork2.ActionInvocation;
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+public class StrutsCodeAttributesExtractor extends CodeAttributesExtractor<ActionInvocation, Void> {
+
+  @Override
+  protected Class<?> codeClass(ActionInvocation actionInvocation) {
+    return actionInvocation.getAction().getClass();
+  }
+
+  @Override
+  protected String methodName(ActionInvocation actionInvocation) {
+    return actionInvocation.getProxy().getMethod();
+  }
+
+  @Override
+  protected @Nullable String filePath(ActionInvocation actionInvocation) {
+    return null;
+  }
+
+  @Override
+  protected @Nullable Long lineNumber(ActionInvocation actionInvocation) {
+    return null;
+  }
+}

instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/StrutsServerSpanNaming.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.struts2;
+
+import com.opensymphony.xwork2.ActionProxy;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
+import java.util.function.Supplier;
+
+public class StrutsServerSpanNaming {
+
+  public static Supplier<String> getServerSpanNameSupplier(
+      Context context, ActionProxy actionProxy) {
+    return () -> getServerSpanName(context, actionProxy);
+  }
+
+  private static String getServerSpanName(Context context, ActionProxy actionProxy) {
+    // We take name from the config, because it contains the path pattern from the
+    // configuration.
+    String result = actionProxy.getConfig().getName();
+
+    String actionNamespace = actionProxy.getNamespace();
+    if (actionNamespace != null && !actionNamespace.isEmpty()) {
+      if (actionNamespace.endsWith("/") || result.startsWith("/")) {
+        result = actionNamespace + result;
+      } else {
+        result = actionNamespace + "/" + result;
+      }
+    }
+
+    if (!result.startsWith("/")) {
+      result = "/" + result;
+    }
+
+    return ServletContextPath.prepend(context, result);
+  }
+}

instrumentation/struts-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/struts2/StrutsSingletons.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.struts2;
+
+import com.opensymphony.xwork2.ActionInvocation;
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.code.CodeSpanNameExtractor;
+
+public class StrutsSingletons {
+  private static final String INSTRUMENTATION_NAME = "io.opentelemetry.struts-2.3";
+
+  private static final Instrumenter<ActionInvocation, Void> INSTRUMENTER;
+
+  static {
+    CodeAttributesExtractor<ActionInvocation, Void> codeAttributes =
+        new StrutsCodeAttributesExtractor();
+    INSTRUMENTER =
+        Instrumenter.<ActionInvocation, Void>newBuilder(
+                GlobalOpenTelemetry.get(),
+                INSTRUMENTATION_NAME,
+                CodeSpanNameExtractor.create(codeAttributes))
+            .addAttributesExtractor(codeAttributes)
+            .newInstrumenter();
+  }
+
+  public static Instrumenter<ActionInvocation, Void> instrumenter() {
+    return INSTRUMENTER;
+  }
+
+  private StrutsSingletons() {}
+}