Address review comment

Author: steverao

instrumentation/apache-elasticjob-3.0/javaagent/build.gradle.kts

@@ -18,8 +18,5 @@ dependencies {
 }
 
 tasks.withType<Test>().configureEach {
-  // required on jdk17
-  jvmArgs("--add-opens=java.base/java.lang=ALL-UNNAMED")
-  jvmArgs("-XX:+IgnoreUnrecognizedVMOptions")
   jvmArgs("-Dotel.instrumentation.apache-elasticjob.experimental-span-attributes=true")
 }

instrumentation/apache-elasticjob-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheelasticjob/v3_0/DataflowJobExecutorInstrumentation.java

@@ -50,15 +50,9 @@ public static ElasticJobHelper.ElasticJobScope onEnter(
         @Advice.Argument(3) ShardingContext shardingContext) {
 
       ElasticJobProcessRequest request =
-          ElasticJobProcessRequest.createWithUserJobInfo(
-              shardingContext.getJobName(),
-              shardingContext.getTaskId(),
-              shardingContext.getShardingItem(),
-              shardingContext.getShardingTotalCount(),
-              shardingContext.getShardingParameter() != null
-                  ? shardingContext.getShardingParameter()
-                  : "",
-              "DATAFLOW",
+          ElasticJobProcessRequest.createFromShardingContext(
+              shardingContext,
+              ElasticJobType.DATAFLOW.getValue(),
               elasticJob.getClass(),
               "processData");
 

instrumentation/apache-elasticjob-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheelasticjob/v3_0/ElasticJobCodeAttributesGetter.java

@@ -24,6 +24,6 @@ public String getMethodName(ElasticJobProcessRequest request) {
     if (request.isScriptJob() || request.isHttpJob()) {
       return null;
     }
-    return request.getUserMethodName() != null ? request.getUserMethodName() : "process";
+    return request.getUserMethodName();
   }
 }

instrumentation/apache-elasticjob-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheelasticjob/v3_0/ElasticJobExecutorInstrumentation.java

@@ -53,8 +53,8 @@ public static ElasticJobHelper.ElasticJobScope onEnter(
         @Advice.Argument(1) ShardingContexts shardingContexts,
         @Advice.Argument(2) int item) {
 
-      String jobType = determineJobTypeFromExecutor(jobItemExecutor);
-      if (!"SCRIPT".equals(jobType) && !"HTTP".equals(jobType)) {
+      ElasticJobType jobType = determineJobTypeFromExecutor(jobItemExecutor);
+      if (jobType != ElasticJobType.SCRIPT && jobType != ElasticJobType.HTTP) {
         return null;
       }
       ElasticJobProcessRequest request =
@@ -66,7 +66,7 @@ public static ElasticJobHelper.ElasticJobScope onEnter(
               shardingContexts.getShardingItemParameters() == null
                   ? ""
                   : shardingContexts.getShardingItemParameters().toString(),
-              jobType);
+              jobType.getValue());
       return helper().startSpan(request);
     }
 

instrumentation/apache-elasticjob-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheelasticjob/v3_0/ElasticJobExperimentalAttributeExtractor.java

@@ -5,25 +5,29 @@
 
 package io.opentelemetry.javaagent.instrumentation.apacheelasticjob.v3_0;
 
+import static io.opentelemetry.api.common.AttributeKey.longKey;
+import static io.opentelemetry.api.common.AttributeKey.stringKey;
+
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
 
 class ElasticJobExperimentalAttributeExtractor
     implements AttributesExtractor<ElasticJobProcessRequest, Void> {
 
   private static final AttributeKey<String> ELASTICJOB_JOB_NAME =
-      AttributeKey.stringKey("scheduling.apache-elasticjob.job.name");
+      stringKey("scheduling.apache-elasticjob.job.name");
   private static final AttributeKey<String> ELASTICJOB_TASK_ID =
-      AttributeKey.stringKey("scheduling.apache-elasticjob.task.id");
+      stringKey("scheduling.apache-elasticjob.task.id");
   private static final AttributeKey<Long> ELASTICJOB_SHARDING_ITEM_INDEX =
-      AttributeKey.longKey("scheduling.apache-elasticjob.sharding.item.index");
+      longKey("scheduling.apache-elasticjob.sharding.item.index");
   private static final AttributeKey<Long> ELASTICJOB_SHARDING_TOTAL_COUNT =
-      AttributeKey.longKey("scheduling.apache-elasticjob.sharding.total.count");
+      longKey("scheduling.apache-elasticjob.sharding.total.count");
   private static final AttributeKey<String> ELASTICJOB_SHARDING_ITEM_PARAMETERS =
-      AttributeKey.stringKey("scheduling.apache-elasticjob.sharding.item.parameters");
+      stringKey("scheduling.apache-elasticjob.sharding.item.parameters");
 
   @Override
   public void onStart(
@@ -35,7 +39,7 @@ public void onStart(
     attributes.put(ELASTICJOB_SHARDING_ITEM_INDEX, elasticJobProcessRequest.getShardingItemIndex());
     attributes.put(
         ELASTICJOB_SHARDING_TOTAL_COUNT, elasticJobProcessRequest.getShardingTotalCount());
-    if (elasticJobProcessRequest.getShardingItemParameters() != null) {
+    if (!StringUtils.isEmpty(elasticJobProcessRequest.getShardingItemParameters())) {
       attributes.put(
           ELASTICJOB_SHARDING_ITEM_PARAMETERS,
           elasticJobProcessRequest.getShardingItemParameters());

instrumentation/apache-elasticjob-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheelasticjob/v3_0/ElasticJobInstrumentationModule.java

@@ -5,24 +5,32 @@
 
 package io.opentelemetry.javaagent.instrumentation.apacheelasticjob.v3_0;
 
+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.Arrays;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
 import java.util.List;
 
 @AutoService(InstrumentationModule.class)
-public class ElasticJobInstrumentationModule extends InstrumentationModule {
+public class ElasticJobInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
 
   public ElasticJobInstrumentationModule() {
     super("apache-elasticjob", "apache-elasticjob-3.0");
   }
 
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
-    return Arrays.asList(
+    return asList(
         new ElasticJobExecutorInstrumentation(),
         new SimpleJobExecutorInstrumentation(),
         new DataflowJobExecutorInstrumentation());
   }
+
+  @Override
+  public boolean isIndyReady() {
+    return true;
+  }
 }

instrumentation/apache-elasticjob-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheelasticjob/v3_0/ElasticJobInstrumenterFactory.java

@@ -5,6 +5,8 @@
 
 package io.opentelemetry.javaagent.instrumentation.apacheelasticjob.v3_0;
 
+import org.apache.shardingsphere.elasticjob.api.ShardingContext;
+
 public final class ElasticJobProcessRequest {
   private String jobName;
   private String taskId;
@@ -14,7 +16,7 @@ public final class ElasticJobProcessRequest {
   private boolean failed;
   private String jobType;
   private Class<?> userJobClass;
-  private String userMethodName;
+  private String userMethodName = "process";
 
   public static ElasticJobProcessRequest create(
       String jobName,
@@ -54,6 +56,22 @@ public static ElasticJobProcessRequest createWithUserJobInfo(
     return request;
   }
 
+  public static ElasticJobProcessRequest createFromShardingContext(
+      ShardingContext shardingContext,
+      String jobType,
+      Class<?> userJobClass,
+      String userMethodName) {
+    return createWithUserJobInfo(
+        shardingContext.getJobName(),
+        shardingContext.getTaskId(),
+        shardingContext.getShardingItem(),
+        shardingContext.getShardingTotalCount(),
+        shardingContext.getShardingParameter(),
+        jobType,
+        userJobClass,
+        userMethodName);
+  }
+
   public void setFailed() {
     this.failed = true;
   }

instrumentation/apache-elasticjob-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheelasticjob/v3_0/ElasticJobProcessRequest.java

@@ -5,14 +5,48 @@
 
 package io.opentelemetry.javaagent.instrumentation.apacheelasticjob.v3_0;
 
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.trace.StatusCode;
+import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
+import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
 
 public final class ElasticJobSingletons {
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.apache-elasticjob-3.0";
+  private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES =
+      AgentInstrumentationConfig.get()
+          .getBoolean("otel.instrumentation.apache-elasticjob.experimental-span-attributes", false);
+
   private static final Instrumenter<ElasticJobProcessRequest, Void> INSTRUMENTER =
-      ElasticJobInstrumenterFactory.create(INSTRUMENTATION_NAME);
+      createInstrumenter();
   private static final ElasticJobHelper HELPER = ElasticJobHelper.create(INSTRUMENTER);
 
+  private static Instrumenter<ElasticJobProcessRequest, Void> createInstrumenter() {
+    ElasticJobCodeAttributesGetter codeAttributesGetter = new ElasticJobCodeAttributesGetter();
+    InstrumenterBuilder<ElasticJobProcessRequest, Void> builder =
+        Instrumenter.<ElasticJobProcessRequest, Void>builder(
+                GlobalOpenTelemetry.get(),
+                INSTRUMENTATION_NAME,
+                new ElasticJobSpanNameExtractor(codeAttributesGetter))
+            .addAttributesExtractor(CodeAttributesExtractor.create(codeAttributesGetter))
+            .setSpanStatusExtractor(
+                (spanStatusBuilder, elasticJobProcessRequest, unused, error) -> {
+                  if (error != null || elasticJobProcessRequest.isFailed()) {
+                    spanStatusBuilder.setStatus(StatusCode.ERROR);
+                  }
+                });
+    if (CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) {
+      builder.addAttributesExtractor(
+          AttributesExtractor.constant(AttributeKey.stringKey("job.system"), "elasticjob"));
+      builder.addAttributesExtractor(new ElasticJobExperimentalAttributeExtractor());
+    }
+
+    return builder.buildInstrumenter();
+  }
+
   public static ElasticJobHelper helper() {
     return HELPER;
   }

instrumentation/apache-elasticjob-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheelasticjob/v3_0/ElasticJobSingletons.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.apacheelasticjob.v3_0;
+
+public enum ElasticJobType {
+  SIMPLE("SIMPLE"),
+  DATAFLOW("DATAFLOW"),
+  HTTP("HTTP"),
+  SCRIPT("SCRIPT"),
+  UNKNOWN("UNKNOWN");
+
+  private final String value;
+
+  ElasticJobType(String value) {
+    this.value = value;
+  }
+
+  public String getValue() {
+    return value;
+  }
+
+  @Override
+  public String toString() {
+    return value;
+  }
+}