cleanups

Author: jaydeluca

.gitignore

@@ -59,3 +59,4 @@ replay_pid*
 .attach_pid*
 
 !java-agent/benchmark/releases/*.jar
+

instrumentation-docs/build.gradle.kts

@@ -9,9 +9,6 @@ otelJava {
 dependencies {
   implementation("org.yaml:snakeyaml:2.4")
   implementation("io.opentelemetry:opentelemetry-sdk-common")
-  implementation("io.opentelemetry:opentelemetry-sdk-metrics")
-  implementation("com.fasterxml.jackson.core:jackson-databind:2.15.2")
-  implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.15.2")
 
   testImplementation(enforcedPlatform("org.junit:junit-bom:5.12.0"))
   testImplementation("org.assertj:assertj-core:3.27.3")

instrumentation-docs/collect.sh

@@ -4,34 +4,9 @@ Runs analysis on instrumentation modules in order to generate documentation.
 
 ## How to use
 
-Until this process is ready for all instrumentations, each module will be modified to include a
-system property configured for when the tests run:
-
-```kotlin
-tasks {
-  test {
-    systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
-    ...
-  }
-}
-```
-
-Prior to running the DocGeneratorApplication, run the following command to collect metadata:
-
-* Run tests to collect metadata
-  * `./gradlew test -collectMetadata=true`
-* Run the doc generator
-  * `./gradlew :instrumentation-docs:generateDocs`
+Run the doc generator:
 
-You can also use an environment variable to set it for the entire suite:
-
-`COLLECT_METADATA=true ./gradlew test`
-
-or use the helper script that will run only the currently supported tests:
-
-```bash
-./instrumentation-docs/collect.sh
-```
+`./gradlew :instrumentation-docs:generateDocs`
 
 ## Instrumentation Hierarchy
 
@@ -82,12 +57,7 @@ public class SpringWebInstrumentationModule extends InstrumentationModule
 * target_versions
   * List of supported versions by the module, broken down by `library` or `javaagent` support
 * scope
-  * The scope of the instrumentation, including the name, version, schemaUrl and attributes
-* metrics
-  * List of metrics that the instrumentation module collects, including the metric name, description, type, and attributes
-* span_data
-  * List of span types that are emitted (internal, client, server)
-  * List of span data that the instrumentation collects, including the attribute name and type
+  * Name: The scope name of the instrumentation, `io.opentelemetry.{instrumentation name}`
 
 ## Methodology
 
@@ -110,57 +80,10 @@ We parse gradle files in order to determine the target versions.
 - Library versions are determined by the library dependency versions
   - when available, latestDepTestLibrary is used to determine the latest supported version
 
-### Scope, Span Data, and Metrics
-
-Since many of our tests generate this information, we can hook into the InstrumentationTestRunner
-class and collect the metrics, spans, and scope generated. We can then leverage the `afterTestClass()`
-in the Agent and library test runners to then write this information into temporary files.
-
-The data is written into a `.telemetry` directory in the root of the instrumentation module. This data
-will be excluded from git and just generated on demand.
+### Scope
 
-Since metadata can be generated by multiple classes, we will generate a file per class.
+For now, the scope name is the only value that is implemented in our instrumentations. The scope
+name is determined by the instrumentation module name:  `io.opentelemetry.{instrumentation name}`
 
-Example output:
-
-```
-clickhouse:
-  instrumentations:
-  - name: clickhouse-client-0.5
-    description: Instruments the V1 ClickHouseClient, providing database client spans
-      and metrics.
-    srcPath: instrumentation/clickhouse-client-0.5
-    target_versions:
-      javaagent:
-      - com.clickhouse.client:clickhouse-client:[0.5.0,)
-    metrics:
-    - name: db.client.operation.duration
-      description: Duration of database client operations.
-      type: HISTOGRAM
-      unit: s
-      attributes:
-      - name: db.namespace
-        type: STRING
-      - name: db.operation.name
-        type: STRING
-      - name: db.system.name
-        type: STRING
-      - name: server.address
-        type: STRING
-      - name: server.port
-        type: LONG
-    span_data:
-      attributes:
-      - name: server.address
-        type: STRING
-      - name: server.port
-        type: LONG
-      - name: db.system.name
-        type: STRING
-      - name: db.query.text
-        type: STRING
-      - name: db.namespace
-        type: STRING
-      - name: db.operation.name
-        type: STRING
-```
+We will implement gatherers for the schemaUrl and attributes when instrumentations start
+implementing them.

instrumentation-docs/readme.md

@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.docs.internal;
 
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
 import java.util.Map;
 import java.util.Set;
 
@@ -18,6 +19,7 @@ public class InstrumentationEntity {
   private final String instrumentationName;
   private final String namespace;
   private final String group;
+  private final InstrumentationScopeInfo scopeInfo;
   private Map<InstrumentationType, Set<String>> targetVersions;
 
   public void setTargetVersions(Map<InstrumentationType, Set<String>> targetVersions) {
@@ -41,6 +43,7 @@ public InstrumentationEntity(Builder builder) {
     this.group = builder.group;
     this.metadata = builder.metadata;
     this.targetVersions = builder.targetVersions;
+    this.scopeInfo = InstrumentationScopeInfo.create("io.opentelemetry." + instrumentationName);
   }
 
   public String getSrcPath() {
@@ -67,6 +70,10 @@ public Map<InstrumentationType, Set<String>> getTargetVersions() {
     return targetVersions;
   }
 
+  public InstrumentationScopeInfo getScopeInfo() {
+    return scopeInfo;
+  }
+
   /**
    * This class is internal and is hence not for public use. Its APIs are unstable and can change at
    * any time.

instrumentation-docs/src/main/java/io/opentelemetry/instrumentation/docs/internal/InstrumentationEntity.java

@@ -70,7 +70,7 @@ public static void printInstrumentationList(
 
   private static Map<String, Object> getScopeMap(InstrumentationEntity entity) {
     Map<String, Object> scopeMap = new LinkedHashMap<>();
-    scopeMap.put("name", "io.opentelemetry." + entity.getInstrumentationName());
+    scopeMap.put("name", entity.getScopeInfo().getName());
     return scopeMap;
   }
 

instrumentation-docs/src/main/java/io/opentelemetry/instrumentation/docs/utils/YamlHelper.java

@@ -52,6 +52,8 @@ void testConvertToEntities() {
     assertThat(log4jEntity.getNamespace()).isEqualTo("log4j");
     assertThat(log4jEntity.getGroup()).isEqualTo("log4j");
     assertThat(log4jEntity.getSrcPath()).isEqualTo("instrumentation/log4j/log4j-appender-2.17");
+    assertThat(log4jEntity.getScopeInfo().getName())
+        .isEqualTo("io.opentelemetry.log4j-appender-2.17");
 
     InstrumentationEntity springEntity =
         entities.stream()
@@ -63,5 +65,6 @@ void testConvertToEntities() {
     assertThat(springEntity.getNamespace()).isEqualTo("spring");
     assertThat(springEntity.getGroup()).isEqualTo("spring");
     assertThat(springEntity.getSrcPath()).isEqualTo("instrumentation/spring/spring-web");
+    assertThat(springEntity.getScopeInfo().getName()).isEqualTo("io.opentelemetry.spring-web");
   }
 }

instrumentation-docs/src/test/java/io/opentelemetry/instrumentation/docs/InstrumentationAnalyzerTest.java

@@ -18,9 +18,3 @@ dependencies {
 
   testImplementation(project(":instrumentation:alibaba-druid-1.0:testing"))
 }
-
-tasks {
-  test {
-    systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
-  }
-}

instrumentation/alibaba-druid-1.0/javaagent/build.gradle.kts

@@ -19,6 +19,5 @@ dependencies {
 tasks {
   withType<Test>().configureEach {
     systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean)
-    systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
   }
 }

instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/build.gradle.kts

@@ -24,12 +24,10 @@ dependencies {
 tasks {
   test {
     usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service)
-    systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
   }
 
   val testStableSemconv by registering(Test::class) {
     jvmArgs("-Dotel.semconv-stability.opt-in=database")
-    systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
   }
 
   check {

instrumentation/clickhouse-client-0.5/javaagent/build.gradle.kts

@@ -5,18 +5,14 @@
 
 package io.opentelemetry.javaagent.instrumentation.clickhouse;
 
-import static io.opentelemetry.instrumentation.testing.junit.db.DbClientMetricsTestUtil.assertDurationMetric;
 import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStable;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
 import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS;
 import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAME;
-import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAMESPACE;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION;
-import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION_NAME;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM;
-import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM_NAME;
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
@@ -93,7 +89,6 @@ void cleanup() {
   }
 
   @Test
-  @SuppressWarnings("deprecation") // using deprecated semconv
   void testConnectionStringWithoutDatabaseSpecifiedStillGeneratesSpans()
       throws ClickHouseException {
     ClickHouseNode server = ClickHouseNode.of("http://" + host + ":" + port + "?compress=0");
@@ -116,15 +111,6 @@ void testConnectionStringWithoutDatabaseSpecifiedStillGeneratesSpans()
                         .hasNoParent()
                         .hasAttributesSatisfyingExactly(
                             attributeAssertions("select * from " + tableName, "SELECT"))));
-
-    assertDurationMetric(
-        testing,
-        "io.opentelemetry.clickhouse-client-0.5",
-        DB_SYSTEM_NAME,
-        DB_OPERATION_NAME,
-        DB_NAMESPACE,
-        SERVER_ADDRESS,
-        SERVER_PORT);
   }
 
   @Test

instrumentation/clickhouse-client-0.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/clickhouse/ClickHouseClientTest.java

@@ -59,7 +59,6 @@ public void afterTestClass() {
     assert adviceFailureCount == 0 : adviceFailureCount + " Advice failures during test";
     int muzzleFailureCount = TestAgentListenerAccess.getAndResetMuzzleFailureCount();
     assert muzzleFailureCount == 0 : muzzleFailureCount + " Muzzle failures during test";
-
     // additional library ignores are ignored during tests, because they can make it really
     // confusing for contributors wondering why their instrumentation is not applied
     //

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/AgentTestRunner.java

@@ -12,14 +12,12 @@
 import io.opentelemetry.instrumentation.testing.util.TelemetryDataUtil;
 import io.opentelemetry.instrumentation.testing.util.ThrowingRunnable;
 import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier;
-import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
 import io.opentelemetry.sdk.logs.data.LogRecordData;
 import io.opentelemetry.sdk.metrics.data.MetricData;
 import io.opentelemetry.sdk.testing.assertj.MetricAssert;
 import io.opentelemetry.sdk.testing.assertj.TraceAssert;
 import io.opentelemetry.sdk.testing.assertj.TracesAssert;
 import io.opentelemetry.sdk.trace.data.SpanData;
-import java.io.IOException;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -46,15 +44,13 @@ public abstract class InstrumentationTestRunner {
 
   private final TestInstrumenters testInstrumenters;
 
-  protected InstrumentationScopeInfo instrumentationScope;
-
   protected InstrumentationTestRunner(OpenTelemetry openTelemetry) {
     testInstrumenters = new TestInstrumenters(openTelemetry);
   }
 
   public abstract void beforeTestClass();
 
-  public abstract void afterTestClass() throws IOException;
+  public abstract void afterTestClass();
 
   public abstract void clearAllExportedData();
 
@@ -68,10 +64,6 @@ protected InstrumentationTestRunner(OpenTelemetry openTelemetry) {
 
   public abstract boolean forceFlushCalled();
 
-  protected boolean collectMetadata =
-      Boolean.getBoolean("collectMetadata")
-          || Boolean.parseBoolean(System.getenv("COLLECT_METADATA"));
-
   /** Return a list of all captured traces, where each trace is a sorted list of spans. */
   public final List<List<SpanData>> traces() {
     return TelemetryDataUtil.groupTraces(getExportedSpans());
@@ -139,7 +131,6 @@ private <T extends Consumer<TraceAssert>> void doAssertTraces(
     if (verifyScopeVersion) {
       TelemetryDataUtil.assertScopeVersion(traces);
     }
-
     if (traceComparator != null) {
       traces.sort(traceComparator);
     }